Save disk space by only caching dev packages into the internal 'sources' directory

[kit-ty-kate]

Fixes #5448
Fixes #4056
Queued on #6912

The .opam-switch/sources directory currently stores the source of every installed packages in the following format:

• <pkgname>: are for pinned packages
• <pkgname>.<version>: are for the rest

In #2825 (opam 2.0.0~beta) the change from packages.dev to sources was introduced to support (as i understand it) non-pinned dev packages (e.g. ocaml-variants.5.4.0+trunk). This is a good change but this also had for side-effect to store the source of every packages regardless of whether it's a dev package or not, which i think isn't a good change given that it duplicates data that's already in the archive cache that could instead be extracted very easily (the cost of extraction vs. copy is negligible).
As a side note, extracting the sources from their tarball would be way faster on Windows that extraction + copy for the same reason detailed in #5741

As it currently stands this PR is a WIP. It compiles but still has many unsolved issues (in particular it breaks caching for non-pinned dev packages) and the reftests fail, but i'm opening it anyway to show the rough area of the code that needs to change and in case anyone wants to take over this work while i'm doing something else.

Post-WIP edit: Further improvements can be made by extracting the tarball directly into the build directory and would also f.i.x #6693, but i think it is simpler to do that separately in a future PR.

[kit-ty-kate]

As a demonstration, opam clean --switch-cleanup --all which primarily cleans up these unnecessary directories, just removed 20GB of disk space on my main machine.

[kit-ty-kate]

xref live test results: #6906 (comment)

[rjbou]

On the idea, lgtm, it is a good enhancement for disk usage saving. On the implementation, i don't think it should be done that way. The source_dir function should keep returning the sources directory, and when we want to bypass that source directory to extract directly on the build dir, it should be done directly and explicitly.

[rjbou]

How does it affects dirtrack added ?
With full debug, the diff with master is

-@@ -120,17 +122,106 @@ added: [
+@@ -120,17 +122,103 @@ added: [
  ### unset OPAMDEBUGSECTIONS
  ### OPAMDEBUG=-5
  ### opam reinstall dot --dry-run
@@ -94,10 +94,6 @@
  ACTION                          download_package: dot.~dev
 PARALLEL                        Job 62929499 finished
 PARALLEL                        Starting job 798933370 (worker 1/15 -/1 -/1): build dot.~dev
-SYSTEM                          rmdir ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev
-SYSTEM                          rm ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev/dot.install
-SYSTEM                          rm ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev/a-file
-SYSTEM                          mkdir ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev
  ACTION                          prepare_package_source: dot.~dev at ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev
 PARALLEL                        Job 798933370 finished
 PARALLEL                        Starting job 743026333 (worker -/15 1/1 -/1): remove dot.~dev
@@ -107,11 +103,12 @@
  -> removed   dot.~dev
 PARALLEL                        Job 743026333 finished
 PARALLEL                        Starting job 699714656 (worker -/15 1/1 -/1): install dot.~dev
-FILE(.install)                  Cannot find ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev/dot.install
-TRACK                           before install: 0 elements scanned in 0.000s
+FILE(.install)                  Read ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev/dot.install in 0.000s
+FILE(.install)                  Wrote ${BASEDIR}/OPAM/inst/.opam-switch/install/dot.install atomically in 0.000s
+TRACK                           before install: 3 elements scanned in 0.000s
  Installing dot.~dev.
-[ERROR] tracked after {}
- TRACK                           after install: 0 elements, 0 added, scanned in 0.000s
+[ERROR] tracked after { share/dot/a-file:F:12fc204edeae5b57713c5ad7dcb97d39, share/dot:D, share:D }
+ TRACK                           after install: 3 elements, 0 added, scanned in 0.000s
  -> installed dot.~dev
 FILE(.config)                   Cannot find ${BASEDIR}/OPAM/inst/.opam-switch/config/dot.config
 PARALLEL                        Job 699714656 finished

[kit-ty-kate]

Thanks for the review. Turns out part of design had bad assumptions and extracting directly into build broke a lot of invariants that wasn't in the path that i tested for.

I redid the design and now the testsuite look good. The change is that we now use the sources directory as temporary directory that will be moved (they're pretty much guaranteed to be on the same partition so the cost of moving is none) to the build directory.

Depending on your personal preference i've pushed two commits. The first one changes the type of OpamSwitchState.source_dir to return whether or not it is meant to be permanent. However this value isn't used most of the time and costs an extra call to OpamSwitchState.is_dev_package so i also pushed a second commit to put back the old return type but add an extra is_source_dir_temporary for when the information is really necessary.

So depending on your preference, feel free to squash the second one or remove it, i'm happy with both solutions.

[rjbou]

I like the second solution, it's more clear, and we don't have extra code (extra snd).

[kit-ty-kate]

Ignoring the macOS failure (#7037)

[kit-ty-kate]

Noteworthy: the opam install lot-io-source lot-io-source2 benchmark show a 24% speedup 🎉