fix: relative data paths in yarn_install & npm_install when symlink_n…

…ode_modules=False and package.json is not at root

This fixes `yarn_install` & `npm_install` to copy `package.json` & lock files into the external repository to a folder that corresponds to the package.json's workspace folder and the package manager is then run out that location so that relative paths to `data` files are preserved (same as they would be outside of bazel) if the `package.json` is _not_ at the root of the WORKSPACE.

If you had a nested `package.json` and were using `yarn_install` or `npm_install` with `symlink_node_modules = False,` and passing in `data` that you were referencing during install, then you would have had to use the work-around of absolute workspace paths to the `data` files. This fix will allow you to use the same relative paths that you would use if you were running `yarn` or `npm` outside of bazel in this case.

For example, if the package.json file is located at `my/nested/package.json` then
it will end up at `_/my/nested/package.json` in the external repository.

When `symlink_node_modules` is `False`, `data` files are copied into the same tree, so that relative paths in
`package.json` that refer to `data` files are preserved.

A `"postinstall": "patch-package --patch-dir patches"` in a nested `package.json` file
expects a `patches` directory relative to the `package.json` file will now work with
`symlink_node_modules = False`.

```
yarn_install(
    name = "my_nested_npm_deps",
    package_json = "//my/nested:package.json",
    yarn_lock = "//my/nested:yarn.lock",
    data = ["//my/nested:patches/jest-haste-map+24.9.0.patch"],
    symlink_node_modules = False,
)
```

Additional fix is that `data` files are now copied to external repository with `mkdir -p && cp -f` instead of `rtcx.template({})` trick. The latter is slower & does not copy over binary files correctly. We must copy `data` files and _not_ symlink them since a `package.json` file with `file:path/to/data` will fail if `path/to/data` is a symlink.

e2e/packages/npm1/package.json

@@ -8,6 +8,6 @@
     "tmp": "0.1.0"
   },
   "scripts": {
-    "postinstall": "node ./postinstall.js"
+    "postinstall": "node ../postinstall.js"
   }
 }

e2e/packages/npm2/package.json

@@ -8,6 +8,6 @@
     "tmp": "0.1.0"
   },
   "scripts": {
-    "postinstall": "node ./postinstall.js"
+    "postinstall": "node ../postinstall.js"
   }
 }

e2e/packages/yarn1/package.json

@@ -8,6 +8,6 @@
     "tmp": "0.1.0"
   },
   "scripts": {
-    "postinstall": "node ./postinstall.js"
+    "postinstall": "node ../postinstall.js"
   }
 }

e2e/packages/yarn2/package.json

@@ -8,6 +8,6 @@
     "tmp": "0.1.0"
   },
   "scripts": {
-    "postinstall": "node ./postinstall.js"
+    "postinstall": "node ../postinstall.js"
   }
 }

internal/npm_install/generate_build_file.ts

@@ -53,9 +53,11 @@ const WORKSPACE = args[0];
 const RULE_TYPE = args[1];
 const PKG_JSON_FILE_PATH = args[2];
 const LOCK_FILE_PATH = args[3];
-const STRICT_VISIBILITY = args[4]?.toLowerCase() === 'true';
-const INCLUDED_FILES = args[5] ? args[5].split(',') : [];
-const BAZEL_VERSION = args[6];
+const WORKSPACE_ROOT_PREFIX = args[4];
+const WORKSPACE_ROOT_BASE = WORKSPACE_ROOT_PREFIX ?.split('/')[0];
+const STRICT_VISIBILITY = args[5]?.toLowerCase() === 'true';
+const INCLUDED_FILES = args[6] ? args[6].split(',') : [];
+const BAZEL_VERSION = args[7];
 
 const PUBLIC_VISIBILITY = '//visibility:public';
 const LIMITED_VISIBILITY = `@${WORKSPACE}//:__subpackages__`;
@@ -122,7 +124,7 @@ export function main() {
   generateBuildFiles(pkgs)
 
   // write a .bazelignore file
-  writeFileSync('.bazelignore', 'node_modules');
+  writeFileSync('.bazelignore', `node_modules\n${WORKSPACE_ROOT_BASE}`);
 }
 
 /**
@@ -171,8 +173,7 @@ function generateRootBuildFile(pkgs: Dep[]) {
 
   let exportsStarlark = '';
   pkgs.forEach(pkg => {pkg._files.forEach(f => {
-                 exportsStarlark += `    "node_modules/${pkg._dir}/${f}",
-`;
+                 exportsStarlark += `    "node_modules/${pkg._dir}/${f}",\n`;
                })});
 
   let buildFile =
@@ -556,7 +557,8 @@ function findScopes() {
   const scopes = listing.filter(f => f.startsWith('@'))
                      .map(f => path.posix.join(p, f))
                      .filter(f => isDirectory(f))
-                     .map(f => f.replace(/^node_modules\//, ''));
+                     // strip 'node_modules/' from filename
+                     .map(f => f.substring('node_modules/'.length));
 
   return scopes;
 }
@@ -575,7 +577,7 @@ export function parsePackage(p: string, dependencies: Set<string> = new Set()):
 
   // Trim the leading node_modules from the path and
   // assign to _dir for future use
-  pkg._dir = p.replace(/^node_modules\//, '');
+  pkg._dir = p.substring('node_modules/'.length)
 
   // Stash the package directory name for future use
   pkg._name = pkg._dir.split('/').pop();
@@ -585,7 +587,7 @@ export function parsePackage(p: string, dependencies: Set<string> = new Set()):
   pkg._moduleName = pkg.name || `${pkg._dir}/${pkg._name}`;
 
   // Keep track of whether or not this is a nested package
-  pkg._isNested = /\/node_modules\//.test(p);
+  pkg._isNested = /\/node_modules\//.test(pkg._dir);
 
   // List all the files in the npm package for later use
   pkg._files = listFiles(p);

internal/npm_install/index.js

@@ -1,5 +1,5 @@
 /* THIS FILE GENERATED FROM .ts; see BUILD.bazel */ /* clang-format off */'use strict';
-var _a;
+var _a, _b;
 Object.defineProperty(exports, "__esModule", { value: true });
 const fs = require("fs");
 const path = require("path");
@@ -13,9 +13,11 @@ const WORKSPACE = args[0];
 const RULE_TYPE = args[1];
 const PKG_JSON_FILE_PATH = args[2];
 const LOCK_FILE_PATH = args[3];
-const STRICT_VISIBILITY = ((_a = args[4]) === null || _a === void 0 ? void 0 : _a.toLowerCase()) === 'true';
-const INCLUDED_FILES = args[5] ? args[5].split(',') : [];
-const BAZEL_VERSION = args[6];
+const WORKSPACE_ROOT_PREFIX = args[4];
+const WORKSPACE_ROOT_BASE = (_a = WORKSPACE_ROOT_PREFIX) === null || _a === void 0 ? void 0 : _a.split('/')[0];
+const STRICT_VISIBILITY = ((_b = args[5]) === null || _b === void 0 ? void 0 : _b.toLowerCase()) === 'true';
+const INCLUDED_FILES = args[6] ? args[6].split(',') : [];
+const BAZEL_VERSION = args[7];
 const PUBLIC_VISIBILITY = '//visibility:public';
 const LIMITED_VISIBILITY = `@${WORKSPACE}//:__subpackages__`;
 function generateBuildFileHeader(visibility = PUBLIC_VISIBILITY) {
@@ -49,7 +51,7 @@ function main() {
     flattenDependencies(pkgs);
     generateBazelWorkspaces(pkgs);
     generateBuildFiles(pkgs);
-    writeFileSync('.bazelignore', 'node_modules');
+    writeFileSync('.bazelignore', `node_modules\n${WORKSPACE_ROOT_BASE}`);
 }
 exports.main = main;
 function generateBuildFiles(pkgs) {
@@ -86,8 +88,7 @@ function generateRootBuildFile(pkgs) {
     let exportsStarlark = '';
     pkgs.forEach(pkg => {
         pkg._files.forEach(f => {
-            exportsStarlark += `    "node_modules/${pkg._dir}/${f}",
-`;
+            exportsStarlark += `    "node_modules/${pkg._dir}/${f}",\n`;
         });
     });
     let buildFile = generateBuildFileHeader() + `load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
@@ -327,18 +328,18 @@ function findScopes() {
     const scopes = listing.filter(f => f.startsWith('@'))
         .map(f => path.posix.join(p, f))
         .filter(f => isDirectory(f))
-        .map(f => f.replace(/^node_modules\//, ''));
+        .map(f => f.substring('node_modules/'.length));
     return scopes;
 }
 function parsePackage(p, dependencies = new Set()) {
     const packageJson = path.posix.join(p, 'package.json');
     const pkg = isFile(packageJson) ?
         JSON.parse(stripBom(fs.readFileSync(packageJson, { encoding: 'utf8' }))) :
         { version: '0.0.0' };
-    pkg._dir = p.replace(/^node_modules\//, '');
+    pkg._dir = p.substring('node_modules/'.length);
     pkg._name = pkg._dir.split('/').pop();
     pkg._moduleName = pkg.name || `${pkg._dir}/${pkg._name}`;
-    pkg._isNested = /\/node_modules\//.test(p);
+    pkg._isNested = /\/node_modules\//.test(pkg._dir);
     pkg._files = listFiles(p);
     pkg._runfiles = pkg._files.filter((f) => !/[^\x21-\x7E]/.test(f));
     pkg._dependencies = [];

internal/npm_install/npm_install.bzl

@@ -126,6 +126,7 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file):
         rule_type,
         repository_ctx.path(repository_ctx.attr.package_json),
         repository_ctx.path(lock_file),
+        _workspace_root_prefix(repository_ctx),
         str(repository_ctx.attr.strict_visibility),
         ",".join(repository_ctx.attr.included_files),
         native.bazel_version,
@@ -147,24 +148,55 @@ def _add_scripts(repository_ctx):
         {},
     )
 
-def _add_package_json(repository_ctx):
-    repository_ctx.symlink(
-        repository_ctx.attr.package_json,
-        repository_ctx.path("package.json"),
+def _workspace_root_path(repository_ctx, f):
+    segments = ["_"]
+    if f.package:
+        segments.append(f.package)
+    segments.append(f.name)
+    return "/".join(segments)
+
+def _workspace_root_prefix(repository_ctx):
+    package_json = repository_ctx.attr.package_json
+    segments = ["_"]
+    if package_json.package:
+        segments.append(package_json.package)
+    segments.extend(package_json.name.split("/"))
+    segments.pop()
+    return "/".join(segments) + "/"
+
+def _copy_file(repository_ctx, f):
+    to = _workspace_root_path(repository_ctx, f)
+
+    # ensure the destination directory exists
+    to_segments = to.split("/")
+    if len(to_segments) > 1:
+        dirname = "/".join(to_segments[:-1])
+        result = repository_ctx.execute(
+            ["mkdir", "-p", dirname],
+            quiet = repository_ctx.attr.quiet,
+        )
+        if result.return_code:
+            fail("mkdir -p %s failed: \nSTDOUT:\n%s\nSTDERR:\n%s" % (dirname, result.stdout, result.stderr))
+
+    # copy the file; don't use the repository_ctx.template trick with empty substitution as this
+    # does not copy over binary files properly
+    result = repository_ctx.execute(
+        ["cp", "-f", repository_ctx.path(f), to],
+        quiet = repository_ctx.attr.quiet,
     )
+    if result.return_code:
+        fail("cp -f %s %s failed: \nSTDOUT:\n%s\nSTDERR:\n%s" % (repository_ctx.path(f), to, result.stdout, result.stderr))
+
+def _symlink_file(repository_ctx, f):
+    repository_ctx.symlink(f, _workspace_root_path(repository_ctx, f))
 
-def _add_data_dependencies(repository_ctx):
+def _copy_data_dependencies(repository_ctx):
     """Add data dependencies to the repository."""
     for f in repository_ctx.attr.data:
-        to = []
-        if f.package:
-            to.append(f.package)
-        to.append(f.name)
-
         # Make copies of the data files instead of symlinking
         # as yarn under linux will have trouble using symlinked
         # files as npm file:// packages
-        repository_ctx.template("/".join(to), f, {})
+        _copy_file(repository_ctx, f)
 
 def _add_node_repositories_info_deps(repository_ctx):
     # Add a dep to the node_info & yarn_info files from node_repositories
@@ -180,7 +212,16 @@ def _add_node_repositories_info_deps(repository_ctx):
 
 def _symlink_node_modules(repository_ctx):
     package_json_dir = repository_ctx.path(repository_ctx.attr.package_json).dirname
-    repository_ctx.symlink(repository_ctx.path(str(package_json_dir) + "/node_modules"), repository_ctx.path("node_modules"))
+    if repository_ctx.attr.symlink_node_modules:
+        repository_ctx.symlink(
+            repository_ctx.path(str(package_json_dir) + "/node_modules"),
+            repository_ctx.path("node_modules"),
+        )
+    else:
+        repository_ctx.symlink(
+            repository_ctx.path(_workspace_root_prefix(repository_ctx) + "node_modules"),
+            repository_ctx.path("node_modules"),
+        )
 
 def _check_min_bazel_version(rule, repository_ctx):
     if repository_ctx.attr.symlink_node_modules:
@@ -213,13 +254,11 @@ def _npm_install_impl(repository_ctx):
 
     npm_args.extend(repository_ctx.attr.args)
 
-    # If symlink_node_modules is true then run the package manager
-    # in the package.json folder; otherwise, run it in the root of
-    # the external repository
+    # Run the package manager in the package.json folder
     if repository_ctx.attr.symlink_node_modules:
-        root = repository_ctx.path(repository_ctx.attr.package_json).dirname
+        root = str(repository_ctx.path(repository_ctx.attr.package_json).dirname)
     else:
-        root = repository_ctx.path("")
+        root = str(repository_ctx.path(_workspace_root_prefix(repository_ctx)))
 
     # The entry points for npm install for osx/linux and windows
     if not is_windows_host:
@@ -250,12 +289,9 @@ cd /D "{root}" && "{npm}" {npm_args}
             executable = True,
         )
 
-    repository_ctx.symlink(
-        repository_ctx.attr.package_lock_json,
-        repository_ctx.path("package-lock.json"),
-    )
-    _add_package_json(repository_ctx)
-    _add_data_dependencies(repository_ctx)
+    _symlink_file(repository_ctx, repository_ctx.attr.package_lock_json)
+    _copy_file(repository_ctx, repository_ctx.attr.package_json)
+    _copy_data_dependencies(repository_ctx)
     _add_scripts(repository_ctx)
     _add_node_repositories_info_deps(repository_ctx)
 
@@ -288,15 +324,15 @@ cd /D "{root}" && "{npm}" {npm_args}
     # removeNPMAbsolutePaths is run on node_modules after npm install as the package.json files
     # generated by npm are non-deterministic. They contain absolute install paths and other private
     # information fields starting with "_". removeNPMAbsolutePaths removes all fields starting with "_".
+    print([node, repository_ctx.path(remove_npm_absolute_paths), root + "/node_modules"])
     result = repository_ctx.execute(
-        [node, repository_ctx.path(remove_npm_absolute_paths), "/".join([str(root), "node_modules"])],
+        [node, repository_ctx.path(remove_npm_absolute_paths), root + "/node_modules"],
     )
 
     if result.return_code:
         fail("remove_npm_absolute_paths failed: %s (%s)" % (result.stdout, result.stderr))
 
-    if repository_ctx.attr.symlink_node_modules:
-        _symlink_node_modules(repository_ctx)
+    _symlink_node_modules(repository_ctx)
 
     _create_build_files(repository_ctx, "npm_install", node, repository_ctx.attr.package_lock_json)
 
@@ -380,13 +416,11 @@ def _yarn_install_impl(repository_ctx):
         yarn_args.extend(["--mutex", "network"])
     yarn_args.extend(repository_ctx.attr.args)
 
-    # If symlink_node_modules is true then run the package manager
-    # in the package.json folder; otherwise, run it in the root of
-    # the external repository
+    # Run the package manager in the package.json folder
     if repository_ctx.attr.symlink_node_modules:
-        root = repository_ctx.path(repository_ctx.attr.package_json).dirname
+        root = str(repository_ctx.path(repository_ctx.attr.package_json).dirname)
     else:
-        root = repository_ctx.path("")
+        root = str(repository_ctx.path(_workspace_root_prefix(repository_ctx)))
 
     # The entry points for npm install for osx/linux and windows
     if not is_windows_host:
@@ -424,12 +458,9 @@ cd /D "{root}" && "{yarn}" {yarn_args}
             executable = True,
         )
 
-    repository_ctx.symlink(
-        repository_ctx.attr.yarn_lock,
-        repository_ctx.path("yarn.lock"),
-    )
-    _add_package_json(repository_ctx)
-    _add_data_dependencies(repository_ctx)
+    _symlink_file(repository_ctx, repository_ctx.attr.yarn_lock)
+    _copy_file(repository_ctx, repository_ctx.attr.package_json)
+    _copy_data_dependencies(repository_ctx)
     _add_scripts(repository_ctx)
     _add_node_repositories_info_deps(repository_ctx)
 
@@ -456,8 +487,7 @@ cd /D "{root}" && "{yarn}" {yarn_args}
     if result.return_code:
         fail("yarn_install failed: %s (%s)" % (result.stdout, result.stderr))
 
-    if repository_ctx.attr.symlink_node_modules:
-        _symlink_node_modules(repository_ctx)
+    _symlink_node_modules(repository_ctx)
 
     _create_build_files(repository_ctx, "yarn_install", node, repository_ctx.attr.yarn_lock)
 

tools/fine_grained_deps_npm/package.json

@@ -12,10 +12,10 @@
     "chokidar": "2.0.4",
     "http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282",
     "klaw": "1.3.1",
-    "local-module": "file:tools/npm_packages/local_module/npm",
+    "local-module": "file:../../tools/npm_packages/local_module/npm",
     "rxjs": "6.5.0"
   },
   "scripts": {
-    "postinstall": "node internal/npm_install/test/postinstall.js"
+    "postinstall": "node ../../internal/npm_install/test/postinstall.js"
   }
 }

tools/fine_grained_deps_yarn/package.json

@@ -12,10 +12,10 @@
     "chokidar": "2.0.4",
     "http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282",
     "klaw": "1.3.1",
-    "local-module": "link:tools/npm_packages/local_module/yarn",
+    "local-module": "link:../../tools/npm_packages/local_module/yarn",
     "rxjs": "6.5.0"
   },
   "scripts": {
-    "postinstall": "node internal/npm_install/test/postinstall.js"
+    "postinstall": "node ../../internal/npm_install/test/postinstall.js"
   }
 }

tools/fine_grained_deps_yarn/yarn.lock

@@ -751,7 +751,7 @@ [email protected]:
   optionalDependencies:
     graceful-fs "^4.1.9"
 
-"local-module@link:tools/npm_packages/local_module/yarn":
+"local-module@link:../npm_packages/local_module/yarn":
   version "0.0.0"
   uid ""