Skip to content

feat: Add default disk cache location via --disk_cache=on#28942

Open
Desel72 wants to merge 2 commits intobazelbuild:masterfrom
Desel72:fix/issue-28899
Open

feat: Add default disk cache location via --disk_cache=on#28942
Desel72 wants to merge 2 commits intobazelbuild:masterfrom
Desel72:fix/issue-28899

Conversation

@Desel72
Copy link

@Desel72 Desel72 commented Mar 10, 2026

Description

Add support for --disk_cache=on and --disk_cache=off sentinel values. --disk_cache=on places the disk cache in a default location at <outputUserRoot>/cache/disk, alongside the existing repository cache at <outputUserRoot>/cache/repos/v1. --disk_cache=off explicitly disables the disk cache (equivalent to not setting the flag).

Motivation

Fixes #28899

Currently, --disk_cache requires an explicit path, forcing users to choose and manage a location. With --disk_cache=on:

  1. The cache automatically follows host conventions (e.g. XDG_CACHE_HOME) since outputUserRoot already respects them.
  2. All Bazel-managed data stays in one place, making it easy to clean/reclaim disk space.
  3. Users don't have to think about where to put the cache.

Build API Changes

  1. Discussed in Default disk cache location, in outputUserRoot #28899
  2. Yes, backward compatible. Existing path values work as before. Only the new literal strings on and off change behavior (previously they would have been interpreted as relative directory names).
  3. N/A

Checklist

  • I have added tests for the new use cases (if any).
  • I have updated the documentation (if applicable).

Release Notes

RELNOTES[NEW]: --disk_cache=on now places the disk cache in a default location under the output user root (<outputUserRoot>/cache/disk). --disk_cache=off explicitly disables the disk cache.

@Desel72 Desel72 requested a review from a team as a code owner March 10, 2026 20:54
@github-actions github-actions bot added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Mar 10, 2026
@Desel72
Copy link
Author

Desel72 commented Mar 11, 2026

@gregestren @meteorcloudy could you please review my pr and share any feedback?

@gregestren
Copy link
Contributor

Thanks for your first contribution here.

build/lib/remote* is not my expertise, so i'll withhold from review. Since @tjgq adjusted the bug, maybe he could help?

@Desel72
Copy link
Author

Desel72 commented Mar 11, 2026

Hi @tjgq, just following up on this PR. If possible, could you please take a look and share your feedback?

if (outputUserRoot == null) {
throw createOptionsExitException(
"--disk_cache=on requires --output_user_root to be set",
FailureDetails.RemoteOptions.Code.EXECUTION_WITH_INVALID_CACHE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange.....if following the standard outputUserRoot resolution, it should be set

@Desel72
Copy link
Author

Desel72 commented Mar 11, 2026

@pauldraper You're right — outputUserRoot is always set by the client as a mandatory startup option. Removed the null check. Please review again. thank you.

@fmeum
Copy link
Collaborator

fmeum commented Mar 11, 2026

Having a magic string that isn't used in other options isn't that great. Could we use the same trick as in BoolOrEnumConverter to support --disk_cache (default location) and --nodisk_cache (disabled, same as --disk_cache)?

@Desel72
Copy link
Author

Desel72 commented Mar 12, 2026

Thank you for your feedback @fmeum I am going to address the magic strings with following way:
Create a DiskCacheOrPath class holding either a DiskCacheMode enum (ON/OFF) or a PathFragment
Add a DiskCacheOrPathConverter that parses --disk_cache inputs
Please let me know if this approach works for you, or if you’d prefer a different design.

@fmeum
Copy link
Collaborator

fmeum commented Mar 13, 2026

Please wait for @tjgq's feedback - I can't promise that this would be accepted, I just think that the UX is cleaner that way. Thanks for working on this!

@tjgq
Copy link
Contributor

tjgq commented Mar 16, 2026

I also prefer @fmeum's suggested approach.

@Desel72
Copy link
Author

Desel72 commented Mar 16, 2026

@tjgq

Thank you for your feedback @fmeum I am going to address the magic strings with following way: Create a DiskCacheOrPath class holding either a DiskCacheMode enum (ON/OFF) or a PathFragment Add a DiskCacheOrPathConverter that parses --disk_cache inputs
Please let me know if this approach works for you, or if you’d prefer a different design.

Is my approach ok to you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default disk cache location, in outputUserRoot

5 participants