use DUNE_CACHE_HOME instead of XDG_CACHE_HOME

[ElectreAAS]

Fixes #11584 and should ease the pain of (but does NOT fix) #11585.

The build cache is currently set up to be at $DUNE_CACHE_ROOT, which itself will default on $XDG_CACHE_HOME if not present. Three different caches skipped $DUNE_CACHE_ROOT and used the XDG directly: the toolchain cache, the git-repo cache, and the lmdb (rev store) cache.

This PR introduces a new variable: $DUNE_CACHE_HOME to allow customization of cache locations without breaking existing behaviour. To recap:

	Current behaviour	With this PR
DUNE_CACHE_HOME defaults to	doesn't exist	XDG_CACHE_HOME/dune
DUNE_CACHE_ROOT defaults to	XDG/dune/db	DUNE_CACHE_HOME/db
Build cache	DUNE_CACHE_ROOT or XDG/dune/db	DUNE_CACHE_ROOT or DUNE_CACHE_HOME/db or XDG/dune/db
Git repo cache	XDG/dune/git-repo only	DUNE_CACHE_HOME/git-repo or XDG/dune/git-repo
Lmdb cache	XDG/dune/rev_store only	DUNE_CACHE_HOME/rev_store or XDG/dune/rev_store
Toolchains cache	XDG/dune/toolchains only	DUNE_CACHE_HOME/toolchains or XDG/dune/toolchains

[ElectreAAS]

I just rebased on main, fixed the conflicts, and addressed your comment @Leonidas-from-XIV. Assuming the CI is green, I think this is good to go!

[Leonidas-from-XIV]

I am generally not sure whether this is the right thing to do. This is because what we call "the dune cache" is a build artifact cache (and its location is controlled by DUNE_CACHE_ROOT). However the rev store is a different kind of cache and stored in a cache folder (like $XDG_CACHE_ROOT/dune/git-repo).

Thus for consistency to me it would make more sense to make DUNE_CACHE_ROOT basically XDG_CACHE_ROOT/dune and then have the dune cache in $DUNE_CACHE_ROOT/db (or so) and the rev store in DUNE_CACHE_ROOT/git-repo (or so). This will however break a number of assumptions about what's inside the folder pointed to by DUNE_CACHE_ROOT.

[ElectreAAS]

I am generally not sure whether this is the right thing to do. This is because what we call "the dune cache" is a build artifact cache (and its location is controlled by DUNE_CACHE_ROOT). However the rev store is a different kind of cache and stored in a cache folder (like $XDG_CACHE_ROOT/dune/git-repo).

Thus for consistency to me it would make more sense to make DUNE_CACHE_ROOT basically XDG_CACHE_ROOT/dune and then have the dune cache in $DUNE_CACHE_ROOT/db (or so) and the rev store in DUNE_CACHE_ROOT/git-repo (or so). This will however break a number of assumptions about what's inside the folder pointed to by DUNE_CACHE_ROOT.

I think I understand where you're coming from, however wouldn't that be breaking? If someone has set DUNE_CACHE_ROOT to foo, then overnight their build cache would move from foo/{files,meta,temp,...} to foo/db/{files,meta,temp,...} Not opposed to a breaking change per se, but I have a compromise option: introduce a new variable DUNE_CACHE_HOME (or so), have that be xdg/dune, have DUNE_CACHE_ROOT be DUNE_CACHE_HOME/db, and have the toolchains/rev-store caches in HOME instead of ROOT.

To recap:

	Current behaviour	Outdated With this PR	Breaking option	Compromise
DUNE_CACHE_HOME defaults to	doesn't exist	doesn't exist	doesn't exist	XDG/dune
DUNE_CACHE_ROOT defaults to	XDG/dune/db	XDG/dune/db	XDG/dune	DUNE_CACHE_HOME/db
Build cache	DUNE_CACHE_ROOT or XDG/dune/db	DUNE_CACHE_ROOT or XDG/dune/db	DUNE_CACHE_ROOT/db or XDG/dune/db	DUNE_CACHE_ROOT or DUNE_CACHE_HOME/db or XDG/dune/db
Rev-store cache	XDG/dune/git-repo only	DUNE_CACHE_ROOT/git-repo or XDG/dune/db/git-repo	DUNE_CACHE_ROOT/git-repo or XDG/dune/git-repo	DUNE_CACHE_HOME/git-repo or $XDG/dune/git-repo
Toolchains cache	XDG/dune/toolchains only	DUNE_CACHE_ROOT/toolchains or XDG/dune/db/toolchains	DUNE_CACHE_ROOT/toolchains or XDG/dune/toolchains	DUNE_CACHE_HOME/toolchains or XDG/dune/toolchains
Pros	status quo	it's done and more or less correct	it's actually correct and simple	allows users to put both caches in separate places, also correct
Cons	it's a bug	heterogeneous contents of cache	breaks some existing user setups	introduces a new variable which can be confusing

I hope I got that right.

I was of the opinion that the breaking option would be better before making that table, now I'm in the 'compromise' camp. However I think having more env vars can be detrimental in the long run, especially if they're vaguely named (for example, the comment above wrote XDG_CACHE_ROOT instead of XDG_CACHE_HOME). We could however in the future completely deprecate DUNE_CACHE_ROOT in favor of like DUNE_BUILD_CACHE & DUNE_PKG_CACHE to make things clearer.

Thoughts?

[Alizter]

I think the idea of introducing a DUNE_CACHE_HOME variable, appropriately documented, is sound.

Removing DUNE_CACHE_ROOT or changing its meaning will take some work unfortunately. Its probably best we don't touch it.

If DUNE_CACHE_HOME is set, then the build cache root should be in there, unless DUNE_CACHE_ROOT is set in which case it should be that.

Finer control for the other kinds of caches dune creates i.e. toolchains and the lmdb cache can be added if the need ever arises, but I am against adding that now. Those should just live in their subdirectories in DUNE_CACHE_HOME.

[Leonidas-from-XIV]

Thanks for making the table @ElectreAAS, it is a great overview! Once implemented that explanation could even go into the documentation because it explains which locations and environment variables are used when.

I was of the opinion that the breaking option would be better before making that table, now I'm in the 'compromise' camp.

Looking through the table, I also find myself nodding at the things proposed in the "compromise" column. It seems fairly reasonable and allows to set all caches to a specific location at once (and maybe git-repo should be renamed into revstore or something like that, but that's besides the point).

In particular I believe there are two no-go's for me:

• We shouldn't put other caches in the same folder as the build cache. I consider the contents of db to be "opaque", for all intents and purposes this could be a single binary file. The fact that it is a folder is more an implementation detail, just like we don't store files in the .git folder, just because its possible.
• DUNE_CACHE_ROOT has always pointed at the build cache since historically that was the only one. I think this needs to stay, to avoid breaking existing usage. We can consider printing a warning when used akin to "please update to DUNE_CACHE_HOME or if you want to set the build cache specifically use DUNE_BUILD_CACHE_ROOT" but I think there might be legitimate reasons why you'd want to put the build cache in a different place (maybe an NFS share or so).

The "compromise" solution seems to address all my concerns. I also agree with @Alizter that keeping the build cache configurable needs to stay but the other caches can for now just be controlled via DUNE_CACHE_HOME until some need arises.

[ElectreAAS]

I've went ahead and implemented the 'compromise' option. We now have DUNE_CACHE_HOME & DUNE_CACHE_ROOT, and tests should be correct by now. I've put off updating the upmost text in this PR until we're satisfied, and also put off updating the docs until a later PR, as it's not in a great state right now, and this is the occasion to fix it.

[Alizter]

I've also suggested a common point, but I think Dune_util is better than introducing another new library for this.

[ElectreAAS]

I have moved most logic from dune_cache_storage/layout into dune_util. Indeed the reasoning behind the previous state was simply "I assumed the layout was for all the caches, not just the build one". I hope the comments are clearer now.

[Alizter]

Let's summarise the cache's we have in dune for clarity and reference:

DUNE_CACHE_ROOT is the location for the shared build cache. When dune builds something with --cache enabled (the default), the build artefacts are placed in the shared cache via a digest of their actions and are hard-linked to the build. This allows them to be easily reused across multiple builds. This is what we typically mean by the "dune cache". The commands dune cache are about managing this. The variable will default to $DUNE_CACHE_HOME/db when unset and can be set independently.

Since package management we have the revision store which is a big git repo containing all the git repos we use. When you download opam or any other package, instead of cloning these repos separately we fetch the files directly to the revision store. This allows us to have a common cache of all downloaded repos. It will live in $DUNE_CACHE_HOME/git-repo.

The revision store contains another cache called the lmdb cache which is used to cache expensive lookups. We typically need to see what files are in a given repo at a given revision and what the content of those files are. This requires running some external git commands with a non-trivial amount of overhead. This will live in $DUNE_CACHE_HOME/rev_store.

Finally there is the toolchains cache where we put non-relocatable packages such as the compiler. That lives in $DUNE_CACHE_HOME/toolchains.

[Alizter]

This PR is still marked as fixing #11585. Is that still the case?

[ElectreAAS]

This PR is still marked as fixing #11585. Is that still the case?

Ah this was an edit by someone else that I didn't catch. No I don't fix that one. Adding dune cache clear --all or something is beyond the scope of this PR

[Alizter]

If you update the PR description and importantly remove the fix #asdf it should resolve it.

[Leonidas-from-XIV]

Oh, my apologies. It was me who added it, I thought that dune clean would remove whatever DUNE_CACHE_HOME is pointing to.

[Alizter]

In the meeting we discussed perhaps biting the bullet and changing the definition of DUNE_CACHE_ROOT.

Currently we are not aware of any actual breakage, but it will mean that users existing caches will be invalidated. This is already the case when we change action digests so perhaps isn't a big deal.

If we change the meaning, then a user may have DUNE_CACHE_ROOT pointed at foo/bar which means the shared build cache is directly available in foo/bar. However now it would be available in foo/bar/db. That doesn't seem so bad in practice.

This has the advantage that setup-ocaml will not need to change the variable, we don't introduce cruft and nothing is broken.

[ElectreAAS]

In the meeting we discussed perhaps biting the bullet and changing the definition of DUNE_CACHE_ROOT.

Currently we are not aware of any actual breakage, but it will mean that users existing caches will be invalidated. This is already the case when we change action digests so perhaps isn't a big deal.

If we change the meaning, then a user may have DUNE_CACHE_ROOT pointed at foo/bar which means the shared build cache is directly available in foo/bar. However now it would be available in foo/bar/db. That doesn't seem so bad in practice.

This has the advantage that setup-ocaml will not need to change the variable, we don't introduce cruft and nothing is broken.

That would mean implementing the 'Breaking' option instead of the compromise one, right? I tried my hand at doing just that (link to my branch), if needed a can make an alternative PR with that.

[Alizter]

That would mean implementing the 'Breaking' option instead of the compromise one, right? I tried my hand at doing just that (link to my branch), if needed a can make an alternative PR with that.

Yes, it would be that proposal. However we should be clear that we don't actually know if it causes any breakages other than missing old caches. I think a separate PR would be good and we can weigh the merits of both.

[ElectreAAS]

#12858 has been opened with the 'breaking' option

[ElectreAAS]

Closed in favor of #12858, which implements the 'breaking' option discussed above