fix: ensure BAZEL_NODE_RUNFILES_HELPER & BAZEL_NODE_PATCH_REQUIRE are absolute #1634
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Toxicable
reviewed
Feb 13, 2020
… absolute ``` export BAZEL_NODE_RUNFILES_HELPER=$(rlocation "build_bazel_rules_nodejs/internal/linker/runfiles_helper.js") ``` resolves to `/Users/greg/google/rules_nodejs/internal/linker/runfiles_helper.js` when workers are on as there is no sandbox & resolves to `bazel-out/host/bin/external/build_bazel_rules_typescript/internal/tsc_wrapped_bin.sh.runfiles/build_bazel_rules_nodejs/internal/linker/runfiles_helper.js` when workers are off as there is a sandbox The latter is a relative path and won't work in a require() statement unless joined with the CWD. This commit adds the CWD in the launcher.
gregmagolan
force-pushed
the
fix-runfiles-helper-path
branch
from
9b373a0
to
89b5edd
Compare
alexeagle
approved these changes
Feb 13, 2020
| const args = process.argv.slice(2); | ||
| const outfile = args.shift(); | ||
| const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']); | ||
| const golden = |
There was a problem hiding this comment.
this line looks strange, lots of spaces for some reason?
There was a problem hiding this comment.
clang format did that for line length I think
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is for using
BAZEL_NODE_RUNFILES_HELPERinbazel buildwhen there are no runfiles and with a tool that does not support workers. The cwd is a path such as:/private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/sandbox/darwin-sandbox/18/execroot/build_bazel_rules_nodejs.resolves to
/Users/greg/google/rules_nodejs/internal/linker/runfiles_helper.jswhen workers are on as there is no sandbox (such as ts_library).&
resolves to
bazel-out/host/bin/external/build_bazel_rules_typescript/internal/tsc_wrapped_bin.sh.runfiles/build_bazel_rules_nodejs/internal/linker/runfiles_helper.jswhen workers are off as there is a sandboxThe latter is a relative path to the CWD and won't work in a require() statement unless joined with the CWD.
This PR adds the CWD in the launcher.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information