-
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
[7.2.0] Preserve paths under hermetic /tmp
in the sandbox
#22407
Merged
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
The bind mounting scheme used with the Linux sandbox' hermetic `/tmp` feature is modified to preserve all paths as they are outside the sandbox, which removes the need to rewrite paths when staging inputs into and, crucially, moving outputs out of the sandbox. Source roots and output base paths under `/tmp` are now treated just like any user-specified bind mount under `/tmp`: They are mounted under the hermetic tmp directory with their path relativized against `/tmp` before the hermetic tmp directory is mounted as `/tmp` as the final step. There is one caveat compared to user-specified mounts: Source roots, which may themselves not lie under `/tmp`, can be symlinks to directories under `/tmp` (e.g., when they arise from a `local_repository`). To handle this situation in the common case, all parent directories of package path entries (up to direct children of `/tmp`) are mounted into the sandbox. If users use `local_repository`s with fixed target paths under `/tmp`, they will need to specify `--sandbox_add_mount_pair`. Overlayfs has been considered as an alternative to this approach, but ultimately doesn't seem to work for this use case since its `lowerpath`, which would be `/tmp`, is not allowed to have child mounts from a different user namespace (see https://unix.stackexchange.com/questions/776030/mounting-overlayfs-in-a-user-namespace-with-child-mounts). However, this is exactly the situation created by a Bazel-in-Bazel test and can also arise if the user has existing mounts under `/tmp` when using Bazel (e.g. the JetBrains toolbox on Linux uses such mounts). This replaces and mostly reverts the following commits, but keeps their tests: * bazelbuild@bf6ebe9 * bazelbuild@fb6658c * bazelbuild@bc1d9d3 * bazelbuild@1829883 * bazelbuild@70691f2 * bazelbuild@a556969 * bazelbuild@8e32f44 (had its test lost in an incorrect merge conflict resolution, this PR adds it back) Fixes bazelbuild#20533 Work towards bazelbuild#20753 Fixes bazelbuild#21215 Fixes bazelbuild#22117 Fixes bazelbuild#22226 Fixes bazelbuild#22290 RELNOTES: Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. Closes bazelbuild#22001. PiperOrigin-RevId: 634381503 Change-Id: I9f7f3948c705be120c55c9b0c51204e5bea45f61
github-actions
bot
added
awaiting-review
PR is awaiting review from an assigned reviewer
team-Local-Exec
Issues and PRs for the Execution (Local) team
team-Performance
Issues for Performance teams
labels
May 16, 2024
oquenchil
approved these changes
May 16, 2024
oquenchil
added
awaiting-PR-merge
PR has been approved by a reviewer and is ready to be merge internally
and removed
awaiting-review
PR is awaiting review from an assigned reviewer
labels
May 16, 2024
keertk
removed
the
awaiting-PR-merge
PR has been approved by a reviewer and is ready to be merge internally
label
May 16, 2024
Merged
via the queue into
bazelbuild:release-7.2.0
with commit May 16, 2024
d98b02e
33 checks passed
hubot
pushed a commit
to aosp-mirror/kernel_common
that referenced
this pull request
Jun 3, 2024
…links in it" This reverts commit b41110b. Reason of revert: the hack in http://r.android.com/2952813 "Kleaf: Hack: Match all combinations for ROOT_DIR" no longer works with Bazel 7.2.0 rc2 due to bazelbuild/bazel#22407 "[7.2.0] Preserve paths under hermetic /tmp in the sandbox". Until we found a better solution, this patch is reverted to fix code indexing. Bug: 343803993 Bug: 320449031 Link: bazelbuild/bazel#22600 Change-Id: I3ba9e924d01149e2b3bf2f051d2e56f4d2129d8a Signed-off-by: Yifan Hong <elsk@google.com>
keertk
pushed a commit
that referenced
this pull request
Jun 10, 2024
Release Notes: Configurability: + aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. (#22135) + Added a new flag `--incompatible_disable_native_repo_rules` to disable native repo rule usage in WORKSPACE. All native repo rules now have a Starlark counterpart that can be used in both WORKSPACE and Bzlmod; see #22080 for more details. (#22203) + Starlark command-line flags can now be referred to through `alias` targets. (#22212) ExternalDeps: + bzlmod `git_repository` now accepts the `strip_prefix` arg and passes it to the underlying `git_repository` call. (#22137) + Added a new `include()` directive to `MODULE.bazel` files, which allows the root module file to be divided into multiple segments. (#22204) + Fixed certain deadlocks in repo fetching with worker threads (`--experimental_worker_for_repo_fetching=auto`). (#22261) + `print` statements in module files are now only executed for the root module and modules subject to non-registry overrides (e.g. `local_path_override`). (#22263) + The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. (#22371) + `Label` instances passed to `print` or `fail` as positional arguments are now formatted with apparent repository names (optimized for human readability). (#22460) + Changes to environment variables read via `getenv` now correctly invalidate module extensions. (#22541) + Git merge conflicts in `MODULE.bazel.lock` files can be resolved automatically. See https://bazel.build/external/lockfile#automatic-resolution for the required setup. (#22650) OSS: + Bazel on Linux and BSD now respects the XDG_CACHE_HOME environment variable instead of assuming that ~/.cache/bazel is writable. (#21817) Performance: + Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. (#22407) Remote-Exec: + The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. (#22327) + The compact and full execution logs now contain start times for spawns (if available). (#22341) Rules-CPP: + The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`. (#22369) Starlark-Interpreter: + Starlark `min` and `max` buitins now allow a `key` callback, similarly to `sorted`. (#21960) Acknowledgements: This release contains contributions from many people at Google, as well as bazel.build machine account, Brentley Jones, Cameron Martin, Daniel Wagner-Hall, Douglas Thor, Fabian Meumertzheim, George Gensure, hvd, Isaac Torres, Keith Smiley, Mark Elliot, oquenchil, Romain Chossart, Son Luong Ngoc, Spencer Putt, Thomas Weischuh, Xdng Yng, Xùdōng Yáng, Zheng Wei Tan.
copybara-service bot
pushed a commit
that referenced
this pull request
Jun 10, 2024
Release Notes: Configurability: + aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. (#22135) + Added a new flag `--incompatible_disable_native_repo_rules` to disable native repo rule usage in WORKSPACE. All native repo rules now have a Starlark counterpart that can be used in both WORKSPACE and Bzlmod; see #22080 for more details. (#22203) + Starlark command-line flags can now be referred to through `alias` targets. (#22212) ExternalDeps: + bzlmod `git_repository` now accepts the `strip_prefix` arg and passes it to the underlying `git_repository` call. (#22137) + Added a new `include()` directive to `MODULE.bazel` files, which allows the root module file to be divided into multiple segments. (#22204) + Fixed certain deadlocks in repo fetching with worker threads (`--experimental_worker_for_repo_fetching=auto`). (#22261) + `print` statements in module files are now only executed for the root module and modules subject to non-registry overrides (e.g. `local_path_override`). (#22263) + The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. (#22371) + `Label` instances passed to `print` or `fail` as positional arguments are now formatted with apparent repository names (optimized for human readability). (#22460) + Changes to environment variables read via `getenv` now correctly invalidate module extensions. (#22541) + Git merge conflicts in `MODULE.bazel.lock` files can be resolved automatically. See https://bazel.build/external/lockfile#automatic-resolution for the required setup. (#22650) OSS: + Bazel on Linux and BSD now respects the XDG_CACHE_HOME environment variable instead of assuming that ~/.cache/bazel is writable. (#21817) Performance: + Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. (#22407) Remote-Exec: + The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. (#22327) + The compact and full execution logs now contain start times for spawns (if available). (#22341) Rules-CPP: + The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`. (#22369) Starlark-Interpreter: + Starlark `min` and `max` buitins now allow a `key` callback, similarly to `sorted`. (#21960) Acknowledgements: This release contains contributions from many people at Google, as well as bazel.build machine account, Brentley Jones, Cameron Martin, Daniel Wagner-Hall, Douglas Thor, Fabian Meumertzheim, George Gensure, hvd, Isaac Torres, Keith Smiley, Mark Elliot, oquenchil, Romain Chossart, Son Luong Ngoc, Spencer Putt, Thomas Weischuh, Xdng Yng, Xùdōng Yáng, Zheng Wei Tan.
rdeushane
pushed a commit
to SibrosTech/bazel
that referenced
this pull request
Jun 19, 2024
Release Notes: Configurability: + aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. (bazelbuild#22135) + Added a new flag `--incompatible_disable_native_repo_rules` to disable native repo rule usage in WORKSPACE. All native repo rules now have a Starlark counterpart that can be used in both WORKSPACE and Bzlmod; see bazelbuild#22080 for more details. (bazelbuild#22203) + Starlark command-line flags can now be referred to through `alias` targets. (bazelbuild#22212) ExternalDeps: + bzlmod `git_repository` now accepts the `strip_prefix` arg and passes it to the underlying `git_repository` call. (bazelbuild#22137) + Added a new `include()` directive to `MODULE.bazel` files, which allows the root module file to be divided into multiple segments. (bazelbuild#22204) + Fixed certain deadlocks in repo fetching with worker threads (`--experimental_worker_for_repo_fetching=auto`). (bazelbuild#22261) + `print` statements in module files are now only executed for the root module and modules subject to non-registry overrides (e.g. `local_path_override`). (bazelbuild#22263) + The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. (bazelbuild#22371) + `Label` instances passed to `print` or `fail` as positional arguments are now formatted with apparent repository names (optimized for human readability). (bazelbuild#22460) + Changes to environment variables read via `getenv` now correctly invalidate module extensions. (bazelbuild#22541) + Git merge conflicts in `MODULE.bazel.lock` files can be resolved automatically. See https://bazel.build/external/lockfile#automatic-resolution for the required setup. (bazelbuild#22650) OSS: + Bazel on Linux and BSD now respects the XDG_CACHE_HOME environment variable instead of assuming that ~/.cache/bazel is writable. (bazelbuild#21817) Performance: + Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. (bazelbuild#22407) Remote-Exec: + The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. (bazelbuild#22327) + The compact and full execution logs now contain start times for spawns (if available). (bazelbuild#22341) Rules-CPP: + The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`. (bazelbuild#22369) Starlark-Interpreter: + Starlark `min` and `max` buitins now allow a `key` callback, similarly to `sorted`. (bazelbuild#21960) Acknowledgements: This release contains contributions from many people at Google, as well as bazel.build machine account, Brentley Jones, Cameron Martin, Daniel Wagner-Hall, Douglas Thor, Fabian Meumertzheim, George Gensure, hvd, Isaac Torres, Keith Smiley, Mark Elliot, oquenchil, Romain Chossart, Son Luong Ngoc, Spencer Putt, Thomas Weischuh, Xdng Yng, Xùdōng Yáng, Zheng Wei Tan.
7 tasks
hopez13
pushed a commit
to hopez13/kernel-superproject
that referenced
this pull request
Dec 26, 2024
This reverts commit 651d0913be7550fd6b9f77ee268124c01f17c924. Reason for revert: There have been enough issues of 7.2.0rc2, mainly due to bazelbuild/bazel#22407 ([7.2.0] Preserve paths under hermetic /tmp in the sandbox), including: - Kythe was broken (b/343803993) - Reproducibility is broken because realpath changes (b/345452375, WIP b/346559442) and due to bazelbuild/bazel#22480 ([7.2.0] Update @rules_java v7.6.1): - docs_dist broken (WIP b/345143232) To avoid stability issues, and because the non rc 7.2.0 won't have enough time to settle down, reverting back to the stabler 7.1.1. This change is part of the rollback. Bug: 345143232 Bug: 343803993 Bug: 345452375 Bug: 346559442 Change-Id: Ic90004e2b9011f4d56ada7fd9f15588c9ef20b95
hopez13
pushed a commit
to hopez13/kernel-superproject
that referenced
this pull request
Dec 26, 2024
This reverts commit 651d0913be7550fd6b9f77ee268124c01f17c924. Reason for revert: There have been enough issues of 7.2.0rc2, mainly due to bazelbuild/bazel#22407 ([7.2.0] Preserve paths under hermetic /tmp in the sandbox), including: - Kythe was broken (b/343803993) - Reproducibility is broken because realpath changes (b/345452375, WIP b/346559442) and due to bazelbuild/bazel#22480 ([7.2.0] Update @rules_java v7.6.1): - docs_dist broken (WIP b/345143232) To avoid stability issues, and because the non rc 7.2.0 won't have enough time to settle down, reverting back to the stabler 7.1.1. This change is part of the rollback. Bug: 345143232 Bug: 343803993 Bug: 345452375 Bug: 346559442 Change-Id: Ic90004e2b9011f4d56ada7fd9f15588c9ef20b95
hopez13
pushed a commit
to hopez13/kernel-superproject
that referenced
this pull request
Dec 26, 2024
This reverts commit 651d0913be7550fd6b9f77ee268124c01f17c924. Reason for revert: There have been enough issues of 7.2.0rc2, mainly due to bazelbuild/bazel#22407 ([7.2.0] Preserve paths under hermetic /tmp in the sandbox), including: - Kythe was broken (b/343803993) - Reproducibility is broken because realpath changes (b/345452375, WIP b/346559442) and due to bazelbuild/bazel#22480 ([7.2.0] Update @rules_java v7.6.1): - docs_dist broken (WIP b/345143232) To avoid stability issues, and because the non rc 7.2.0 won't have enough time to settle down, reverting back to the stabler 7.1.1. This change is part of the rollback. Bug: 345143232 Bug: 343803993 Bug: 345452375 Bug: 346559442 Change-Id: Ic90004e2b9011f4d56ada7fd9f15588c9ef20b95
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
team-Local-Exec
Issues and PRs for the Execution (Local) team
team-Performance
Issues for Performance teams
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.
The bind mounting scheme used with the Linux sandbox' hermetic
/tmp
feature is modified to preserve all paths as they are outside the sandbox, which removes the need to rewrite paths when staging inputs into and, crucially, moving outputs out of the sandbox.Source roots and output base paths under
/tmp
are now treated just like any user-specified bind mount under/tmp
: They are mounted under the hermetic tmp directory with their path relativized against/tmp
before the hermetic tmp directory is mounted as/tmp
as the final step.There is one caveat compared to user-specified mounts: Source roots, which may themselves not lie under
/tmp
, can be symlinks to directories under/tmp
(e.g., when they arise from alocal_repository
). To handle this situation in the common case, all parent directories of package path entries (up to direct children of/tmp
) are mounted into the sandbox. If users uselocal_repository
s with fixed target paths under/tmp
, they will need to specify--sandbox_add_mount_pair
.Overlayfs has been considered as an alternative to this approach, but ultimately doesn't seem to work for this use case since its
lowerpath
, which would be/tmp
, is not allowed to have child mounts from a different user namespace (see https://unix.stackexchange.com/questions/776030/mounting-overlayfs-in-a-user-namespace-with-child-mounts). However, this is exactly the situation created by a Bazel-in-Bazel test and can also arise if the user has existing mounts under/tmp
when using Bazel (e.g. the JetBrains toolbox on Linux uses such mounts).This replaces and mostly reverts the following commits, but keeps their tests:
Fixes #20533
Work towards #20753
Fixes #21215
Fixes #22117
Fixes #22226
Fixes #22290
RELNOTES: Paths in the Linux sandbox are now again identical to those outside the sandbox, even with
--incompatible_sandbox_hermetic_tmp
.Closes #22001.
PiperOrigin-RevId: 634381503
Change-Id: I9f7f3948c705be120c55c9b0c51204e5bea45f61
Fixes #22291