Skip to content

Commit

Permalink
feat(builtin): add env attribute to nodejs test and binary and run_no…
Browse files Browse the repository at this point in the history
…de helper (#2499)
  • Loading branch information
mattem authored Mar 5, 2021
1 parent 94a6151 commit c9b159f
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 2 deletions.
10 changes: 10 additions & 0 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.""",
Expand Down
17 changes: 16 additions & 1 deletion internal/node/npm_package_bin.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ jasmine_node_test(
data = [
":dump_build_env.json",
":dump_build_env_alt.json",
":dump_build_env_attr_json",
],
)

Expand Down Expand Up @@ -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",
Expand All @@ -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"],
Expand Down
6 changes: 6 additions & 0 deletions internal/node/test/env.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
6 changes: 5 additions & 1 deletion internal/providers/node_runtime_deps_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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"]
Expand Down

0 comments on commit c9b159f

Please sign in to comment.