-
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
Sandboxing allows creating files in the directories of input files #7273
Labels
not stale
Issues or PRs that are inactive but not considered stale
P2
We'll consider working on this in future. (Assignee optional)
team-Local-Exec
Issues and PRs for the Execution (Local) team
type: feature request
Milestone
Comments
jmmv
added
type: feature request
P2
We'll consider working on this in future. (Assignee optional)
team-Local-Exec
Issues and PRs for the Execution (Local) team
labels
Jan 28, 2019
bazel-io
pushed a commit
that referenced
this issue
Jan 28, 2019
The sandboxfs sandboxed spawn was creating writable file hierarchies for all input files. This should not be necessary if actions are well-behaved. Therefore, remove this for stricter sandboxing and to improve sandboxfs-based performance by avoiding a significant number of directory operations per action. More specifically, this code was put in place to workaround an issue with linking on macOS where a helper tool (make_hashed_objlist.py) wanted to create symlinks in the directories where inputs lived. I don't know why I concluded that the sandboxing code was wrong instead of thinking that the tooling had to be fixed. I have now fixed the offending tools in unknown commit so we can do this simplification. Addresses part of #7273. Tested: Built a relatively large iOS app with this change and the only problem I found is the aforementioned one in make_hashed_objlist.py. It is possible that this strictness change breaks other rules but the success of this build makes me relatively confident that problems are not widespread. RELNOTES: None. PiperOrigin-RevId: 231242503
weixiao-huang
pushed a commit
to weixiao-huang/bazel
that referenced
this issue
Jan 31, 2019
The sandboxfs sandboxed spawn was creating writable file hierarchies for all input files. This should not be necessary if actions are well-behaved. Therefore, remove this for stricter sandboxing and to improve sandboxfs-based performance by avoiding a significant number of directory operations per action. More specifically, this code was put in place to workaround an issue with linking on macOS where a helper tool (make_hashed_objlist.py) wanted to create symlinks in the directories where inputs lived. I don't know why I concluded that the sandboxing code was wrong instead of thinking that the tooling had to be fixed. I have now fixed the offending tools in unknown commit so we can do this simplification. Addresses part of bazelbuild#7273. Tested: Built a relatively large iOS app with this change and the only problem I found is the aforementioned one in make_hashed_objlist.py. It is possible that this strictness change breaks other rules but the success of this build makes me relatively confident that problems are not widespread. RELNOTES: None. PiperOrigin-RevId: 231242503
luca-digrazia
pushed a commit
to luca-digrazia/DatasetCommitsDiffSearch
that referenced
this issue
Sep 4, 2022
The sandboxfs sandboxed spawn was creating writable file hierarchies for all input files. This should not be necessary if actions are well-behaved. Therefore, remove this for stricter sandboxing and to improve sandboxfs-based performance by avoiding a significant number of directory operations per action. More specifically, this code was put in place to workaround an issue with linking on macOS where a helper tool (make_hashed_objlist.py) wanted to create symlinks in the directories where inputs lived. I don't know why I concluded that the sandboxing code was wrong instead of thinking that the tooling had to be fixed. I have now fixed the offending tools in unknown commit so we can do this simplification. Addresses part of bazelbuild/bazel#7273. Tested: Built a relatively large iOS app with this change and the only problem I found is the aforementioned one in make_hashed_objlist.py. It is possible that this strictness change breaks other rules but the success of this build makes me relatively confident that problems are not widespread. RELNOTES: None. PiperOrigin-RevId: 231242503
Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so. |
sgowroji
added
the
not stale
Issues or PRs that are inactive but not considered stale
label
Feb 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
not stale
Issues or PRs that are inactive but not considered stale
P2
We'll consider working on this in future. (Assignee optional)
team-Local-Exec
Issues and PRs for the Execution (Local) team
type: feature request
When sandboxing is enabled, an action is currently allowed to create new files in the directories that correspond to input files.
This is probably an accident of how symlink-based sandboxing works: the directories are created in the sandbox location and are never protected. But, for sandboxfs-based sandboxing, this requires extra code and incurs a significant runtime cost due to the need to create directories just in case an action might want to write to them.
I think this is a mistake: treating the location of input files as read-only is generally good practice (imagine building off a source tree on a read-only file system). Thus we should move towards stricter sandboxing. I have an upcoming change to do this in the sandboxfs variant, but we should probably do the same in the symlink variant for consistency reasons.
However, doing this change might break existing builds so it's not trivial to roll out (might need to go through the incompatible changes dance). So far I have encountered a single problem (fixed in f876830) though, so the risk is probably low.
The text was updated successfully, but these errors were encountered: