Skip to content

Commit

Permalink
Merge pull request #9967 from sheaf/prog-db-configure
Browse files Browse the repository at this point in the history 
Be more careful in handling unconfigured programs in the program database
  • Loading branch information
@mergify
mergify[bot] authored Jun 15, 2024
2 parents 355067d + 6ac9dfb commit 5ea22e2
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 65 deletions.
118 changes: 70 additions & 48 deletions Cabal/src/Distribution/Simple/Configure.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2210,55 +2210,77 @@ configureRequiredProgram
verbosity
progdb
(LegacyExeDependency progName verRange) =
case lookupKnownProgram progName progdb of
Nothing ->
-- Try to configure it as a 'simpleProgram' automatically
--
-- There's a bit of a story behind this line. In old versions
-- of Cabal, there were only internal build-tools dependencies. So the
-- behavior in this case was:
--
-- - If a build-tool dependency was internal, don't do
-- any checking.
--
-- - If it was external, call 'configureRequiredProgram' to
-- "configure" the executable. In particular, if
-- the program was not "known" (present in 'ProgramDb'),
-- then we would just error. This was fine, because
-- the only way a program could be executed from 'ProgramDb'
-- is if some library code from Cabal actually called it,
-- and the pre-existing Cabal code only calls known
-- programs from 'defaultProgramDb', and so if it
-- is calling something else, you have a Custom setup
-- script, and in that case you are expected to register
-- the program you want to call in the ProgramDb.
--
-- OK, so that was fine, until I (ezyang, in 2016) refactored
-- Cabal to support per-component builds. In this case, what
-- was previously an internal build-tool dependency now became
-- an external one, and now previously "internal" dependencies
-- are now external. But these are permitted to exist even
-- when they are not previously configured (something that
-- can only occur by a Custom script.)
case lookupProgramByName progName progdb of
Just prog ->
-- If the program has already been configured, use it
-- (as long as the version is compatible).
--
-- So, I decided, "Fine, let's just accept these in any
-- case." Thus this line. The alternative would have been to
-- somehow detect when a build-tools dependency was "internal" (by
-- looking at the unflattened package description) but this
-- would also be incompatible with future work to support
-- external executable dependencies: we definitely cannot
-- assume they will be preinitialized in the 'ProgramDb'.
configureProgram verbosity (simpleProgram progName) progdb
Just prog
-- requireProgramVersion always requires the program have a version
-- but if the user says "build-depends: foo" ie no version constraint
-- then we should not fail if we cannot discover the program version.
| verRange == anyVersion -> do
(_, progdb') <- requireProgram verbosity prog progdb
return progdb'
| otherwise -> do
(_, _, progdb') <- requireProgramVersion verbosity prog verRange progdb
return progdb'
-- Not doing so means falling back to the "simpleProgram" path below,
-- which might fail if the program has custom logic to find a version
-- (such as hsc2hs).
let loc = locationPath $ programLocation prog
in case programVersion prog of
Nothing
| verRange == anyVersion ->
return progdb
| otherwise ->
dieWithException verbosity $!
UnknownVersionDb (programId prog) verRange loc
Just version
| withinRange version verRange ->
return progdb
| otherwise ->
dieWithException verbosity $!
BadVersionDb (programId prog) version verRange loc
Nothing ->
-- Otherwise, try to configure it as a 'simpleProgram' automatically
case lookupKnownProgram progName progdb of
Nothing ->
-- There's a bit of a story behind this line. In old versions
-- of Cabal, there were only internal build-tools dependencies. So the
-- behavior in this case was:
--
-- - If a build-tool dependency was internal, don't do
-- any checking.
--
-- - If it was external, call 'configureRequiredProgram' to
-- "configure" the executable. In particular, if
-- the program was not "known" (present in 'ProgramDb'),
-- then we would just error. This was fine, because
-- the only way a program could be executed from 'ProgramDb'
-- is if some library code from Cabal actually called it,
-- and the pre-existing Cabal code only calls known
-- programs from 'defaultProgramDb', and so if it
-- is calling something else, you have a Custom setup
-- script, and in that case you are expected to register
-- the program you want to call in the ProgramDb.
--
-- OK, so that was fine, until I (ezyang, in 2016) refactored
-- Cabal to support per-component builds. In this case, what
-- was previously an internal build-tool dependency now became
-- an external one, and now previously "internal" dependencies
-- are now external. But these are permitted to exist even
-- when they are not previously configured (something that
-- can only occur by a Custom script.)
--
-- So, I decided, "Fine, let's just accept these in any
-- case." Thus this line. The alternative would have been to
-- somehow detect when a build-tools dependency was "internal" (by
-- looking at the unflattened package description) but this
-- would also be incompatible with future work to support
-- external executable dependencies: we definitely cannot
-- assume they will be preinitialized in the 'ProgramDb'.
configureProgram verbosity (simpleProgram progName) progdb
Just prog
-- requireProgramVersion always requires the program have a version
-- but if the user says "build-depends: foo" ie no version constraint
-- then we should not fail if we cannot discover the program version.
| verRange == anyVersion -> do
(_, progdb') <- requireProgram verbosity prog progdb
return progdb'
| otherwise -> do
(_, _, progdb') <- requireProgramVersion verbosity prog verRange progdb
return progdb'

-- -----------------------------------------------------------------------------
-- Configuring pkg-config package dependencies
Expand Down
12 changes: 2 additions & 10 deletions Cabal/src/Distribution/Simple/Program/Builtin.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,21 +226,13 @@ hpcProgram =
-- during the configure phase.
haskellSuiteProgram :: Program
haskellSuiteProgram =
(simpleProgram "haskell-suite")
{ -- pretend that the program exists, otherwise it won't be in the
-- "configured" state
programFindLocation = \_verbosity _searchPath ->
return $ Just ("haskell-suite-dummy-location", [])
}
simpleProgram "haskell-suite"

-- This represent a haskell-suite package manager. See the comments for
-- haskellSuiteProgram.
haskellSuitePkgProgram :: Program
haskellSuitePkgProgram =
(simpleProgram "haskell-suite-pkg")
{ programFindLocation = \_verbosity _searchPath ->
return $ Just ("haskell-suite-pkg-dummy-location", [])
}
simpleProgram "haskell-suite-pkg"

happyProgram :: Program
happyProgram =
Expand Down
9 changes: 7 additions & 2 deletions cabal-install/src/Distribution/Client/ProjectPlanning.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ configureCompiler
let extraPath = fromNubList packageConfigProgramPathExtra
progdb <- liftIO $ prependProgramSearchPath verbosity extraPath [] defaultProgramDb
let progdb' = userSpecifyPaths (Map.toList (getMapLast packageConfigProgramPaths)) progdb
result@(_, _, progdb'') <-
(comp, plat, progdb'') <-
liftIO $
Cabal.configCompilerEx
hcFlavor
Expand All @@ -500,7 +500,12 @@ configureCompiler
-- programs it cares about, and those are the ones we monitor here.
monitorFiles (programsMonitorFiles progdb'')

return result
-- 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, finalProgDb)
where
hcFlavor = flagToMaybe projectConfigHcFlavor
hcPath = flagToMaybe projectConfigHcPath
Expand Down
15 changes: 12 additions & 3 deletions cabal-install/src/Distribution/Client/SetupWrapper.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ import Distribution.Simple.Program
, runDbProgramCwd
)
import Distribution.Simple.Program.Db
( prependProgramSearchPath
( configureAllKnownPrograms
, prependProgramSearchPath
, progOverrideEnv
)
import Distribution.Simple.Program.Find
Expand Down Expand Up @@ -1035,11 +1036,19 @@ getExternalSetupMethod verbosity options pkg bt = do
createDirectoryIfMissingVerbose verbosity True setupCacheDir
installExecutableFile verbosity src cachedSetupProgFile
-- Do not strip if we're using GHCJS, since the result may be a script
when (maybe True ((/= GHCJS) . compilerFlavor) $ useCompiler options') $
when (maybe True ((/= GHCJS) . compilerFlavor) $ useCompiler options') $ do
-- Add the relevant PATH overrides for the package to the
-- program database.
setupProgDb
<- prependProgramSearchPath verbosity
(useExtraPathEnv options)
(useExtraEnvOverrides options)
(useProgramDb options')
>>= configureAllKnownPrograms verbosity
Strip.stripExe
verbosity
platform
(useProgramDb options')
setupProgDb
cachedSetupProgFile
return cachedSetupProgFile
where
Expand Down
2 changes: 0 additions & 2 deletions cabal-testsuite/PackageTests/ExtraProgPath/setup.out
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# cabal v2-build
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:
Expand Down

0 comments on commit 5ea22e2

Please sign in to comment.