From 945f88c5588c779854bcffe9cdf5bb217fa1fd22 Mon Sep 17 00:00:00 2001 From: Matthew Pickering Date: Thu, 9 Jan 2025 12:01:47 +0000 Subject: [PATCH] cabal-install: Be less eager to configure external programs 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. A surgical way to fix this is to avoid configuring the programs, and manually adding back the builtinPrograms to the ProgramDb, so the ProgramDb returned by configureCompiler always contains all the unconfigured programs. The testcase is not so easy to write because * The bug only surfaces when the build-tool you are depending on is known (ie alex, happy etc) * But then it is tricky to write a test, as we can't depend on the known tools or bundle the source for them. * So we create a fake "alex", which cabal then invokes on a fake ".x" file. This is maybe a bit fragile if the way cabal invokes alex changes in future, but then the test can be modified as well. Also see #2241 and #9840 Fixes #10692 --- .../src/Distribution/Client/ProjectPlanning.hs | 14 ++++++++------ .../BuildToolDependsExternal/cabal.project | 2 ++ .../BuildToolDependsExternal/client/Hello.x | 3 +++ .../client/client.cabal | 13 +++++++++++++ .../pre-proc/MyCustomPreprocessor.hs | 13 +++++++++++++ .../pre-proc/pre-proc.cabal | 17 +++++++++++++++++ .../BuildToolDependsExternal/scripts/alex | 4 ++++ .../BuildToolDependsExternal/setup.out | 14 ++++++++++++++ .../BuildToolDependsExternal/setup.test.hs | 10 ++++++++++ .../PackageTests/ExtraProgPath/setup.out | 6 ++++-- changelog.d/pr-10731 | 12 ++++++++++++ 11 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/cabal.project create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/client/Hello.x create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/client/client.cabal create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/MyCustomPreprocessor.hs create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/pre-proc.cabal create mode 100755 cabal-testsuite/PackageTests/BuildToolDependsExternal/scripts/alex create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.out create mode 100644 cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.test.hs create mode 100644 changelog.d/pr-10731 diff --git a/cabal-install/src/Distribution/Client/ProjectPlanning.hs b/cabal-install/src/Distribution/Client/ProjectPlanning.hs index 582e526af53..64c3001a21b 100644 --- a/cabal-install/src/Distribution/Client/ProjectPlanning.hs +++ b/cabal-install/src/Distribution/Client/ProjectPlanning.hs @@ -475,7 +475,7 @@ configureCompiler let fileMonitorCompiler = newFileMonitor $ distProjectCacheFile "compiler" progsearchpath <- liftIO $ getSystemSearchPath - rerunIfChanged + (recomp_comp, recomp_plat, recomp_progdb) <- rerunIfChanged verbosity fileMonitorCompiler ( hcFlavor @@ -507,12 +507,14 @@ 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'' + return (comp, plat, progdb'') - return (comp, plat, finalProgDb) + -- If the above section is not recomputed, ProgramDB will be read from the + -- cache and lacking the knowledge about unconfigured programs. + -- Therefore the builtinPrograms are added here (since we started from the defaultProgramDb, + -- this is the correct list). + let restored_progdb = restoreProgramDb builtinPrograms recomp_progdb + return (recomp_comp, recomp_plat, restored_progdb) where hcFlavor = flagToMaybe projectConfigHcFlavor hcPath = flagToMaybe projectConfigHcPath diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/cabal.project b/cabal-testsuite/PackageTests/BuildToolDependsExternal/cabal.project new file mode 100644 index 00000000000..359fdc811bd --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/cabal.project @@ -0,0 +1,2 @@ +packages: client +optional-packages: pre-proc diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/client/Hello.x b/cabal-testsuite/PackageTests/BuildToolDependsExternal/client/Hello.x new file mode 100644 index 00000000000..28f179506bf --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/client/Hello.x @@ -0,0 +1,3 @@ +module Main where + +main = print 0 diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/client/client.cabal b/cabal-testsuite/PackageTests/BuildToolDependsExternal/client/client.cabal new file mode 100644 index 00000000000..a13e99b682a --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/client/client.cabal @@ -0,0 +1,13 @@ +name: client +version: 0.1.0.0 +synopsis: Checks build-tool-depends are put in PATH +license: BSD3 +category: Testing +build-type: Simple +cabal-version: >=1.10 + +executable hello-world + main-is: Hello.hs + build-depends: base + build-tool-depends: pre-proc:alex + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/MyCustomPreprocessor.hs b/cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/MyCustomPreprocessor.hs new file mode 100644 index 00000000000..bcc4af46d1c --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/MyCustomPreprocessor.hs @@ -0,0 +1,13 @@ +module Main where + +import System.Environment +import System.IO + +-- This is a "fake" version of alex, so it should take the command line arguments +-- as alex. +main :: IO () +main = do + (_:"-o":target:source:_) <- getArgs + let f '0' = '1' + f c = c + writeFile target . map f =<< readFile source diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/pre-proc.cabal b/cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/pre-proc.cabal new file mode 100644 index 00000000000..5724eb58296 --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/pre-proc/pre-proc.cabal @@ -0,0 +1,17 @@ +name: pre-proc +version: 999.999.999 +synopsis: Checks build-tool-depends are put in PATH +license: BSD3 +category: Testing +build-type: Simple +cabal-version: >=1.10 + +executable alex + main-is: MyCustomPreprocessor.hs + build-depends: base, directory + default-language: Haskell2010 + +executable bad-do-not-build-me + main-is: MyMissingPreprocessor.hs + build-depends: base, directory + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/scripts/alex b/cabal-testsuite/PackageTests/BuildToolDependsExternal/scripts/alex new file mode 100755 index 00000000000..27bdc4e880d --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/scripts/alex @@ -0,0 +1,4 @@ +#! /usr/bin/env bash + +echo "I am not the alex you are looking for" +exit 1 diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.out b/cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.out new file mode 100644 index 00000000000..0600824409d --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.out @@ -0,0 +1,14 @@ +# cabal v2-build +Configuration is affected by the following files: +- cabal.project +Resolving dependencies... +Build profile: -w ghc- -O1 +In order, the following will be built: + - pre-proc-999.999.999 (exe:alex) (first run) + - client-0.1.0.0 (exe:hello-world) (first run) +Configuring executable 'alex' for pre-proc-999.999.999... +Preprocessing executable 'alex' for pre-proc-999.999.999... +Building executable 'alex' for pre-proc-999.999.999... +Configuring executable 'hello-world' for client-0.1.0.0... +Preprocessing executable 'hello-world' for client-0.1.0.0... +Building executable 'hello-world' for client-0.1.0.0... diff --git a/cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.test.hs b/cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.test.hs new file mode 100644 index 00000000000..7eef3cd4870 --- /dev/null +++ b/cabal-testsuite/PackageTests/BuildToolDependsExternal/setup.test.hs @@ -0,0 +1,10 @@ +import Test.Cabal.Prelude +-- Test build-tool-depends isn't influenced by PATH +-- This test fails with the message +-- +Warning: cannot determine version of scripts/alex : +-- "" +-- If the scripts/alex script is executed rather than the one from the correct package. + +main = cabalTest $ do + env <- getTestEnv + addToPath (testTmpDir env "scripts/") $ cabal "v2-build" ["client"] diff --git a/cabal-testsuite/PackageTests/ExtraProgPath/setup.out b/cabal-testsuite/PackageTests/ExtraProgPath/setup.out index ed22f251035..13729def443 100644 --- a/cabal-testsuite/PackageTests/ExtraProgPath/setup.out +++ b/cabal-testsuite/PackageTests/ExtraProgPath/setup.out @@ -1,8 +1,10 @@ # cabal v2-build -Warning: cannot determine version of /pkg-config : -"" Configuration is affected by the following files: - cabal.project +Warning: cannot determine version of /pkg-config : +"" +Warning: cannot determine version of /pkg-config : +"" Resolving dependencies... Error: [Cabal-7107] Could not resolve dependencies: diff --git a/changelog.d/pr-10731 b/changelog.d/pr-10731 new file mode 100644 index 00000000000..74f909f2789 --- /dev/null +++ b/changelog.d/pr-10731 @@ -0,0 +1,12 @@ +synopsis: Fix regression where build-tool-depends are not used +packages: cabal-install +prs: #10731 + +description: { + +Fix a bug where an executable +used from your system rather than one built and provided by cabal-install would +be used to satisfy a build-tool-depends field. + +} +