feat(builtin): add env attribute to nodejs test and binary and run_node helper (#2499)

Author: mattem

internal/node/node.bzl

@@ -174,6 +174,10 @@ def _nodejs_binary_impl(ctx):
     # runfiles helpers to use.
     env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
 
+    # Add all env vars from the ctx attr
+    for [key, value] in ctx.attr.env.items():
+        env_vars += "export %s=%s\n" % (key, expand_location_into_runfiles(ctx, value, ctx.attr.data))
+
     # While we can derive the workspace from the pwd when running locally
     # because it is in the execroot path `execroot/my_wksp`, on RBE the
     # `execroot/my_wksp` path is reduced a path such as `/w/f/b` so
@@ -448,6 +452,12 @@ nodejs_binary(
         mandatory = True,
         allow_single_file = True,
     ),
+    "env": attr.string_dict(
+        doc = """Specifies additional environment variables to set when the target is executed, subject to location
+expansion.
+        """,
+        default = {},
+    ),
     "link_workspace_root": attr.bool(
         doc = """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.""",

internal/node/npm_package_bin.bzl

@@ -11,6 +11,7 @@ _ATTRS = {
     "chdir": attr.string(),
     "configuration_env_vars": attr.string_list(default = []),
     "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]),
+    "env": attr.string_dict(default = {}),
     "exit_code_out": attr.output(),
     "link_workspace_root": attr.bool(),
     "output_dir": attr.bool(),
@@ -80,6 +81,7 @@ def _impl(ctx):
         arguments = [args],
         configuration_env_vars = ctx.attr.configuration_env_vars,
         chdir = expand_variables(ctx, ctx.attr.chdir),
+        env = ctx.attr.env,
         stdout = ctx.outputs.stdout,
         stderr = ctx.outputs.stderr,
         exit_code_out = ctx.outputs.exit_code_out,
@@ -93,7 +95,18 @@ _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, chdir = None, **kwargs):
+def npm_package_bin(
+        tool = None,
+        package = None,
+        package_bin = None,
+        data = [],
+        env = {},
+        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.
@@ -192,6 +205,7 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
                 args = ["/".join([".."] * _package_segments + ["$@"])],
             )
             ```
+        env: specifies additional environment variables to set when the target is executed
         **kwargs: additional undocumented keyword args
     """
     if not tool:
@@ -205,6 +219,7 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
         outs = outs,
         args = args,
         chdir = chdir,
+        env = env,
         output_dir = output_dir,
         tool = tool,
         link_workspace_root = link_workspace_root,

internal/node/test/BUILD.bazel

@@ -76,6 +76,7 @@ jasmine_node_test(
     data = [
         ":dump_build_env.json",
         ":dump_build_env_alt.json",
+        ":dump_build_env_attr_json",
     ],
 )
 
@@ -384,6 +385,15 @@ jasmine_node_test(
     ],
 )
 
+nodejs_binary(
+    name = "dump_build_env_attr",
+    data = ["dump_build_env.js"],
+    entry_point = "dump_build_env.js",
+    env = {
+        "LOC": "$(location :dump_build_env.js)",
+    },
+)
+
 nodejs_binary(
     name = "dump_build_env",
     entry_point = "dump_build_env.js",
@@ -409,6 +419,16 @@ npm_package_bin(
     tool = ":dump_build_env_alt",
 )
 
+npm_package_bin(
+    name = "dump_build_env_attr_json",
+    outs = ["dump_build_env_attr.json"],
+    args = ["$@"],
+    env = {
+        "FOO": "BAR",
+    },
+    tool = ":dump_build_env_attr",
+)
+
 nodejs_binary(
     name = "test_runfiles_helper",
     data = [":test_runfiles_helper.golden"],

internal/node/test/env.spec.js

@@ -96,4 +96,10 @@ describe('launcher.sh environment', function() {
        ]
        expectPathsToMatch(env['BAZEL_PATCH_ROOTS'].split(','), expectedRoots);
      });
+
+  it('should setup correct bazel environment variables from env attr', function() {
+    const env = require(runfiles.resolvePackageRelative('dump_build_env_attr.json'));
+    expect(env['FOO']).toBe('BAR');
+    expect(env['LOC']).toBe('build_bazel_rules_nodejs/internal/node/test/dump_build_env.js');
+  });
 });

internal/providers/node_runtime_deps_info.bzl

@@ -14,6 +14,7 @@
 
 """Custom provider that mimics the Runfiles, but doesn't incur the expense of creating the runfiles symlink tree"""
 
+load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfiles")
 load("//internal/linker:link_node_modules.bzl", "add_arg", "write_node_modules_manifest")
 load("//internal/providers:external_npm_package_info.bzl", "ExternalNpmPackageInfo")
 
@@ -115,7 +116,10 @@ def run_node(ctx, inputs, arguments, executable, chdir = None, **kwargs):
     if chdir:
         add_arg(arguments, "--bazel_node_working_dir=" + chdir)
 
-    env = kwargs.pop("env", {})
+    env = dict({}, **kwargs.pop("env", {}))
+    if hasattr(ctx.attr, "data"):
+        for [key, value] in env.items():
+            env[key] = expand_location_into_runfiles(ctx, value, ctx.attr.data)
 
     # Always forward the COMPILATION_MODE to node process as an environment variable
     configuration_env_vars = kwargs.pop("configuration_env_vars", []) + ["COMPILATION_MODE"]