Skip to content

Commit

Permalink
fix(builtin): don't restart npm_install rule just to look up a label (#…
Browse files Browse the repository at this point in the history
…2621)

Fixes #2620
  • Loading branch information
alexeagle authored Apr 22, 2021
1 parent fea3db3 commit 16d3a25
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions internal/npm_install/npm_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ cd /D "{root}" && "{npm}" {npm_args}
env[env_key] = "1"
env["BUILD_BAZEL_RULES_NODEJS_VERSION"] = VERSION

# NB: after running npm install, it's essential that we don't cause the repository rule to restart
# This means we must not reference any additional labels after this point.
# See https://github.com/bazelbuild/rules_nodejs/issues/2620
repository_ctx.report_progress("Running npm install on %s" % repository_ctx.attr.package_json)
result = repository_ctx.execute(
[repository_ctx.path("_npm.cmd" if is_windows_host else "_npm.sh")],
Expand All @@ -484,16 +487,18 @@ cd /D "{root}" && "{npm}" {npm_args}
if result.return_code:
fail("npm_install failed: %s (%s)" % (result.stdout, result.stderr))

remove_npm_absolute_paths = Label("//third_party/github.com/juanjoDiaz/removeNPMAbsolutePaths:bin/removeNPMAbsolutePaths")

# removeNPMAbsolutePaths is run on node_modules after npm install as the package.json files
# generated by npm are non-deterministic. They contain absolute install paths and other private
# information fields starting with "_". removeNPMAbsolutePaths removes all fields starting with "_".
fix_absolute_paths_cmd = [
node,
repository_ctx.path(repository_ctx.attr._remove_npm_absolute_paths),
root + "/node_modules",
]

if not repository_ctx.attr.quiet:
print([node, repository_ctx.path(remove_npm_absolute_paths), root + "/node_modules"])
result = repository_ctx.execute(
[node, repository_ctx.path(remove_npm_absolute_paths), root + "/node_modules"],
)
print(fix_absolute_paths_cmd)
result = repository_ctx.execute(fix_absolute_paths_cmd)

if result.return_code:
fail("remove_npm_absolute_paths failed: %s (%s)" % (result.stdout, result.stderr))
Expand Down Expand Up @@ -526,6 +531,7 @@ See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of su
mandatory = True,
allow_single_file = True,
),
"_remove_npm_absolute_paths": attr.label(default = Label("//third_party/github.com/juanjoDiaz/removeNPMAbsolutePaths:bin/removeNPMAbsolutePaths")),
}),
doc = """Runs npm install during workspace setup.
Expand Down

0 comments on commit 16d3a25

Please sign in to comment.