feat(builtin): perform make variable substitution in npm_package_bin env vars (bazel-contrib#3343)

kormide · alexeagle · commit 87da87467146 · 2022-02-28T10:51:49.000-08:00
diff --git a/docs/Built-ins.md b/docs/Built-ins.md
@@ -2223,7 +2223,8 @@ Defaults to `[]`
 
 <h4 id="npm_package_bin-env">env</h4>
 
-specifies additional environment variables to set when the target is executed
+specifies additional environment variables to set when the target is executed. The values of environment variables
+are subject to 'Make variable' substitution (see [args](#npm_package_bin-args)).
 
 Defaults to `{}`
 
diff --git a/internal/node/npm_package_bin.bzl b/internal/node/npm_package_bin.bzl
@@ -63,6 +63,10 @@ def _impl(ctx):
     for a in ctx.attr.args:
         args.add_all([expand_variables(ctx, e, outs = ctx.outputs.outs, output_dir = ctx.attr.output_dir) for e in _expand_locations(ctx, a)])
 
+    envs = {}
+    for k, v in ctx.attr.env.items():
+        envs[k] = " ".join([expand_variables(ctx, e, outs = ctx.outputs.outs, output_dir = ctx.attr.output_dir, attribute_name = "env") for e in _expand_locations(ctx, v)])
+
     tool_outputs = []
     if ctx.outputs.stdout:
         tool_outputs.append(ctx.outputs.stdout)
@@ -81,7 +85,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,
+        env = envs,
         stdout = ctx.outputs.stdout,
         stderr = ctx.outputs.stderr,
         exit_code_out = ctx.outputs.exit_code_out,
@@ -211,7 +215,8 @@ def npm_package_bin(
                 args = ["/".join([".."] * _package_segments + ["$@"])],
             )
             ```
-        env: specifies additional environment variables to set when the target is executed
+        env: specifies additional environment variables to set when the target is executed. The values of environment variables
+            are subject to 'Make variable' substitution (see [args](#npm_package_bin-args)).
         **kwargs: additional undocumented keyword args
     """
     if not tool:
diff --git a/internal/node/test/BUILD.bazel b/internal/node/test/BUILD.bazel
@@ -369,6 +369,15 @@ npm_package_bin(
         "somearg$$",
         "some0arg",
     ],
+    env = {
+        "OUTFILE": "$@",
+        "COMPLATION_MODE": "$(COMPILATION_MODE)",
+        "TARGET_CPU": "$(TARGET_CPU)",
+        "BINDIR": "$(BINDIR)",
+        "SOME_TEST_ENV": "$(SOME_TEST_ENV)",
+        "SOMEARG$$": "somearg$$",
+        "SOME0ARG": "some0arg",
+    },
     tool = ":expand_variables",
 )
 
@@ -378,6 +387,7 @@ jasmine_node_test(
     data = [":expand_variables.out"],
     templated_args = [
         "$(rootpath :expand_variables.out)",
+        "$(execpath :expand_variables.out)",
         "$(COMPILATION_MODE)",
         "$(TARGET_CPU)",
         "$(BINDIR)",
diff --git a/internal/node/test/expand_variables.js b/internal/node/test/expand_variables.js
@@ -1,4 +1,16 @@
 const fs = require('fs');
 const args = process.argv.slice(2);
-const outfile = args.shift();
-fs.writeFileSync(outfile, JSON.stringify(args, null, 2), 'utf-8');
+const outfile = args[0];
+const dump = {
+    args,
+    env: {
+        OUTFILE: process.env.OUTFILE,
+        COMPLATION_MODE: process.env.COMPLATION_MODE,
+        TARGET_CPU: process.env.TARGET_CPU,
+        BINDIR: process.env.BINDIR,
+        SOME_TEST_ENV: process.env.SOME_TEST_ENV,
+        SOMEARG$$: process.env.SOMEARG$$,
+        SOME0ARG: process.env.SOME0ARG,
+    }
+}
+fs.writeFileSync(outfile, JSON.stringify(dump, null, 2), 'utf-8');
diff --git a/internal/node/test/expand_variables.spec.js b/internal/node/test/expand_variables.spec.js
@@ -20,10 +20,21 @@ const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
 const args = process.argv.slice(2);
 const out = JSON.parse(
     require('fs').readFileSync(runfiles.resolveWorkspaceRelative(args.shift()), 'utf-8'));
-const expected = args;
+const expected_args = args;
 
 describe('nodejs_test templated_args variable expansion', function() {
   it('should match variable expansion in npm_package_bin args', function() {
-    expect(out).toEqual(expected);
+    expect(out.args).toEqual(expected_args);
+  });
+  it('should match variable expansion in npm_package_bin env vars', function() {
+    expect(out.env).toEqual({
+      OUTFILE: expected_args[0],
+      COMPLATION_MODE: expected_args[1],
+      TARGET_CPU: expected_args[2],
+      BINDIR: expected_args[3],
+      SOME_TEST_ENV: expected_args[4],
+      SOMEARG$$: expected_args[5],
+      SOME0ARG: expected_args[6],
+    })
   });
 });
