ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.6k stars 395 forks source link

[Pkg] Enable cache by default #10729

Closed maiste closed 1 week ago

maiste commented 1 month ago

For the Dune Developer Preview program, we would like to have the cache enabled by default to save bandwidth when you are trying to install packages. This modification would respect the following:

  1. If there is a value in the configuration, the CLI or in the environment, we want to make sure this one is used.
  2. Otherwise, the cache would be enabled by default at configure time only for the developer preview executable.

A solution to this would be to parametrize the default solution using some flags and enabled them (it?) at configure time.

The first proposition can be found on #10710

maiste commented 1 month ago

@rgrinberg I opened this PR on @emillon suggestion so we can discuss a possible solution for this.

The problem lies in that most users do not have such a configuration. Moreover, most project contain user rules of the form: (rule ..) That are absolutely not safe to cache. Therefore, flipping the default like is done in this PR is highly likely to cause a lot of unintended breakage.

IIUC the problem, for many users, their rules depend on the fact that they are not caching anything? As the goal of the PRs it to avoid the package to be download multiple times, could a specific variable for this cache be a solution?

rgrinberg commented 1 month ago

The end goal is correct, but we need to adjust the plan on how to get there. So what we'd like is the ability to cache only package rules. To get there, we can:

  1. Introduce a shared cache toggle in build rules themselves (we have one internally called can_go_in_shared_cache)
  2. Set this toggle to false for all rules by default
  3. Set this toggle to true for package rules
  4. Enable the shared cache
  5. Introduce a way to change the default for normal rules to get back the old behavior for users that want to cache their own rules
maiste commented 1 month ago

Ok, IIUC, the idea is to split the shared cache toggle, in multiple toggles, so we can have specific caches per rule?

rgrinberg commented 1 month ago

We'd like the following behavior:

  1. DUNE_CACHE=enabled everything is cached like the current behavior
  2. DUNE_CACHE=disabled nothing is cached
  3. DUNE_CACHE is not set. Only the package rules are cached.

A per rule toggle would be a reasonable way to implement this

maiste commented 1 month ago

I see. If I understand correctly how it would work, it would mean rewrite the shared cache from one unique cache to a split version of it?

emillon commented 1 month ago

I see. If I understand correctly how it would work, it would mean rewrite the shared cache from one unique cache to a split version of it?

I don't think so. You keep a single shared cache, but you change the logic to determine if a rule can use it.

Leonidas-from-XIV commented 1 week ago

Solved by #10710 and #10850.