0.14 has no way to unset --disk_cache flag (interferes with remote build testing)

[jmillikin-stripe]

Description of the problem / feature request:

The local disk cache and remote execution action strategy are incompatible -- if both are enabled in flags or bazelrc, Bazel will refuse to run.

In 0.13, disk caching is controlled by --experimental_local_disk_cache_path (string) and --experimental_local_disk_cache (bool). It was possible to enable caching in /etc/bazel.bazelrc, and disable it in a project's tools/bazel.rc for testing remote execution.

In 0.14 the flags were unified into --disk_cache (string) which is definitely nicer, but provides no mechanism to disable the disk cache in a project's local bazelrc. The check for "disk cache enabled?" is whether the diskCache option is not null, which is true if the flag has ever been set to anything.

This makes it very difficult to work with remote builds in 0.14 when running in a build environment with a standard cross-project /etc/bazel.bazelrc.

It would be nice if an empty value could also be considered "disk cache not enabled", so passing an empty string like bazel build --disk-cache= //... would work.

[jmillikin-stripe]

Related to (possible duplicate of?) #4595

[ittaiz]

Actually I’d like to suggest that the empty string will mean bazel uses a sensible default (like external dependencies cache) and will be on by default.
I agree there still needs to be a way to turn it off and in addition I think the real issue is the clash between the two cached.
I know @buchgr is working on an interim solution for this problem

[jmillikin-stripe]

Gentle ping -- this is kinda important for us (Stripe) because we're canarying remote execution in some of our larger builds, and we can't upgrade Bazel to 0.14 or later if doing so will break the remote execution flags.

[buchgr]

I am currently on vacation until the 18th of June. I have totally missed that use case. I‘d be happy to fix that then or you can submit a PR and @laurentlb can cherry pick that into 0.15 (I think?).

Both /dev/null and the empty string sound good to me.

[ittaiz]

I’d really ask reserving the empty string to mean working in a default location (like the external dependencies cache).
@jmillikin-stripe,
Have you considered composing bazelrc files? That’s what we do. We have a baserc and then a remoterc/.bazelrc which both import the baserc (has 95% if content). Then when we run the builds in CI with remote execution we explicitly specify the remoterc as the one to use.

This doesn’t mean I don’t think we need to solve this issue (toggle off / unifying the behavior like what @buchgr started to work on) but I wonder if you can’t unblock yourself for the time being

[jmillikin-stripe]

@buchgr OK, sent #5338 to treat an empty disk cache path the same as being unset.

@ittaiz I think we might be talking about different things. There is no meaningful "default location" for the SimpleBlobStore enabled by --disk_cache. And composing bazelrcs doesn't help when the presence of a flag in an "earlier" bazelrc will cause another flag to become invalid regardless of later overrides.

[ittaiz]

Why do you think there’s no meaningful default location for the disk cache? Also, My point about composition is that the disk cache options only exist in the .bazelrc which doesn’t get evaluated when you run in CI since there you override the bazelrc location to point to the remote.bazelrc. I’d really ask not to merge #5308 before we get to a conclusion here (cc @dslomov)

…

On Wed, 6 Jun 2018 at 20:46 jmillikin-stripe ***@***.***> wrote: @buchgr <https://github.com/buchgr> OK, sent #5338 <#5338> to treat an empty disk cache path the same as being unset. @ittaiz <https://github.com/ittaiz> I think we might be talking about different things. There is no meaningful "default location" for the SimpleBlobStore enabled by --disk_cache. And composing bazelrcs doesn't help when the presence of a flag in an "earlier" bazelrc will cause another flag to become invalid regardless of later overrides. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF8aXZ3E15aJNM-Jq3wFKJDuHLqucks5t6BVbgaJpZM4UV-2k> .

[jmillikin-stripe]

Why do you think there’s no meaningful default location for the disk cache?

Because if I run bazel build //:foo, the local disk cache is not enabled or used.

My point about composition is that the disk cache options only exist in the
.bazelrc which doesn’t get evaluated when you run in CI

If my CI system has a /etc/bazel.bazelrc that contains build --disk_cache=/some/cache/dir, and my project is canarying remote execution with its tools/bazel.rc containing build --spawn_strategy=remote, then Bazel will fail with an error about conflicting flags. In Bazel 0.14.0, with this setup, there is no possible way to override the --disk_cache flag in the project's bazelrc such that remote execution will work.

[ittaiz]

Re default- what I meant is running “bazel build —disk_cache= //:foo” (in actuality —disk_cache is specified in the project bazelrc). The reason to use this notation is because different operating systems might have different options for where to store it and Bazel people prefer not to allow ~ in the options.

Re unset- I now understand the difference in perspectives. I was thinking about only Dev machines using this flag while CI using only remote so my composition idea works.

Wdyt about adding another flag to unset it?

[jmillikin-stripe]

In your model, how would the behavior of bazel build --unset_disk_cache_flag //:foo and bazel build --disk_cache= //:foo be different?

[ittaiz]

Given “—disk_cache=“ bazel will turn on the disk cache and write it to ~/.bazel/cache (or something like this). I think that in future releases running “bazel build //:foo” will implicitly have a disk cache which uses the above location. Given “—unset_disk_cache” bazel will not use the disk cache at all.

…

On Wed, 6 Jun 2018 at 23:49 jmillikin-stripe ***@***.***> wrote: In your model, how would the behavior of bazel build --unset_disk_cache_flag //:foo and bazel build --disk_cache= //:foo be different? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF8m9rrVRULYJOKJ2g25XlsVqPcCTks5t6EBngaJpZM4UV-2k> .

[ulfjack]

I don't think the empty string should turn on the disk cache, that seems rather confusing. Note that we do always use the action cache by default, which is also on disk.

The difference between the disk cache and the action cache is that the disk cache contains full copies of all the files in your build, increasing on-disk footprint by at least a factor of 2, or more if you want it to be actually useful. As such, I don't see us enabling it by default.

[dslomov]

In the interest of time, here is the resolution:

• in 0.14.1 and 0.15 we will accept Allow disabling the simple blob caches via CLI flag overrides. #5338. Empty value for --disk_cache disables disk cache.
• going forward (I guess 0.16 and beyond) we will introduce a boolean --enable_disk_cache flag, off by default that will by default use some default location for disk cache. That location will be overridable with --disk_cache flag. --enable_disk_cache will enable disk cache even if --disk_cache= (empty string) is supplied (in that case, default location is used)

[jmillikin-stripe]

Ulf, Dmitry, thank you both for the quick response and the patch cherrypick!

[ittaiz]

Thanks @dslomov!
I think this resolution is indeed in everyone’s best interest.
@ulfjack the action cache is almost irrelevant when working without remote execution and on a developer machine (between branches for example). You can also see it from the issues people opened in this repo to have this support.

[ulfjack]

@ittaiz - I understand that - I have the disk cache enabled myself -, but I'm not sure that's the common case across all Bazel users, and I am concerned about the increase in disk usage in the case where users just checkout a single source tree and build it once or twice. I think the disk usage increase is amortized if you have multiple checkouts for the same code base.

[or-shachar]

@dslomov - seems like the --enable_disk_cache flag is not there in 0.17.2.
Currently we disable it by overriding disk_cache with empty value. Is this the final solution for this or should we follow some other issue or PR?