[8.5.1] Partially revert "Make download cache entries read-only"#28161
[8.5.1] Partially revert "Make download cache entries read-only"#28161iancha1992 merged 1 commit intobazelbuild:release-8.5.1from
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.
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 <[email protected]>
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