fix(builtin): make linker deterministic when resolving from manifest & fix link_workspace_root with no runfiles

Author: gregmagolan

internal/linker/index.js

@@ -12,6 +12,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
 const fs = require("fs");
 const path = require("path");
 const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS'];
+const BAZEL_OUT_REGEX = /(\/bazel-out\/|\/bazel-~1\/x64_wi~1\/)/;
 function log_verbose(...m) {
     if (VERBOSE_LOGS)
         console.error('[link_node_modules.js]', ...m);
@@ -124,7 +125,7 @@ function resolveRoot(root, startCwd, isExecroot, runfiles) {
         if (isExecroot) {
             return root ? `${startCwd}/external/${root}` : `${startCwd}/node_modules`;
         }
-        const match = startCwd.match(/(\/bazel-out\/|\/bazel-~1\/x64_wi~1\/)/);
+        const match = startCwd.match(BAZEL_OUT_REGEX);
         if (!match) {
             if (!root) {
                 return `${startCwd}/node_modules`;
@@ -176,14 +177,22 @@ class Runfiles {
     lookupDirectory(dir) {
         if (!this.manifest)
             return undefined;
+        let result;
         for (const [k, v] of this.manifest) {
             if (k.startsWith(`${dir}/external`))
                 continue;
             if (k.startsWith(dir)) {
                 const l = k.length - dir.length;
-                return v.substring(0, v.length - l);
+                const maybe = v.substring(0, v.length - l);
+                if (maybe.match(BAZEL_OUT_REGEX)) {
+                    return maybe;
+                }
+                else {
+                    result = maybe;
+                }
             }
         }
+        return result;
     }
     loadRunfilesManifest(manifestPath) {
         log_verbose(`using runfiles manifest ${manifestPath}`);
@@ -470,6 +479,13 @@ function main(args, runfiles) {
                             }
                             try {
                                 target = runfiles.resolve(runfilesPath);
+                                if (runfiles.manifest && root == 'execroot' && modulePath.startsWith(`${bin}/`)) {
+                                    if (!target.includes(`/${bin}/`)) {
+                                        const e = new Error(`could not resolve modulePath ${modulePath}`);
+                                        e.code = 'MODULE_NOT_FOUND';
+                                        throw e;
+                                    }
+                                }
                             }
                             catch (_a) {
                                 target = '<runfiles resolution failed>';

internal/linker/link_node_modules.ts

@@ -9,6 +9,10 @@ import * as path from 'path';
 // Run Bazel with --define=VERBOSE_LOGS=1 to enable this logging
 const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS'];
 
+// NB: on windows thanks to legacy 8-character path segments it might be like
+// c:/b/ojvxx6nx/execroot/build_~1/bazel-~1/x64_wi~1/bin/internal/npm_in~1/test
+const BAZEL_OUT_REGEX = /(\/bazel-out\/|\/bazel-~1\/x64_wi~1\/)/;
+
 function log_verbose(...m: string[]) {
   if (VERBOSE_LOGS) console.error('[link_node_modules.js]', ...m);
 }
@@ -175,9 +179,7 @@ async function resolveRoot(
   // runfiles on rbe, bazel runs the process in a directory such as
   // `/b/f/w/bazel-out/k8-fastbuild/bin/path/to/pkg/some_test.sh.runfiles/my_wksp`. From here we can
   // determine the execroot `b/f/w` by finding the first instance of bazel-out.
-  // NB: on windows thanks to legacy 8-character path segments it might be like
-  // c:/b/ojvxx6nx/execroot/build_~1/bazel-~1/x64_wi~1/bin/internal/npm_in~1/test
-  const match = startCwd.match(/(\/bazel-out\/|\/bazel-~1\/x64_wi~1\/)/);
+  const match = startCwd.match(BAZEL_OUT_REGEX);
   if (!match) {
     // No execroot found. This can happen if we are inside a nodejs_image or a nodejs_binary is
     // run manually.
@@ -278,6 +280,7 @@ export class Runfiles {
   lookupDirectory(dir: string): string|undefined {
     if (!this.manifest) return undefined;
 
+    let result: string|undefined;
     for (const [k, v] of this.manifest) {
       // Account for Bazel --legacy_external_runfiles
       // which pollutes the workspace with 'my_wksp/external/...'
@@ -289,9 +292,15 @@ export class Runfiles {
       // calculate l = length(`/semver/LICENSE`)
       if (k.startsWith(dir)) {
         const l = k.length - dir.length;
-        return v.substring(0, v.length - l);
+        const maybe = v.substring(0, v.length - l);
+        if (maybe.match(BAZEL_OUT_REGEX)) {
+          return maybe;
+        } else {
+          result = maybe;
+        }
       }
     }
+    return result;
   }
 
 
@@ -731,6 +740,15 @@ export async function main(args: string[], runfiles: Runfiles) {
           }
           try {
             target = runfiles.resolve(runfilesPath);
+            // if we're resolving from a manifest then make sure we don't resolve
+            // into the source tree when we are expecting the output tree
+            if (runfiles.manifest && root == 'execroot' && modulePath.startsWith(`${bin}/`)) {
+              if (!target.includes(`/${bin}/`)) {
+                const e = new Error(`could not resolve modulePath ${modulePath}`);
+                (e as any).code = 'MODULE_NOT_FOUND';
+                throw e;
+              }
+            }
           } catch {
             target = '<runfiles resolution failed>';
           }

internal/linker/test/workspace_link/BUILD.bazel

@@ -4,7 +4,6 @@ jasmine_node_test(
     name = "test",
     srcs = ["test.js"],
     link_workspace_root = True,
-    tags = ["fix-windows"],
     templated_args = ["--nobazel_patch_module_resolver"],
     deps = [
         "//internal/linker/test/workspace_link/bar",