support {chroot} for program arguments in remote execution
#21803
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
tdyas
commented
Dec 30, 2024
Comment on lines
+1112
to
+1113
| Some(_) => "/bin/bash", // the array features need bash and not just plain old /bin/sh | ||
| _ => "/bin/sh", |
There was a problem hiding this comment.
These paths (and others) ideally would be discovered in the remote execution environment, but the REAPI client is what runs those programs in the remote execution environment in the first place. Maybe we should make these configurable in the future?
There was a problem hiding this comment.
Or an alternate solution like download BusyBox and use its ash shell (possibly only if array variables are supported).
There was a problem hiding this comment.
Hm, interesting approach. Definitely seems expedient.
huonw
approved these changes
Dec 31, 2024
Comment on lines
+1112
to
+1113
| Some(_) => "/bin/bash", // the array features need bash and not just plain old /bin/sh | ||
| _ => "/bin/sh", |
There was a problem hiding this comment.
Hm, interesting approach. Definitely seems expedient.
tdyas
added a commit
that referenced
this pull request
Jan 3, 2025
…ution (#21810) As described in #17519, remote execution environments did not substitute the `{chroot}` marker with the path to the execution sandbox as is done with local and Docker environments. #21803 added this support for program arguments. This PR completes the feature and implements that support for environment variables. Closes #17519.
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.
As reported in #17519, remote environments do not currently support the
{chroot}replacement marker supported by the local and Docker environments. With local and Docker environments, Pants will replace any{chroot}marker with the absolute path of the sandbox directory if it appears in a program argument or environment variable.This PR is a partial solution and implements
{chroot}replacement for prorgram arguments used in remote execution. It by extends the existing wrapper script used for append-only caches to also support{chroot}replacements.This PR does not try to implement
{chroot}replacement for environment variables. That will need to come after I figure out a good way to do that.