-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add RBE support for generated unresolved symlinks #15781
Conversation
bfe4b10
to
7f76ae8
Compare
@coeuvre Can you review this PR? This takes care of 1. in #15454 (comment) for generated symlinks. Supporting source symlinks would require additional, possibly backwards incompatible changes. Since these are actually quite important for the new |
// for non-normalized symlink targets, but the normalization applied by PathFragment.create | ||
// does not take directory symlinks into account. However, Bazel's file system API does not | ||
// currently offer a way to specify a raw String as a symlink target. | ||
dst.createSymbolicLink(PathFragment.create(symlink.getTarget())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be fixed after #15773.
@coeuvre Friendly ping |
@coeuvre Many Node.js users are waiting for this for a long time to have RBE with rules_js. It would be nice to get it reviewed. 😄 |
Sure. I will definitely review it once I get the time. |
|
||
bazel clean --expunge | ||
bazel \ | ||
--windows_enable_symlinks \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we check whether it is running on windows to conditionally add the flag --windows_enable_symlinks
instead of running the build twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added --windows_enable_symlinks
to both builds - it is a no-op on non-Windows platforms and the difference between the two builds should only be --experimental_remote_merkle_tree_cache
.
LGTM. Please rebase and update the test. |
Also adds support for such symlinks to the remote worker implementation.
7f76ae8
to
26c84be
Compare
Also adds support for such symlinks to the remote worker implementation. Work towards bazelbuild#10298 Closes bazelbuild#15781. PiperOrigin-RevId: 469196671 Change-Id: I441da6d0a658d142153ada6cc0f836bf2ed3bcff
Also adds support for such symlinks to the remote worker implementation.
Work towards #10298