-
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
Wire up PathMapper
in RemoteExecutionService
#19721
Conversation
src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
Outdated
Show resolved
Hide resolved
`PathMapper`s rewrite paths in command lines to make them more cache friendly, which requires executor support to stage files at the rewritten paths. This commit wires up the `PathMapper` used by a given `Spawn` in `SpawnInputsExpander`, which takes care of this for inputs to `Spawn`s executed in a sandbox or remotely. Constructs specific to Blaze, filesets and archived tree artifacts, are not covered by this change. An end-to-end test will be added in #18155, but requires #19718, #19719, and #19721. Work towards #6526 Closes #19718. PiperOrigin-RevId: 571109361 Change-Id: Ia38464011f658178ab2a1981a3ddaf5aead7c8fa
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 you please add two tests:
- One to verify that
buildRemoteAction
maps both inputs and outputs correctly. - One to verify that
downloadOutputs
correctly downloadsbazel-out/xxx/bin/foo
even when it gets path-stripped intobazel-out/bin/foo
(pass a customRemoteOutputChecker
into theRemoteExecutionService
).
src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
Show resolved
Hide resolved
2a468b6
to
cadb5ef
Compare
`PathMapper`s rewrite paths in command lines to make them more cache friendly, which requires executor support to stage files at the rewritten paths. This commit wires up the `PathMapper` used by a given `Spawn` with sandbox outputs logic for sandboxed and worker sandboxed execution. An end-to-end test will be added in #18155, but requires #19718, #19719, and #19721. Work towards #6526 Closes #19719. PiperOrigin-RevId: 571950903 Change-Id: I4a9d15da578da11f8677279b09e0864b8c33e9fc
`PathMapper`s rewrite paths in command lines to make them more cache friendly, which requires executor support to stage files at the rewritten paths. This commit wires up the `PathMapper` used by a given `Spawn` in `RemoteExecutionService`, which ensures that paths of inputs and outputs are correctly mapped before being sent off to the remote executor and mapped back to the correct local paths when downloading the results. Work towards bazelbuild#6526
cadb5ef
to
d54bc88
Compare
I resolved the merge conflicts. |
@tjgq Is this still on track for getting merged? Happy to look into any issues it causes. |
Sorry for the delay. It should be merged in today! |
PathMapper
s rewrite paths in command lines to make them more cache friendly, which requires executor support to stage files at the rewritten paths. This commit wires up thePathMapper
used by a givenSpawn
inRemoteExecutionService
, which ensures that paths of inputs and outputs are correctly mapped before being sent off to the remote executor and mapped back to the correct local paths when downloading the results.An end-to-end test will be added in #18155, but requires #19718, #19719, and #19721.
Work towards #6526