fix(builtin): linker silently not generating expected links in windows

devversion · alexeagle · commit 2979fad65796 · 2020-07-04T06:54:47.000-07:00
Currently the builtin linker sometimes does not generate links as
defined in the module mappings on Windows. This results in unexpected
and confusing runtime issues, and differences with other platforms.

The linker fails to generate links if module mappings result in
a different symlink/module hierarchy (as computed by `reduceModules`).

For example, consider in a previous linker run, we linked the module
`@angular/cdk/overlay` into `node_modules/@angular/cdk/overlay`. In
a second run then, we actually link `@angular/cdk`. The linker will
fail to do that as the `node_modules/@angular/cdk` folder already
exists (due to missing sandbox/runfile symlinking in windows)

We fix this by clearing such leftover linker directories so that
the newly configured module mapping can be created. In order to
avoid race conditions in non-sandboxed environments, we need to
pay special attention to potential concurrent resource accesses,
and also need to preserve possible child links from previous or
concurrent linker runs.
diff --git a/internal/linker/index.js b/internal/linker/index.js
@@ -45,6 +45,56 @@ function mkdirp(p) {
         }
     });
 }
+function gracefulLstat(path) {
+    return __awaiter(this, void 0, void 0, function* () {
+        try {
+            return yield fs.promises.lstat(path);
+        }
+        catch (e) {
+            if (e.code === 'ENOENT') {
+                return null;
+            }
+            throw e;
+        }
+    });
+}
+function unlink(moduleName) {
+    return __awaiter(this, void 0, void 0, function* () {
+        const stat = yield gracefulLstat(moduleName);
+        if (stat === null) {
+            return;
+        }
+        log_verbose(`unlink( ${moduleName} )`);
+        if (stat.isDirectory()) {
+            yield deleteDirectory(moduleName);
+        }
+        else {
+            log_verbose("Deleting file: ", moduleName);
+            yield fs.promises.unlink(moduleName);
+        }
+    });
+}
+function deleteDirectory(p) {
+    return __awaiter(this, void 0, void 0, function* () {
+        log_verbose("Deleting children of", p);
+        for (let entry of yield fs.promises.readdir(p)) {
+            const childPath = path.join(p, entry);
+            const stat = yield gracefulLstat(childPath);
+            if (stat === null) {
+                throw Error(`File does not exist, but is listed as directory entry: ${childPath}`);
+            }
+            if (stat.isDirectory()) {
+                yield deleteDirectory(childPath);
+            }
+            else {
+                log_verbose("Deleting file", childPath);
+                yield fs.promises.unlink(childPath);
+            }
+        }
+        log_verbose("Cleaning up dir", p);
+        yield fs.promises.rmdir(p);
+    });
+}
 function symlink(target, p) {
     return __awaiter(this, void 0, void 0, function* () {
         log_verbose(`symlink( ${p} -> ${target} )`);
@@ -210,21 +260,12 @@ class Runfiles {
 exports.Runfiles = Runfiles;
 function exists(p) {
     return __awaiter(this, void 0, void 0, function* () {
-        try {
-            yield fs.promises.stat(p);
-            return true;
-        }
-        catch (e) {
-            if (e.code === 'ENOENT') {
-                return false;
-            }
-            throw e;
-        }
+        return ((yield gracefulLstat(p)) !== null);
     });
 }
 function existsSync(p) {
     try {
-        fs.statSync(p);
+        fs.lstatSync(p);
         return true;
     }
     catch (e) {
@@ -324,6 +365,23 @@ function isDirectChildLink([parentRel, parentPath], [childRel, childPath]) {
 function isNameLinkPathTopAligned(namePath, [, linkPath]) {
     return path.basename(namePath) === path.basename(linkPath);
 }
+function visitDirectoryPreserveLinks(dirPath, visit) {
+    return __awaiter(this, void 0, void 0, function* () {
+        for (const entry of yield fs.promises.readdir(dirPath)) {
+            const childPath = path.join(dirPath, entry);
+            const stat = yield gracefulLstat(childPath);
+            if (stat === null) {
+                continue;
+            }
+            if (stat.isDirectory()) {
+                yield visitDirectoryPreserveLinks(childPath, visit);
+            }
+            else {
+                yield visit(childPath, stat);
+            }
+        }
+    });
+}
 function main(args, runfiles) {
     return __awaiter(this, void 0, void 0, function* () {
         if (!args || args.length < 1)
@@ -346,6 +404,41 @@ function main(args, runfiles) {
         }
         yield symlink(rootDir, 'node_modules');
         process.chdir(rootDir);
+        function isLeftoverDirectoryFromLinker(stats, modulePath) {
+            return __awaiter(this, void 0, void 0, function* () {
+                if (runfiles.manifest === undefined) {
+                    return false;
+                }
+                if (!stats.isDirectory()) {
+                    return false;
+                }
+                let isLeftoverFromPreviousLink = true;
+                yield visitDirectoryPreserveLinks(modulePath, (childPath, childStats) => __awaiter(this, void 0, void 0, function* () {
+                    if (!childStats.isSymbolicLink()) {
+                        isLeftoverFromPreviousLink = false;
+                    }
+                }));
+                return isLeftoverFromPreviousLink;
+            });
+        }
+        function createSymlinkAndPreserveContents(stats, modulePath, target) {
+            return __awaiter(this, void 0, void 0, function* () {
+                const tmpPath = `${modulePath}__linker_tmp`;
+                log_verbose(`createSymlinkAndPreserveContents( ${modulePath} )`);
+                yield symlink(target, tmpPath);
+                yield visitDirectoryPreserveLinks(modulePath, (childPath, stat) => __awaiter(this, void 0, void 0, function* () {
+                    if (stat.isSymbolicLink()) {
+                        const targetPath = path.join(tmpPath, path.relative(modulePath, childPath));
+                        log_verbose(`Cloning symlink into temporary created link ( ${childPath} )`);
+                        yield mkdirp(path.dirname(targetPath));
+                        yield symlink(targetPath, yield fs.promises.realpath(childPath));
+                    }
+                }));
+                log_verbose(`Removing existing module so that new link can take place ( ${modulePath} )`);
+                yield unlink(modulePath);
+                yield fs.promises.rename(tmpPath, modulePath);
+            });
+        }
         function linkModules(m) {
             return __awaiter(this, void 0, void 0, function* () {
                 yield mkdirp(path.dirname(m.name));
@@ -381,16 +474,22 @@ function main(args, runfiles) {
                             }
                             break;
                     }
-                    yield symlink(target, m.name);
+                    const stats = yield gracefulLstat(m.name);
+                    if (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name))) {
+                        yield createSymlinkAndPreserveContents(stats, m.name, target);
+                    }
+                    else {
+                        yield symlink(target, m.name);
+                    }
                 }
                 if (m.children) {
                     yield Promise.all(m.children.map(linkModules));
                 }
             });
         }
-        const moduleHeirarchy = reduceModules(modules);
-        log_verbose(`mapping hierarchy ${JSON.stringify(moduleHeirarchy)}`);
-        const links = moduleHeirarchy.map(linkModules);
+        const moduleHierarchy = reduceModules(modules);
+        log_verbose(`mapping hierarchy ${JSON.stringify(moduleHierarchy)}`);
+        const links = moduleHierarchy.map(linkModules);
         let code = 0;
         yield Promise.all(links).catch(e => {
             log_error(e);
diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts
@@ -47,6 +47,60 @@ async function mkdirp(p: string) {
   }
 }
 
+/**
+ * Gets the `lstat` results for a given path. Returns `null` if the path
+ * does not exist on disk.
+ */
+async function gracefulLstat(path: string): Promise<fs.Stats|null> {
+  try {
+    return await fs.promises.lstat(path);
+  } catch (e) {
+    if (e.code === 'ENOENT') {
+      return null;
+    }
+    throw e;
+  }
+}
+
+/**
+ * Deletes the given module name from the current working directory (i.e. symlink root).
+ * If the module name resolves to a directory, the directory is deleted. Otherwise the
+ * existing file or junction is unlinked.
+ */
+async function unlink(moduleName: string) {
+  const stat = await gracefulLstat(moduleName);
+  if (stat === null) {
+    return;
+  }
+  log_verbose(`unlink( ${moduleName} )`);
+  if (stat.isDirectory()) {
+    await deleteDirectory(moduleName);
+  } else {
+    log_verbose("Deleting file: ", moduleName);
+    await fs.promises.unlink(moduleName);
+  }
+}
+
+/** Asynchronously deletes a given directory (with contents). */
+async function deleteDirectory(p: string) {
+  log_verbose("Deleting children of", p);
+  for (let entry of await fs.promises.readdir(p)) {
+    const childPath = path.join(p, entry);
+    const stat = await gracefulLstat(childPath);
+    if (stat === null) {
+      throw Error(`File does not exist, but is listed as directory entry: ${childPath}`);
+    }
+    if (stat.isDirectory()) {
+      await deleteDirectory(childPath);
+    } else {
+      log_verbose("Deleting file", childPath);
+      await fs.promises.unlink(childPath);
+    }
+  }
+  log_verbose("Cleaning up dir", p);
+  await fs.promises.rmdir(p);
+}
+
 async function symlink(target: string, p: string): Promise<boolean> {
   log_verbose(`symlink( ${p} -> ${target} )`);
 
@@ -55,7 +109,7 @@ async function symlink(target: string, p: string): Promise<boolean> {
   // it is necessary for the time being.
   if (!await exists(target)) {
     // This can happen if a module mapping is propogated from a dependency
-    // but the targat that generated the mapping in not in the deps. We don't
+    // but the target that generated the mapping in not in the deps. We don't
     // want to create symlinks to non-existant targets as this will
     // break any nested symlinks that may be created under the module name
     // after this.
@@ -334,20 +388,12 @@ declare global {
 // There is no fs.promises.exists function because
 // node core is of the opinion that exists is always too racey to rely on.
 async function exists(p: string) {
-  try {
-    await fs.promises.stat(p)
-    return true;
-  } catch (e) {
-    if (e.code === 'ENOENT') {
-      return false;
-    }
-    throw e;
-  }
+  return (await gracefulLstat(p) !== null);
 }
 
 function existsSync(p: string) {
   try {
-    fs.statSync(p)
+    fs.lstatSync(p);
     return true;
   } catch (e) {
     if (e.code === 'ENOENT') {
@@ -508,6 +554,22 @@ function isNameLinkPathTopAligned(namePath: string, [, linkPath]: Link) {
   return path.basename(namePath) === path.basename(linkPath);
 }
 
+async function visitDirectoryPreserveLinks(
+    dirPath: string, visit: (filePath: string, stat: fs.Stats) => Promise<void>) {
+  for (const entry of await fs.promises.readdir(dirPath)) {
+    const childPath = path.join(dirPath, entry);
+    const stat = await gracefulLstat(childPath);
+    if (stat === null) {
+      continue;
+    }
+    if (stat.isDirectory()) {
+      await visitDirectoryPreserveLinks(childPath, visit);
+    } else {
+      await visit(childPath, stat);
+    }
+  }
+}
+
 // See link_node_modules.bzl where these link roots types
 // are used to indicate which root the linker should target
 // for each package:
@@ -567,6 +629,64 @@ export async function main(args: string[], runfiles: Runfiles) {
   // symlinks will be created under node_modules
   process.chdir(rootDir);
 
+  /**
+   * Whether the given module resolves to a directory that has been created by a previous linker
+   * run purely to make space for deep module links. e.g. consider a mapping for `my-pkg/a11y`.
+   * The linker will create folders like `node_modules/my-pkg/` so that the `a11y` symbolic
+   * junction can be created. The `my-pkg` folder is then considered a leftover from a previous
+   * linker run as it only contains symbolic links and no actual source files.
+   */
+  async function isLeftoverDirectoryFromLinker(stats: fs.Stats, modulePath: string) {
+    // If we are running without a runfiles manifest (i.e. in sandbox or with symlinked runfiles),
+    // then this is guaranteed to be not an artifact from a previous linker run.
+    if (runfiles.manifest === undefined) {
+      return false;
+    }
+    if (!stats.isDirectory()) {
+      return false;
+    }
+    let isLeftoverFromPreviousLink = true;
+    // If the directory contains actual files, this cannot be a leftover from a previous
+    // linker run. The linker only creates directories in the node modules that hold
+    // symbolic links for configured module mappings.
+    await visitDirectoryPreserveLinks(modulePath, async (childPath, childStats) => {
+      if (!childStats.isSymbolicLink()) {
+        isLeftoverFromPreviousLink = false;
+      }
+    });
+    return isLeftoverFromPreviousLink;
+  }
+
+  /**
+   * Creates a symlink for the given module. Existing child symlinks which are part of
+   * the module are preserved in order to not cause race conditions in non-sandbox
+   * environments where multiple actions rely on the same node modules root.
+   *
+   * To avoid unexpected resource removal, a new temporary link for the target is created.
+   * Then all symlinks from the existing module are cloned. Once done, the existing module
+   * is unlinked while the temporary link takes place for the given module. This ensures
+   * that the module link is never removed at any time (causing race condition failures).
+   */
+  async function createSymlinkAndPreserveContents(stats: fs.Stats, modulePath: string,
+                                                  target: string) {
+    const tmpPath = `${modulePath}__linker_tmp`;
+    log_verbose(`createSymlinkAndPreserveContents( ${modulePath} )`);
+
+    await symlink(target, tmpPath);
+    await visitDirectoryPreserveLinks(modulePath, async (childPath, stat) => {
+      if (stat.isSymbolicLink()) {
+        const targetPath = path.join(tmpPath, path.relative(modulePath, childPath));
+        log_verbose(`Cloning symlink into temporary created link ( ${childPath} )`);
+        await mkdirp(path.dirname(targetPath));
+        await symlink(targetPath, await fs.promises.realpath(childPath));
+      }
+    });
+
+    log_verbose(`Removing existing module so that new link can take place ( ${modulePath} )`);
+    await unlink(modulePath);
+    await fs.promises.rename(tmpPath, modulePath);
+  }
+
   async function linkModules(m: LinkerTreeElement) {
     // ensure the parent directory exist
     await mkdirp(path.dirname(m.name));
@@ -605,7 +725,20 @@ export async function main(args: string[], runfiles: Runfiles) {
           break;
       }
 
-      await symlink(target, m.name);
+      const stats = await gracefulLstat(m.name);
+      // In environments where runfiles are not symlinked (e.g. Windows), existing linked
+      // modules are preserved. This could cause issues when a link is created at higher level
+      // as a conflicting directory is already on disk. e.g. consider in a previous run, we
+      // linked the modules `my-pkg/overlay`. Later on, in another run, we have a module mapping
+      // for `my-pkg` itself. The linker cannot create `my-pkg` because the directory `my-pkg`
+      // already exists. To ensure that the desired link is generated, we create the new desired
+      // link and move all previous nested links from the old module into the new link. Read more
+      // about this in the description of `createSymlinkAndPreserveContents`.
+      if (stats !== null && await isLeftoverDirectoryFromLinker(stats, m.name)) {
+        await createSymlinkAndPreserveContents(stats, m.name, target);
+      } else {
+        await symlink(target, m.name);
+      }
     }
 
     // Process each child branch concurrently
@@ -614,11 +747,11 @@ export async function main(args: string[], runfiles: Runfiles) {
     }
   }
 
-  const moduleHeirarchy = reduceModules(modules);
-  log_verbose(`mapping hierarchy ${JSON.stringify(moduleHeirarchy)}`);
+  const moduleHierarchy = reduceModules(modules);
+  log_verbose(`mapping hierarchy ${JSON.stringify(moduleHierarchy)}`);
 
   // Process each root branch concurrently
-  const links = moduleHeirarchy.map(linkModules);
+  const links = moduleHierarchy.map(linkModules);
 
   let code = 0;
   await Promise.all(links).catch(e => {
diff --git a/internal/linker/test/link_node_modules.spec.ts b/internal/linker/test/link_node_modules.spec.ts
