diff --git a/.bazelrc b/.bazelrc index 106e26158a..8b41665a72 100644 --- a/.bazelrc +++ b/.bazelrc @@ -24,6 +24,7 @@ build --noincompatible_no_support_tools_in_action_inputs # Define environment value used by some tests such as //internal/npm_install/test:bazel_bin_test build --define=SOME_TEST_ENV=some_value test --define=SOME_TEST_ENV=some_value +build --action_env=SOME_OTHER_ENV=some_other_value ############################### # Remote Build Execution support diff --git a/.circleci/config.yml b/.circleci/config.yml index 0250baa413..34b5925a2d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -10,7 +10,7 @@ ## IMPORTANT # If you change the `default_docker_image` version, also change the `cache_key` version var_1: &default_docker_image circleci/node:10.16 -var_3: &cache_key node-0.16-{{ checksum "yarn.lock" }} +var_3: &cache_key node-0.16-{{ checksum "yarn.lock" }}-v2 var_4: &init_environment run: diff --git a/internal/node/node.bzl b/internal/node/node.bzl index 7121a302c7..b3a29cae65 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -178,8 +178,13 @@ def _nodejs_binary_impl(ctx): env_vars = "export BAZEL_TARGET=%s\n" % ctx.label env_vars += "export BAZEL_WORKSPACE=%s\n" % ctx.workspace_name for k in ctx.attr.configuration_env_vars + ctx.attr.default_env_vars: + # Check ctx.var first & if env var not in there then check + # ctx.configuration.default_shell_env. The former will contain values from --define=FOO=BAR + # and latter will contain values from --action_env=FOO=BAR (but not from --action_env=FOO). if k in ctx.var.keys(): env_vars += "export %s=\"%s\"\n" % (k, ctx.var[k]) + elif k in ctx.configuration.default_shell_env.keys(): + env_vars += "export %s=\"%s\"\n" % (k, ctx.configuration.default_shell_env[k]) expected_exit_code = 0 if hasattr(ctx.attr, "expected_exit_code"): diff --git a/internal/node/npm_package_bin.bzl b/internal/node/npm_package_bin.bzl index 7cc10704b0..ee86ada734 100644 --- a/internal/node/npm_package_bin.bzl +++ b/internal/node/npm_package_bin.bzl @@ -9,6 +9,7 @@ load("//internal/linker:link_node_modules.bzl", "module_mappings_aspect") _ATTRS = { "outs": attr.output_list(), "args": attr.string_list(mandatory = True), + "configuration_env_vars": attr.string_list(default = []), "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]), "output_dir": attr.bool(), "tool": attr.label( @@ -57,6 +58,7 @@ def _impl(ctx): inputs = inputs, outputs = outputs, arguments = [args], + configuration_env_vars = ctx.attr.configuration_env_vars, ) return [DefaultInfo(files = depset(outputs))] diff --git a/internal/node/test/BUILD.bazel b/internal/node/test/BUILD.bazel index aa6784f409..52c4f29da8 100644 --- a/internal/node/test/BUILD.bazel +++ b/internal/node/test/BUILD.bazel @@ -70,7 +70,10 @@ nodejs_binary( nodejs_test( name = "define_var", - configuration_env_vars = ["SOME_TEST_ENV"], + configuration_env_vars = [ + "SOME_TEST_ENV", + "SOME_OTHER_ENV", + ], data = glob(["*.spec.js"]), entry_point = ":define.spec.js", ) @@ -271,6 +274,7 @@ npm_package_bin( "$@", "$(SOME_TEST_ENV)", ], + configuration_env_vars = ["SOME_OTHER_ENV"], tool = ":expand_variables", ) diff --git a/internal/node/test/define.spec.js b/internal/node/test/define.spec.js index 96e31d6921..92fa093305 100644 --- a/internal/node/test/define.spec.js +++ b/internal/node/test/define.spec.js @@ -6,3 +6,11 @@ if (process.env['SOME_TEST_ENV'] !== 'some_value') { console.error('should accept vars; must be run with --define=SOME_TEST_ENV=some_value'); process.exitCode = 1; } + +// Similar to above but testing another env variable that is expected to be specified with +// `--action_env=SOME_OTHER_ENV=some_other_value`. +if (process.env['SOME_OTHER_ENV'] !== 'some_other_value') { + console.error( + 'should accept vars; must be run with --action_env=SOME_OTHER_ENV=some_other_value'); + process.exitCode = 1; +} diff --git a/internal/node/test/expand_variables.golden b/internal/node/test/expand_variables.golden index a01ce16d0c..365138a739 100644 --- a/internal/node/test/expand_variables.golden +++ b/internal/node/test/expand_variables.golden @@ -1,3 +1,4 @@ [ - "some_value" + "some_value", + "some_other_value" ] \ No newline at end of file diff --git a/internal/node/test/expand_variables.js b/internal/node/test/expand_variables.js index 99754db7ca..60831552f6 100644 --- a/internal/node/test/expand_variables.js +++ b/internal/node/test/expand_variables.js @@ -1,4 +1,5 @@ const fs = require('fs'); const args = process.argv.slice(2); const outfile = args.shift(); +args.push(process.env['SOME_OTHER_ENV']); fs.writeFileSync(outfile, JSON.stringify(args, null, 2), 'utf-8'); diff --git a/internal/npm_install/test/bazel_bin_test.sh b/internal/npm_install/test/bazel_bin_test.sh index 181bac736c..77d3f20bbc 100755 --- a/internal/npm_install/test/bazel_bin_test.sh +++ b/internal/npm_install/test/bazel_bin_test.sh @@ -12,8 +12,9 @@ source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ # --- end runfiles.bash initialization v2 --- readonly OUT=$($(rlocation "npm/testy/bin/testy.sh")) +readonly EXPECTED="Hello some_value && some_other_value" -if [ "$OUT" != "Hello some_value" ]; then - echo "Expected output 'Hello world' but was '$OUT'" +if [ "${OUT}" != "${EXPECTED}" ]; then + echo "Expected output '${EXPECTED}' but was '${OUT}'" exit 1 fi diff --git a/internal/providers/node_runtime_deps_info.bzl b/internal/providers/node_runtime_deps_info.bzl index d9cd60a590..67068fa48f 100644 --- a/internal/providers/node_runtime_deps_info.bzl +++ b/internal/providers/node_runtime_deps_info.bzl @@ -63,10 +63,20 @@ def run_node(ctx, inputs, arguments, executable, **kwargs): # To access runfiles, you must use a runfiles helper in the program instead add_arg(arguments, "--nobazel_patch_module_resolver") - # Forward the COMPILATION_MODE to node process as an environment variable env = kwargs.pop("env", {}) - if "COMPILATION_MODE" not in env.keys(): - env["COMPILATION_MODE"] = ctx.var["COMPILATION_MODE"] + + # Always forward the COMPILATION_MODE to node process as an environment variable + configuration_env_vars = kwargs.pop("configuration_env_vars", []) + ["COMPILATION_MODE"] + for var in configuration_env_vars: + if var not in env.keys(): + # If env is not explicitely specified, check ctx.var first & if env var not in there + # then check ctx.configuration.default_shell_env. The former will contain values from + # --define=FOO=BAR and latter will contain values from --action_env=FOO=BAR + # (but not from --action_env=FOO). + if var in ctx.var.keys(): + env[var] = ctx.var[var] + elif var in ctx.configuration.default_shell_env.keys(): + env[var] = ctx.configuration.default_shell_env[var] ctx.actions.run( inputs = inputs + extra_inputs, diff --git a/tools/npm_packages/testy/index.js b/tools/npm_packages/testy/index.js index 59b585b450..cd7f678976 100755 --- a/tools/npm_packages/testy/index.js +++ b/tools/npm_packages/testy/index.js @@ -1 +1 @@ -console.log(`Hello ${process.env['SOME_TEST_ENV']}`); +console.log(`Hello ${process.env['SOME_TEST_ENV']} && ${process.env['SOME_OTHER_ENV']}`); diff --git a/tools/npm_packages/testy/package.json b/tools/npm_packages/testy/package.json index 00e7c80a89..7be0d1e41a 100644 --- a/tools/npm_packages/testy/package.json +++ b/tools/npm_packages/testy/package.json @@ -7,7 +7,7 @@ "bazelBin": { "testy": { "additionalAttributes": { - "configuration_env_vars": "[\"SOME_TEST_ENV\"]" + "configuration_env_vars": "[\"SOME_TEST_ENV\", \"SOME_OTHER_ENV\"]" } } }