From 801935d1c211b75374e6cfcd613e7c786cfec885 Mon Sep 17 00:00:00 2001
From: Greg Magolan <gmagolan@gmail.com>
Date: Thu, 16 Apr 2020 16:56:10 -0700
Subject: [PATCH] fix(builtin): fix linker issue when running test with "local"
 tag on osx & linux (#1835)

---
 internal/linker/index.js                     |  4 +++
 internal/linker/link_node_modules.ts         | 13 +++++++--
 internal/linker/test/local/BUILD.bazel       |  9 ++++++
 internal/linker/test/local/fit/BUILD.bazel   | 29 ++++++++++++++++++++
 internal/linker/test/local/fit/main.ts       |  1 +
 internal/linker/test/local/fit/package.json  |  5 ++++
 internal/linker/test/local/fit/tsconfig.json |  5 ++++
 internal/linker/test/local/test.js           |  6 ++++
 8 files changed, 70 insertions(+), 2 deletions(-)
 create mode 100644 internal/linker/test/local/BUILD.bazel
 create mode 100644 internal/linker/test/local/fit/BUILD.bazel
 create mode 100644 internal/linker/test/local/fit/main.ts
 create mode 100644 internal/linker/test/local/fit/package.json
 create mode 100644 internal/linker/test/local/fit/tsconfig.json
 create mode 100644 internal/linker/test/local/test.js

diff --git a/internal/linker/index.js b/internal/linker/index.js
index e130315ad0..b77064389c 100644
--- a/internal/linker/index.js
+++ b/internal/linker/index.js
@@ -89,6 +89,10 @@ function resolveRoot(root, startCwd, isExecroot, runfiles) {
             return fromManifest;
         }
         else {
+            const maybe = path.resolve(`${symlinkRoot}/external/${root}`);
+            if (fs.existsSync(maybe)) {
+                return maybe;
+            }
             return path.resolve(`${startCwd}/../${root}`);
         }
     });
diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts
index 7929b2561b..093c65dc62 100644
--- a/internal/linker/link_node_modules.ts
+++ b/internal/linker/link_node_modules.ts
@@ -142,8 +142,17 @@ async function resolveRoot(
   if (fromManifest) {
     return fromManifest;
   } else {
-    // Under runfiles, the root will be one folder up from the startCwd `runfiles/my_wksp`.
-    // This is true whether legacy external runfiles are on or off.
+    const maybe = path.resolve(`${symlinkRoot}/external/${root}`);
+    if (fs.existsSync(maybe)) {
+      // Under runfiles, when not in the sandbox we must symlink node_modules down at the execroot
+      // `execroot/my_wksp/external/npm/node_modules` since `runfiles/npm/node_modules` will be a
+      // directory and not a symlink back to the root node_modules where we expect
+      // to resolve from. This case is tested in internal/linker/test/local.
+      return maybe;
+    }
+    // However, when in the sandbox, `execroot/my_wksp/external/npm/node_modules` does not exist,
+    // so we must symlink into `runfiles/npm/node_modules`. This directory exists whether legacy
+    // external runfiles are on or off.
     return path.resolve(`${startCwd}/../${root}`)
   }
 }
diff --git a/internal/linker/test/local/BUILD.bazel b/internal/linker/test/local/BUILD.bazel
new file mode 100644
index 0000000000..0ebecfed71
--- /dev/null
+++ b/internal/linker/test/local/BUILD.bazel
@@ -0,0 +1,9 @@
+load("@npm_bazel_jasmine//:index.from_src.bzl", "jasmine_node_test")
+
+jasmine_node_test(
+    name = "test",
+    srcs = ["test.js"],
+    tags = ["local"],
+    templated_args = ["--nobazel_patch_module_resolver"],
+    deps = ["//internal/linker/test/local/fit"],
+)
diff --git a/internal/linker/test/local/fit/BUILD.bazel b/internal/linker/test/local/fit/BUILD.bazel
new file mode 100644
index 0000000000..a7a7ee799a
--- /dev/null
+++ b/internal/linker/test/local/fit/BUILD.bazel
@@ -0,0 +1,29 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "pkg_npm")
+load("@npm//typescript:index.bzl", "tsc")
+
+tsc(
+    name = "fit_lib",
+    outs = [
+        "main.d.ts",
+        "main.js",
+    ],
+    args = [
+        "-p",
+        "$(execpath tsconfig.json)",
+        "--outDir",
+        # $(RULEDIR) is a shorthand for the dist/bin directory where Bazel requires we write outputs
+        "$(RULEDIR)",
+    ],
+    data = [
+        "main.ts",
+        "tsconfig.json",
+    ],
+)
+
+pkg_npm(
+    name = "fit",
+    package_name = "fit",
+    srcs = ["package.json"],
+    visibility = ["//internal/linker/test/local:__pkg__"],
+    deps = [":fit_lib"],
+)
diff --git a/internal/linker/test/local/fit/main.ts b/internal/linker/test/local/fit/main.ts
new file mode 100644
index 0000000000..e7dc44f51a
--- /dev/null
+++ b/internal/linker/test/local/fit/main.ts
@@ -0,0 +1 @@
+export const fit: string = 'fit';
diff --git a/internal/linker/test/local/fit/package.json b/internal/linker/test/local/fit/package.json
new file mode 100644
index 0000000000..1cc0e5c17d
--- /dev/null
+++ b/internal/linker/test/local/fit/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "fit",
+  "main": "main.js",
+  "typings": "main.d.ts"
+}
diff --git a/internal/linker/test/local/fit/tsconfig.json b/internal/linker/test/local/fit/tsconfig.json
new file mode 100644
index 0000000000..8f1cc9cec2
--- /dev/null
+++ b/internal/linker/test/local/fit/tsconfig.json
@@ -0,0 +1,5 @@
+{
+  "compilerOptions": {
+    "declaration": true
+  }
+}
\ No newline at end of file
diff --git a/internal/linker/test/local/test.js b/internal/linker/test/local/test.js
new file mode 100644
index 0000000000..481989f023
--- /dev/null
+++ b/internal/linker/test/local/test.js
@@ -0,0 +1,6 @@
+describe('linker', () => {
+  it('should work when job run with "local" tag', () => {
+    const fit = require('fit');
+    expect(fit.fit).toBe('fit');
+  });
+});