-
Notifications
You must be signed in to change notification settings - Fork 4.4k
[8.5.1] Partially revert "Make download cache entries read-only" #28161
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
Conversation
This reverts commit 65fe463. Breaks repo rules that hit a download cache entry and subsequently attempt to overwrite the file (e.g. http_archive's handling of remote module files). Keeps the added test as it doesn't depend on the change. Work towards bazelbuild#28031 Closes bazelbuild#28148. PiperOrigin-RevId: 852373210 Change-Id: I9e5e9ae87696068fbcb41cfd4dc77c36fea8248b
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.
Code Review
This pull request partially reverts a change that made download cache entries read-only. While this fixes a regression where some repository rules were breaking, it re-introduces a critical risk of download cache corruption. When hardlinks are used, any modification to a file retrieved from the cache will also modify the entry in the cache itself, which could lead to incorrect builds. Although this revert seems necessary as a short-term fix for a patch release, a more robust long-term solution should be prioritized to mitigate this risk.
I am having trouble creating individual review comments. Click here to see my feedback.
src/main/java/com/google/devtools/build/lib/bazel/repository/cache/DownloadCache.java (249-250)
Removing this write-protection on cache entries re-introduces a significant risk of cache corruption. When hardlinks are enabled for the cache, any rule that modifies a file fetched from the cache will also corrupt the entry in the cache. This can lead to incorrect and non-reproducible builds. While this revert is understandable to fix a regression for a patch release, a long-term solution is needed. A better approach might be for rules that modify cached files to request a copy instead of a hardlink (e.g., by using mayHardlink=false in the get() call).
This upgrade brings several performance improvements and bug fixes: #### Performance improvements - remote execution: bazelbuild/bazel#27564 - module extensions: bazelbuild/bazel#27296 #### Reliability improvements - cache invalidation: bazelbuild/bazel#27417 - configuration: bazelbuild/bazel#27128 - Git repositories: bazelbuild/bazel#27705 - query: - bazelbuild/bazel#27560 - bazelbuild/bazel#27117 - registry mirrors: bazelbuild/bazel#27531 #### Bug fixes - remote cache: bazelbuild/bazel#27996 - repository handling: bazelbuild/bazel#27995 - repository cache: bazelbuild/bazel#28161 - local execution: bazelbuild/bazel#27994
This upgrade brings several performance improvements and bug fixes: ## Performance improvements (8.5.0) - Remote execution: Add --remote_max_concurrency_per_connection flag to control concurrent gRPC requests (default: 100) bazelbuild/bazel#27564 - Module extensions: Support storing/retrieving JSON-like Starlark objects without invalidation, reducing unnecessary rebuilds bazelbuild/bazel#27296 ## Reliability improvements (8.5.0) - Cache invalidation: Source directory contents now tracked for proper invalidation bazelbuild/bazel#27417 - Configuration: Add ctx.configuration.short_id for identifying configurations bazelbuild/bazel#27128 - Git repositories: git_repository now checks out default branch when unspecified bazelbuild/bazel#27705 - Query: Add executables() function and fix genquery for external repos bazelbuild/bazel#27560 bazelbuild/bazel#27117 - Registry mirrors: --module_mirrors now supports per-registry mirror specification bazelbuild/bazel#27531 ## Bug fixes (8.5.1) - Remote cache: Add option to continue with local execution if remote cache is unavailable bazelbuild/bazel#27996 - Repository handling: Fix crash when mixing use_repo_rule and --inject_repository bazelbuild/bazel#27995 - Repository cache: Fix permission denied issue with --experimental_repository_cache_hardlinks bazelbuild/bazel#28161 - Local execution: Fix incorrect SkyframeLookupResult usage bazelbuild/bazel#27994 Both 8.5.0 and 8.5.1 are fully backward compatible with Bazel 8.0. ## Dependency updates - Upgrade rules_go from 0.57.0 to 0.59.0 for Bazel 8.5+ compatibility bazel-contrib/rules_go#4493 - Configure sh_configure extension for rules_shell to auto-detect shell toolchain ## Platform-specific changes - Windows: Configure hermetic shell via --repo_env=BAZEL_SH which is used by both sh_configure (sh_binary/sh_test) and --shell_executable (genrule/run_shell). This eliminates dependency on system environment variables. - Windows: Disable code coverage collection (--nocollect_code_coverage) to avoid shell toolchain issues. Coverage requires sh_binary (collect_coverage) which needs a hermetic shell toolchain not yet available. bazelbuild/rules_shell#4 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This upgrade brings several performance improvements and bug fixes: #### Performance improvements - remote execution: bazelbuild/bazel#27564 - module extensions: bazelbuild/bazel#27296 #### Reliability improvements - cache invalidation: bazelbuild/bazel#27417 - configuration: bazelbuild/bazel#27128 - Git repositories: bazelbuild/bazel#27705 - query: - bazelbuild/bazel#27560 - bazelbuild/bazel#27117 - registry mirrors: bazelbuild/bazel#27531 #### Bug fixes - remote cache: bazelbuild/bazel#27996 - repository handling: bazelbuild/bazel#27995 - repository cache: bazelbuild/bazel#28161 - local execution: bazelbuild/bazel#27994
This reverts commit 65fe463.
Breaks repo rules that hit a download cache entry and subsequently attempt to overwrite the file (e.g. http_archive's handling of remote module files).
Keeps the added test as it doesn't depend on the change.
Work towards #28031
Closes #28148.
PiperOrigin-RevId: 852373210
Change-Id: I9e5e9ae87696068fbcb41cfd4dc77c36fea8248b
Commit d5dba3f