Revert "cabal-install configureCompiler: configure progdb"

This reverts commit 8bdda9c.

In configureCompiler the call to configureAllKnownPrograms was too
eager. When called it selected the version of tools from PATH (such as
alex), and then when a package was configured these tools were passed
using `--with-alex` options to configure, which meant that the
build-tool-depends versions were not used. (See #10692)

Why was this call introduced in the first place? Because
configureCompiler would a different result depending on whether:

* It is run for the first time, the `ProgramDb` will contain
  unconfigured programs.
* It is run subsequently, `ProgramDb` is read from disk, it does not
  contain unconfigured programs.

Reverting this commit rexposes the bug that the serialised ProgramDb
will not contain UnconfiguredPrograms (in the case where reconfiguring
is avoided).

However, there are no code paths which require this logic in
`cabal-install` currently. The configuration phase happens again each
time that `Cabal` is called, with a populated `ProgramDb`. We will
fix this before the next major `cabal-install` release, but it would not
be suitable to backport.

In the future we will fix this properly by refactoring
`configureCompiler` so that `ProgramDb` is not serialised. The general
approach will be to make `configCompilerEx` return a pair of configured
programs (`ghc` and `ghc-pkg`) and expose an additional function from
`Cabal` which uses these two programs to perform the modifications to
the `ProgramDb` which `configCompilerEx` performs.

Also see #2238 and #9840

Fixes #10692

Author: mpickering

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

@@ -484,6 +484,7 @@ configureCompiler
     let fileMonitorCompiler = newFileMonitor $ distProjectCacheFile "compiler"
 
     progsearchpath <- liftIO $ getSystemSearchPath
+
     rerunIfChanged
       verbosity
       fileMonitorCompiler
@@ -499,7 +500,7 @@ configureCompiler
         let extraPath = fromNubList packageConfigProgramPathExtra
         progdb <- liftIO $ prependProgramSearchPath verbosity extraPath [] defaultProgramDb
         let progdb' = userSpecifyPaths (Map.toList (getMapLast packageConfigProgramPaths)) progdb
-        (comp, plat, progdb'') <-
+        result@(_, _, progdb'') <-
           liftIO $
             Cabal.configCompilerEx
               hcFlavor
@@ -516,17 +517,55 @@ configureCompiler
         -- programs it cares about, and those are the ones we monitor here.
         monitorFiles (programsMonitorFiles progdb'')
 
-        -- Configure the unconfigured programs in the program database,
-        -- as we can't serialise unconfigured programs.
-        -- See also #2241 and #9840.
-        finalProgDb <- liftIO $ configureAllKnownPrograms verbosity progdb''
+        -- Note: There is currently a bug here: we are dropping unconfigured
+        -- programs from the 'ProgramDb' when we re-use the cache created by
+        -- 'rerunIfChanged'.
+        --
+        -- See Note [Caching the result of configuring the compiler]
 
-        return (comp, plat, finalProgDb)
+        return result
     where
       hcFlavor = flagToMaybe projectConfigHcFlavor
       hcPath = flagToMaybe projectConfigHcPath
       hcPkg = flagToMaybe projectConfigHcPkg
 
+{- Note [Caching the result of configuring the compiler]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+We can't straightforwardly cache anything that contains a 'ProgramDb', because
+the 'Binary' instance for 'ProgramDb' discards all unconfigured programs.
+See that instance, as well as 'restoreProgramDb', for a few more details.
+
+This means that if we try to cache the result of configuring the compiler (which
+contains a 'ProgramDb'):
+
+ - On the first run, we will obtain a 'ProgramDb' which may contain several
+   unconfigured programs. In particular, configuring GHC will add tools such
+   as `ar` and `ld` as unconfigured programs to the 'ProgramDb', with custom
+   logic for finding their location based on the location of the GHC binary.
+ - On subsequent runs, if we use the cache created by 'rerunIfChanged', we will
+   deserialise the 'ProgramDb' from disk, which means it won't include any
+   unconfigured programs, which might mean we are unable to find 'ar' or 'ld'.
+
+This is not currently a huge problem because, in the Cabal library, we eagerly
+re-run the configureCompiler step (thus recovering any lost information), but
+this is wasted work that we should stop doing in Cabal, given that cabal-install
+has already figured out all the necessary information about the compiler.
+
+To fix this bug, we can't simply eagerly configure all unconfigured programs,
+as was originally attempted, for a couple of reasons:
+
+ - it does more work than necessary, by configuring programs that we may not
+   end up needing,
+ - it means that we prioritise system executables for built-in build tools
+   (such as `alex` and `happy`), instead of using the proper version for a
+   package or package component, as specified by a `build-tool-depends` stanza
+   or by package-level `extra-prog-path` arguments.
+   This lead to bug reports #10633 and #10692.
+
+See #9840 for more information about the problems surrounding the lossly
+Binary ProgramDb instance.
+-}
+
 ------------------------------------------------------------------------------
 
 -- * Deciding what to do: making an 'ElaboratedInstallPlan'

cabal-testsuite/PackageTests/ExtraProgPath/setup.out

@@ -1,8 +1,10 @@
 # cabal v2-build
-Warning: cannot determine version of <ROOT>/pkg-config :
-""
 Configuration is affected by the following files:
 - cabal.project
+Warning: cannot determine version of <ROOT>/pkg-config :
+""
+Warning: cannot determine version of <ROOT>/pkg-config :
+""
 Resolving dependencies...
 Error: [Cabal-7107]
 Could not resolve dependencies:

changelog.d/pr-10731

@@ -0,0 +1,4 @@
+synopsis: Fix regression where build-tool-depends are not used
+packages: cabal-install
+prs: #10731
+issues: #10633 #10692