diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh index 80579e1310..c28ee51c29 100644 --- a/internal/node/launcher.sh +++ b/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. diff --git a/internal/node/node.bzl b/internal/node/node.bzl index 2fd77caec5..09ca897d3d 100644 --- a/internal/node/node.bzl +++ b/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: diff --git a/internal/node/npm_package_bin.bzl b/internal/node/npm_package_bin.bzl index f613885353..2516bf9778 100644 --- a/internal/node/npm_package_bin.bzl +++ b/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,7 +93,7 @@ _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. @@ -99,11 +101,8 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [], 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, diff --git a/internal/node/test/chdir/BUILD.bazel b/internal/node/test/chdir/BUILD.bazel index e4fb5d3dfa..6d47989844 100644 --- a/internal/node/test/chdir/BUILD.bazel +++ b/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))"], ) diff --git a/internal/node/test/chdir/package.json b/internal/node/test/chdir/_package.json similarity index 100% rename from internal/node/test/chdir/package.json rename to internal/node/test/chdir/_package.json diff --git a/internal/node/test/chdir/cp.js b/internal/node/test/chdir/cp.js new file mode 100644 index 0000000000..3e810e4c3a --- /dev/null +++ b/internal/node/test/chdir/cp.js @@ -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); diff --git a/internal/node/test/chdir/test.js b/internal/node/test/chdir/test.js index 5a0f922002..de949b6e6c 100644 --- a/internal/node/test/chdir/test.js +++ b/internal/node/test/chdir/test.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')); diff --git a/internal/providers/node_runtime_deps_info.bzl b/internal/providers/node_runtime_deps_info.bzl index 4d96e745ef..b9ca4ab168 100644 --- a/internal/providers/node_runtime_deps_info.bzl +++ b/internal/providers/node_runtime_deps_info.bzl @@ -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