feat(builtin): add a chdir attribute to nodejs_test and npm_package_bin

Fixes #2323

Author: Alex Eagle

internal/node/launcher.sh

@@ -176,6 +176,7 @@ ALL_ARGS=(TEMPLATED_args "$@")
 STDOUT_CAPTURE=""
 STDERR_CAPTURE=""
 EXIT_CODE_CAPTURE=""
+NODE_WORKING_DIR=""
 
 RUN_LINKER=true
 NODE_PATCHES=true
@@ -205,6 +206,7 @@ for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
     --nobazel_run_linker) RUN_LINKER=false PATCH_REQUIRE=true ;;
     # If running an NPM package, run it from execroot instead of from external
     --bazel_run_from_execroot) FROM_EXECROOT=true ;;
+    --bazel_node_working_dir=*) NODE_WORKING_DIR="${ARG#--bazel_node_working_dir=}" ;;
     # Let users pass through arguments to node itself
     --node_options=*) USER_NODE_OPTIONS+=( "${ARG#--node_options=}" ) ;;
     # Remaining argv is collected to pass to the program
@@ -302,6 +304,11 @@ else
   fi
 fi
 
+if [[ -n "$NODE_WORKING_DIR" ]]; then
+  echo "process.chdir(__dirname)" > "$NODE_WORKING_DIR/__chdir.js"
+  LAUNCHER_NODE_OPTIONS+=( "--require" "./$NODE_WORKING_DIR/__chdir.js" )
+fi
+
 # The EXPECTED_EXIT_CODE lets us write bazel tests which assert that
 # a binary fails to run. Otherwise any failure would make such a test
 # fail before we could assert that we expected that failure.

internal/node/node.bzl

@@ -228,11 +228,15 @@ fi
     # legacy uses of "$(rlocation"
     expanded_args = [preserve_legacy_templated_args(a) for a in ctx.attr.templated_args]
 
-    # First expand predefined source/output path variables:
+    # Add the working dir argument before expansions
+    if ctx.attr.chdir:
+        expanded_args.append("--bazel_node_working_dir=" + ctx.attr.chdir)
+
+    # Next expand predefined source/output path variables:
     # $(execpath), $(rootpath) & legacy $(location)
     expanded_args = [expand_location_into_runfiles(ctx, a, ctx.attr.data) for a in expanded_args]
 
-    # Next expand predefined variables & custom variables
+    # Finally expand predefined variables & custom variables
     rule_dir = _join(ctx.bin_dir.path, ctx.label.workspace_root, ctx.label.package)
     additional_substitutions = {
         "@D": rule_dir,
@@ -324,6 +328,18 @@ fi
     ]
 
 _NODEJS_EXECUTABLE_ATTRS = {
+    "chdir": attr.string(
+        doc = """Working directory to run the binary or test in, relative to the workspace.
+        By default, Bazel always runs in the workspace root.
+
+        To run in the directory containing the `nodejs_binary` / `nodejs_test` use
+        `chdir = package_name()`
+        (or if you're in a macro, use `native.package_name()`)
+        
+        NOTE that this can affect other paths passed to the program, which are workspace-relative.
+        You may need `../../` segments to re-relativize such paths to the new working directory.
+        """,
+    ),
     "configuration_env_vars": attr.string_list(
         doc = """Pass these configuration environment variables to the resulting binary.
         Chooses a subset of the configuration environment variables (taken from `ctx.var`), which also
@@ -589,11 +605,8 @@ If you just want to run a standard test using a test runner from npm, use the ge
 *_test target created by npm_install/yarn_install, such as `mocha_test`.
 Some test runners like Karma and Jasmine have custom rules with added features, e.g. `jasmine_node_test`.
 
-Bazel always runs tests with a working directory set to your workspace root.
-If your test needs to run in a different directory, you can write a `process.chdir` helper script
-and invoke it before the test with a `--require` argument, like
-`templated_args = ["--node_options=--require=./$(rootpath chdir.js)"]`.
-See rules_nodejs/internal/node/test/chdir for an example.
+By default, Bazel runs tests with a working directory set to your workspace root.
+Use the `chdir` attribute to change the working directory before the program starts.
 
 To debug a Node.js test, we recommend saving a group of flags together in a "config".
 Put this in your `tools/bazel.rc` so it's shared with your team:

internal/node/npm_package_bin.bzl

@@ -8,6 +8,7 @@ load("//internal/linker:link_node_modules.bzl", "module_mappings_aspect")
 # so that we can generate macros that act as either an output-producing tool or an executable
 _ATTRS = {
     "args": attr.string_list(mandatory = True),
+    "chdir": attr.string(),
     "configuration_env_vars": attr.string_list(default = []),
     "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]),
     "exit_code_out": attr.output(),
@@ -78,6 +79,7 @@ def _impl(ctx):
         outputs = outputs,
         arguments = [args],
         configuration_env_vars = ctx.attr.configuration_env_vars,
+        chdir = expand_variables(ctx, ctx.attr.chdir),
         stdout = ctx.outputs.stdout,
         stderr = ctx.outputs.stderr,
         exit_code_out = ctx.outputs.exit_code_out,
@@ -91,19 +93,16 @@ _npm_package_bin = rule(
     attrs = _ATTRS,
 )
 
-def npm_package_bin(tool = None, package = None, package_bin = None, data = [], outs = [], args = [], output_dir = False, link_workspace_root = False, **kwargs):
+def npm_package_bin(tool = None, package = None, package_bin = None, data = [], outs = [], args = [], output_dir = False, link_workspace_root = False, chdir = None, **kwargs):
     """Run an arbitrary npm package binary (e.g. a program under node_modules/.bin/*) under Bazel.
 
     It must produce outputs. If you just want to run a program with `bazel run`, use the nodejs_binary rule.
 
     This is like a genrule() except that it runs our launcher script that first
     links the node_modules tree before running the program.
 
-    Bazel always runs actions with a working directory set to your workspace root.
-    If your tool needs to run in a different directory, you can write a `process.chdir` helper script
-    and invoke it before the action with a `--require` argument, like
-    `args = ["--node_options=--require=./$(execpath chdir.js)"]`
-    See rules_nodejs/internal/node/test/chdir for an example.
+    By default, Bazel runs actions with a working directory set to your workspace root.
+    Use the `chdir` attribute to change the working directory before the program runs.
 
     This is a great candidate to wrap with a macro, as documented:
     https://docs.bazel.build/versions/master/skylark/macros.html#full-example
@@ -168,6 +167,30 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
               Note that you can also refer to a binary in your local workspace.
         link_workspace_root: Link the workspace root to the bin_dir to support absolute requires like 'my_wksp/path/to/file'.
               If source files need to be required then they can be copied to the bin_dir with copy_to_bin.
+        chdir: Working directory to run the binary or test in, relative to the workspace.
+
+            By default, Bazel always runs in the workspace root.
+
+            To run in the directory containing the `npm_package_bin` under the source tree, use
+            `chdir = package_name()`
+            (or if you're in a macro, use `native.package_name()`).
+
+            To run in the output directory where the npm_package_bin writes outputs, use
+            `chdir = "$(RULEDIR)"`
+
+            NOTE that this can affect other paths passed to the program, which are workspace-relative.
+            You may need `../../` segments to re-relativize such paths to the new working directory.
+            In a BUILD file you could do something like this to point to the output path:
+
+            ```python
+            _package_segments = len(package_name().split("/"))
+            npm_package_bin(
+                ...
+                chdir = package_name(),
+                args = ["/".join([".."] * _package_segments + ["$@"])],
+            )
+            ```
+        **kwargs: additional undocumented keyword args
     """
     if not tool:
         if not package:
@@ -179,6 +202,7 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
         data = data,
         outs = outs,
         args = args,
+        chdir = chdir,
         output_dir = output_dir,
         tool = tool,
         link_workspace_root = link_workspace_root,

internal/node/test/chdir/BUILD.bazel

@@ -3,26 +3,29 @@
 # the project when you call them.
 # See https://github.com/bazelbuild/rules_nodejs/issues/1840
 
-load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin", "nodejs_binary", "nodejs_test", "npm_package_bin")
-load("//third_party/github.com/bazelbuild/bazel-skylib:rules/write_file.bzl", "write_file")
+load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test", "npm_package_bin")
+
+# Trivial tool that expects to run in source directory under the package
+nodejs_binary(
+    name = "tool_cp",
+    entry_point = "cp.js",
+)
 
 # A tool like react-scripts needs to run in the output directory since it writes outputs
 # to $pwd/build
 # That means it also needs to find inputs in that directory.
 # So we copy all the inputs it needs.
-copy_to_bin(
-    name = "copy_input",
-    srcs = ["package.json"],
-)
+_package_segments = len(package_name().split("/"))
 
-# Here's our trick: write a process.chdir one-liner JS script
-write_file(
-    name = "write_chdir_script",
-    out = "chdir.js",
-    # __dirname will be whatever directory the other outputs
-    # from this package are in, either in execroot or runfiles root
-    # depending on where Bazel places this file.
-    content = ["process.chdir(__dirname)"],
+npm_package_bin(
+    name = "do_copy",
+    outs = ["package.json"],
+    # We have to compensate for the changed directory, adapting other arguments
+    # to reach back to parent directory
+    args = ["/".join([".."] * _package_segments + ["$@"])],
+    chdir = package_name(),
+    data = ["_package.json"],
+    tool = ":tool_cp",
 )
 
 # Trivial tool to mock react-scripts
@@ -36,27 +39,15 @@ nodejs_binary(
 npm_package_bin(
     name = "call_tool",
     outs = ["build/app.js"],
-    # This tool produces outputs, so it is a build action and needs execpath helper
-    args = ["--node_options=--require=./$(execpath chdir.js)"],
-    # We can't reference label "package.json" here because it's ambiguous
-    # whether that points to the InputArtifact package.json in the src
-    # folder or the output. Using "copy_input" is unambiguous.
-    data = [
-        "chdir.js",
-        "copy_input",
-    ],
+    chdir = "$(RULEDIR)",
+    data = ["package.json"],
     tool = ":tool_bin",
 )
 
 nodejs_test(
     name = "test",
-    data = [
-        "build/app.js",
-        "chdir.js",
-    ],
+    # Also run a test in the output directory
+    chdir = package_name(),
+    data = ["build/app.js"],
     entry_point = "test.js",
-    # Also run a test in the output directory, this needs rootpath helper
-    # NB: --require=./$(rootpath chdir.js) works when runfiles are symlinked
-    # but $$(rlocation) is needed for Windows.
-    templated_args = ["--node_options=--require=$$(rlocation $(rootpath chdir.js))"],
 )

internal/node/test/chdir/package.json internal/node/test/chdir/_package.jsoninternal/node/test/chdir/package.json renamed to internal/node/test/chdir/_package.json

@@ -0,0 +1,14 @@
+// assumes the working directory contains _package.json
+const fs = require('fs');
+const {dirname} = require('path');
+const dest = process.argv[2];
+
+function mkdirp(p) {
+  if (!fs.existsSync(p)) {
+    mkdirp(dirname(p));
+    fs.mkdirSync(p);
+  }
+}
+
+mkdirp(dirname(dest));
+fs.copyFileSync('_package.json', dest);

internal/node/test/chdir/cp.js

@@ -3,4 +3,4 @@ const assert = require('assert');
 
 // this test should be run in a working directory with
 // that file in it
-assert.equal('console.log("hello world")', readFileSync('build/app.js', 'utf-8'));
+assert.strictEqual('console.log("hello world")', readFileSync('build/app.js', 'utf-8'));

internal/node/test/chdir/test.js

@@ -56,7 +56,7 @@ def _compute_node_modules_root(ctx):
                 fail("All npm dependencies need to come from a single workspace. Found '%s' and '%s'." % (node_modules_root, possible_root))
     return node_modules_root
 
-def run_node(ctx, inputs, arguments, executable, **kwargs):
+def run_node(ctx, inputs, arguments, executable, chdir = None, **kwargs):
     """Helper to replace ctx.actions.run
 
     This calls node programs with a node_modules directory in place
@@ -66,10 +66,8 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
         inputs: list or depset of inputs to the action
         arguments: list or ctx.actions.Args object containing arguments to pass to the executable
         executable: stringy representation of the executable this action will run, eg eg. "my_executable" rather than ctx.executable.my_executable
-        mnemonic: optional action mnemonic, used to differentiate module mapping files from the same rule context
-        link_workspace_root: Link the workspace root to the bin_dir to support absolute requires like 'my_wksp/path/to/file'.
-            If source files need to be required then they can be copied to the bin_dir with copy_to_bin.
-        kwargs: all other args accepted by ctx.actions.run
+        chdir: directory we should change to be the working dir
+        **kwargs: all other args accepted by ctx.actions.run
     """
     if (type(executable) != "string"):
         fail("""run_node requires that executable be provided as a string,
@@ -112,6 +110,9 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
         add_arg(arguments, "--bazel_capture_exit_code=%s" % exit_code_file.path)
         outputs = outputs + [exit_code_file]
 
+    if chdir:
+        add_arg(arguments, "--bazel_node_working_dir=" + chdir)
+
     env = kwargs.pop("env", {})
 
     # Always forward the COMPILATION_MODE to node process as an environment variable