-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Inject unresolved symlinks with BwoB #24833
base: master
Are you sure you want to change the base?
Conversation
e073054
to
837ccff
Compare
Creating unresolved symlinks locally when building with `--remote_download_outputs=minimal` results in unnecessary I/O and may not even be supported by the host (e.g. on Windows). Instead, only create it in the remote action file system.
837ccff
to
7438799
Compare
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.
Thanks for looking into this. I'd prefer if we simply changed RAFS#createSymbolicLink to only create the symlink in the in-memory component (instead of adding a separate injection API). I'm curious to know if you tried that approach but ran into trouble elsewhere? (One issue is that we can't use RAFS in SymlinkTreeHelper since they would never be materialized on disk, but that already needs to change in light of #24867.)
I would prefer that approach, in particular because it would also resolve (or at least help with) #21747. Should I give that a try in this PR? I remember that we weren't certain whether that would be safe. |
Let's give it a try in a separate PR and see what breaks? We can then reevaluate whether to attempt to fix whatever does, or come back to this one. |
I sent #24897 and will set the current PR to Draft. |
Creating unresolved symlinks locally when building with
--remote_download_outputs=minimal
results in unnecessary I/O and may not even be supported by the host (e.g. on Windows). Instead, only create it in the remote action file system.