Skip to content

Commit

Permalink
Remove local execution requirement for npm_package's create_package (#…
Browse files Browse the repository at this point in the history
…1443)

* fix(builtin): remove local execution requirement for npm_package assembly action

Removing the execution_requirement of local execution as this ends up being more
costly when in a fully remote cachable tree of actions.

* fix(builtin): add an indentifying mnemonic to create_package run action

Adds a mnemonic to npm_package's create_package run action to allow
it to be more easily/better identified.
  • Loading branch information
josephperrott authored and alexeagle committed Dec 13, 2019
1 parent edc3905 commit b4782b8
Showing 1 changed file with 1 addition and 8 deletions.
9 changes: 1 addition & 8 deletions internal/npm_package/npm_package.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,11 @@ def create_package(ctx, deps_sources, nested_packages):

ctx.actions.run(
progress_message = "Assembling npm package %s" % package_dir.short_path,
mnemonic = "AssembleNpmPackage",
executable = ctx.executable._packager,
inputs = inputs,
outputs = [package_dir, ctx.outputs.pack, ctx.outputs.publish],
arguments = [args],
execution_requirements = {
# Never schedule this action remotely because it's not computationally expensive.
# It just copies files into a directory; it's not worth copying inputs and outputs to a remote worker.
# Also don't run it in a sandbox, because it resolves an absolute path to the bazel-out directory
# allowing the .pack and .publish runnables to work with no symlink_prefix
# See https://github.com/bazelbuild/rules_nodejs/issues/187
"local": "1",
},
)
return package_dir

Expand Down

1 comment on commit b4782b8

@alexeagle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this change broke our release - like it says, .pack and .publish were relying on this to avoid having the outputs go into a transient sandbox outdir.
@gregmagolan I'm hacking around this to get today's release out the door but it needs a more principled sol'n and also we should have a test on CI that runs the .pack command under Bazel run so we have some coverage

Please sign in to comment.