Skip to content

Commit 525ac46

Browse files
sheafsheaf
committed
configureRequiredProgram: take care if program already configured
In configureRequiredProgram, we would unconditionally configure the program, even if it was configured already. To explain why this is problematic: - The programs we try to configure in this function are those that arise from a "build-tool-depends" field of a package. - If the program is not in the "unconfigured" state, we would unconditionally treat it as a simpleProgram. This means that if such a program is builtin but is not a simpleProgram, we might fail to configure it properly. This problem arises when we configure programs more eagerly: when, in the past, we might have gone looking up an unconfigured program and successfully configured it, now we might find the program is already configured. One example in which this would cause an issue is when we have build-tool-depends: hsc2hs If we end up re-configuring hsc2hs in configureRequiredProgram, we would treat it as a simple program, which would cause us to be unable to determine its version.
1 parent 97cd4c5 commit 525ac46

File tree

1 file changed

+70
-48
lines changed

1 file changed

+70
-48
lines changed

‎Cabal/src/Distribution/Simple/Configure.hs‎

Lines changed: 70 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,55 +2210,77 @@ configureRequiredProgram
22102210
verbosity
22112211
progdb
22122212
(LegacyExeDependency progName verRange) =
2213-
case lookupKnownProgram progName progdb of
2214-
Nothing ->
2215-
-- Try to configure it as a 'simpleProgram' automatically
2216-
--
2217-
-- There's a bit of a story behind this line. In old versions
2218-
-- of Cabal, there were only internal build-tools dependencies. So the
2219-
-- behavior in this case was:
2220-
--
2221-
-- - If a build-tool dependency was internal, don't do
2222-
-- any checking.
2223-
--
2224-
-- - If it was external, call 'configureRequiredProgram' to
2225-
-- "configure" the executable. In particular, if
2226-
-- the program was not "known" (present in 'ProgramDb'),
2227-
-- then we would just error. This was fine, because
2228-
-- the only way a program could be executed from 'ProgramDb'
2229-
-- is if some library code from Cabal actually called it,
2230-
-- and the pre-existing Cabal code only calls known
2231-
-- programs from 'defaultProgramDb', and so if it
2232-
-- is calling something else, you have a Custom setup
2233-
-- script, and in that case you are expected to register
2234-
-- the program you want to call in the ProgramDb.
2235-
--
2236-
-- OK, so that was fine, until I (ezyang, in 2016) refactored
2237-
-- Cabal to support per-component builds. In this case, what
2238-
-- was previously an internal build-tool dependency now became
2239-
-- an external one, and now previously "internal" dependencies
2240-
-- are now external. But these are permitted to exist even
2241-
-- when they are not previously configured (something that
2242-
-- can only occur by a Custom script.)
2213+
case lookupProgramByName progName progdb of
2214+
Just prog ->
2215+
-- If the program has already been configured, use it
2216+
-- (as long as the version is compatible).
22432217
--
2244-
-- So, I decided, "Fine, let's just accept these in any
2245-
-- case." Thus this line. The alternative would have been to
2246-
-- somehow detect when a build-tools dependency was "internal" (by
2247-
-- looking at the unflattened package description) but this
2248-
-- would also be incompatible with future work to support
2249-
-- external executable dependencies: we definitely cannot
2250-
-- assume they will be preinitialized in the 'ProgramDb'.
2251-
configureProgram verbosity (simpleProgram progName) progdb
2252-
Just prog
2253-
-- requireProgramVersion always requires the program have a version
2254-
-- but if the user says "build-depends: foo" ie no version constraint
2255-
-- then we should not fail if we cannot discover the program version.
2256-
| verRange == anyVersion -> do
2257-
(_, progdb') <- requireProgram verbosity prog progdb
2258-
return progdb'
2259-
| otherwise -> do
2260-
(_, _, progdb') <- requireProgramVersion verbosity prog verRange progdb
2261-
return progdb'
2218+
-- Not doing so means falling back to the "simpleProgram" path below,
2219+
-- which might fail if the program has custom logic to find a version
2220+
-- (such as hsc2hs).
2221+
let loc = locationPath $ programLocation prog
2222+
in case programVersion prog of
2223+
Nothing
2224+
| verRange == anyVersion ->
2225+
return progdb
2226+
| otherwise ->
2227+
dieWithException verbosity $!
2228+
UnknownVersionDb (programId prog) verRange loc
2229+
Just version
2230+
| withinRange version verRange ->
2231+
return progdb
2232+
| otherwise ->
2233+
dieWithException verbosity $!
2234+
BadVersionDb (programId prog) version verRange loc
2235+
Nothing ->
2236+
-- Otherwise, try to configure it as a 'simpleProgram' automatically
2237+
case lookupKnownProgram progName progdb of
2238+
Nothing ->
2239+
-- There's a bit of a story behind this line. In old versions
2240+
-- of Cabal, there were only internal build-tools dependencies. So the
2241+
-- behavior in this case was:
2242+
--
2243+
-- - If a build-tool dependency was internal, don't do
2244+
-- any checking.
2245+
--
2246+
-- - If it was external, call 'configureRequiredProgram' to
2247+
-- "configure" the executable. In particular, if
2248+
-- the program was not "known" (present in 'ProgramDb'),
2249+
-- then we would just error. This was fine, because
2250+
-- the only way a program could be executed from 'ProgramDb'
2251+
-- is if some library code from Cabal actually called it,
2252+
-- and the pre-existing Cabal code only calls known
2253+
-- programs from 'defaultProgramDb', and so if it
2254+
-- is calling something else, you have a Custom setup
2255+
-- script, and in that case you are expected to register
2256+
-- the program you want to call in the ProgramDb.
2257+
--
2258+
-- OK, so that was fine, until I (ezyang, in 2016) refactored
2259+
-- Cabal to support per-component builds. In this case, what
2260+
-- was previously an internal build-tool dependency now became
2261+
-- an external one, and now previously "internal" dependencies
2262+
-- are now external. But these are permitted to exist even
2263+
-- when they are not previously configured (something that
2264+
-- can only occur by a Custom script.)
2265+
--
2266+
-- So, I decided, "Fine, let's just accept these in any
2267+
-- case." Thus this line. The alternative would have been to
2268+
-- somehow detect when a build-tools dependency was "internal" (by
2269+
-- looking at the unflattened package description) but this
2270+
-- would also be incompatible with future work to support
2271+
-- external executable dependencies: we definitely cannot
2272+
-- assume they will be preinitialized in the 'ProgramDb'.
2273+
configureProgram verbosity (simpleProgram progName) progdb
2274+
Just prog
2275+
-- requireProgramVersion always requires the program have a version
2276+
-- but if the user says "build-depends: foo" ie no version constraint
2277+
-- then we should not fail if we cannot discover the program version.
2278+
| verRange == anyVersion -> do
2279+
(_, progdb') <- requireProgram verbosity prog progdb
2280+
return progdb'
2281+
| otherwise -> do
2282+
(_, _, progdb') <- requireProgramVersion verbosity prog verRange progdb
2283+
return progdb'
22622284

22632285
-- -----------------------------------------------------------------------------
22642286
-- Configuring pkg-config package dependencies

0 commit comments

Comments
 (0)