fix(builtin): always symlink node_modules at execroot/my_wksp/node_modules even when running in runfiles
#1805
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
gregmagolan
force-pushed
the
linker_at_execroot
branch
from
0969e20 to
2ae8bb2
Compare
|
Noting here that I tested manually on my Windows laptop to verify that this resolves #1781. |
gregmagolan
force-pushed
the
linker_at_execroot
branch
3 times, most recently
from
bad474f to
8c7303d
Compare
alexeagle
approved these changes
Apr 9, 2020
| const startCwd = process.cwd().replace(/\\/g, '/'); | ||
| log_verbose('startCwd', startCwd); | ||
|
|
||
| // We can derive if the process is being run in the execroot if there is a bazel-out folder. |
There was a problem hiding this comment.
this is a different mechanism than in your other PR where you check for execroot in the path. Is one better than the other?
There was a problem hiding this comment.
Yes. Looking for execroot basename will not work in RBE. I'll fix up the other PR. Thanks!
There was a problem hiding this comment.
Actually, the other PR needs to change once it is rebased on top of this one after it lands since the location of the linker node_modules has moved here.
There was a problem hiding this comment.
#1800 for future reference
gregmagolan
force-pushed
the
linker_at_execroot
branch
from
8c7303d to
9b84776
Compare
…odules` even when running in runfiles Under runfiles, the linker should symlink node_modules at `execroot/my_wksp` so that when there are no runfiles (default on Windows) and scripts run out of `execroot/my_wksp` they can resolve node_modules with standard node_module resolution Also, restore BAZEL_WORKSPACE name environment variable. The optimization of deriving the workspace name from the path does not work on RBE. 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 the workspace name is obfuscated from the path. So we provide the workspace name here as an environment variable avaiable to all actions for the runfiles helpers to use.
gregmagolan
force-pushed
the
linker_at_execroot
branch
from
9b84776 to
f399be3
Compare
This was referenced Apr 9, 2020
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1803 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1813 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1813 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1813 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1813 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1813 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
to gregmagolan/rules_nodejs
that referenced
this pull request
Apr 9, 2020
Issue bazel-contrib#1813 was fixed by bazel-contrib#1800 which is already landed. bazel-contrib#1805 also fixes via a different mechanism and will also land soon.
gregmagolan
added a commit
that referenced
this pull request
Apr 9, 2020
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.
Fixes #1781.
Under runfiles, the linker should symlink node_modules at
execroot/my_wkspso that when there are no runfiles (default on Windows) and scripts run out ofexecroot/my_wkspthey can resolve node_modules with standard node_module resolution.Also, restore BAZEL_WORKSPACE name environment variable. The optimization of deriving the workspace name from the path does not work on RBE. While we can derive the workspace from the pwd when running locally because it is in the execroot path
execroot/my_wksp, on RBE theexecroot/my_wksppath is reduced a path such as/w/f/bso the workspace name is obfuscated from the path. So we provide the workspace name here as an environment variable avaiable to all actions for the runfiles helpers to use.