Cache the solver's pkg-config calls

[georgefst]

A proof-of-concept fix for #8930. I will ultimately need some assistance from an existing maintainer in order to get this over the line, but it's maybe too early even for that.

Note that this totally overwrites the first commit of #9134. As discussed several times in that thread, the batch call seems more trouble than it's worth. Since this PR caches on a per-package basis there really seems no reason to keep it.

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog. Try each pkg-config query separatedly #9134's changelog entry should perhaps just be removed.
• Various TODOs in source
• Tests?

EDIT: There's been a lot more discussion on #8930 about whether this is even the right approach. I won't be working on this further until there's more of a consensus.

[gbaz]

I would advise you not modify the readPkgConfigDb function at all, but instead its call-site in cabal-install:

cabal/cabal-install/src/Distribution/Client/ProjectPlanning.hs

Line 1045 in a260cde

liftIO $ readPkgConfigDb verbosity progdb

In that call-site we are already in the Rebuild monad and so can make use of existing caching combinators and tooling instead of ad-hoc creating new ones.

This leaves the Setup.hs and v1- paths untouched, but that should be fine, imho.

[andreabedini]

@georgefst, I appreciate you took the initiative on this. Note that we already have the infrastructure to cache results and refresh them based on input keys and/or file changes (see https://hackage.haskell.org/package/cabal-install-3.10.1.0/docs/Distribution-Client-RebuildMonad.html).

Have you tried the approach I suggest in #8930 (comment)?

Leaving along cabal v1 commands and cabal-install-solver (as @gbaz suggests); readPkgConfigDb is called only in rebuildInstallPlan, so we can add our caching there.

diff --git a/cabal-install/src/Distribution/Client/ProjectPlanning.hs b/cabal-install/src/Distribution/Client/ProjectPlanning.hs
index 1b92a8aa5..90a549089 100644
--- a/cabal-install/src/Distribution/Client/ProjectPlanning.hs
+++ b/cabal-install/src/Distribution/Client/ProjectPlanning.hs
@@ -724,7 +724,11 @@ rebuildInstallPlan
                   withRepoCtx
                   (solverSettingIndexState solverSettings)
                   (solverSettingActiveRepos solverSettings)
-              pkgConfigDB <- getPkgConfigDb verbosity progdb
+              pkgConfigDB <-
+                liftIO $
+                  runRebuild "~/.cache/cabal" $
+                    rerunIfChanged verbosity (newFileMonitor "pkg-config.cache") () $
+                      getPkgConfigDb verbosity progdb

               -- TODO: [code cleanup] it'd be better if the Compiler contained the
               -- ConfiguredPrograms that it needs, rather than relying on the progdb

The path ~/.cache/cabal can be defined through a new field in cabalDirLayout (argument to fileMonitorSolverPlan) that defines the location for global caches. This will be similar to how fileMonitorSolverPlan is defined in terms of distProjectCacheFile just above.

getPkgConfigDb already sets up monitoring for pkg-config paths so we can assume that works ok. I don't think there is any explicit input so I put () as key to rerunIfChanged.

Maybe I am mistaken and this approach has some flaws I don't see?

Note: I am actually quite unsure about how Rebuild works. The argument to runRebuild is some sort of root directory but newFileMonitor does not seem to use it? pkg-config.cache is written to the current directory.

Note 2: I think this will look for changes in pkg-config database (i.e. the files monitored by getPkgConfigDb) only if something earlier has changed, thus triggering this IO. What happens if we do rerunIfChanged verbosity (newFileMonitor "pkg-config.cache") in the scope of the top level runRebuild?

Note 3: what happens if the monitor file gets shared between different cabal versions?

[georgefst]

Thanks @gbaz and @andreabedini! I'll try to rewrite this with your suggestions in mind.

I do think that even if we make no other changes to readPkgConfigDb, we should just drop the bulk call, since it often fails anyway, and with this change we'd almost always be reading the DB incrementally. But it might be necessary to collect timings from a variety of machines in order to weigh up the pros and cons. In particular, it's possible that the slowdown on first install for some users would be significant enough to justify retaining it.

(I've edited this and the comment below after making a major mistake of confusing the --list-all call, which is fast, with the single---modversion-with-all-packages call, which can fail and/or be slow.)

[georgefst]

Actually it's unclear to me how we can really avoid modifying readPkgConfigDb. As I pointed out in #8930 (comment), we really want to avoid the bulk call at all costs even in the absence of broken packages.

[andreabedini]

@georgefst Just checking, are you still interested in finishing this? As far as I can tell #9422 achieves the same goal.

[georgefst]

I guess not. #9422 looks like the pragmatic option. I'll return to #9478 if I find it to be significantly less than ideal in practice.