From ea01b9d55b0f9799667cc5e0d2afab2684fc1280 Mon Sep 17 00:00:00 2001 From: Kristen Kozak Date: Sun, 25 Feb 2018 21:45:38 -0800 Subject: [PATCH 1/2] Look for transitive setup dependency on Cabal when choosing Cabal spec version. Fixes #4288. Previously, new-build only looked for a direct dependency on Cabal when choosing the spec version to use for running the setup script. When the setup script only had a transitive dependency on Cabal, cabal used the package's cabal-version field, which could easily differ from the actual Cabal dependency's version. Then cabal could pass flags to the setup script that weren't recognized by the Cabal dependency. This commit considers any setup dependency on Cabal when choosing the Cabal spec version. --- .../Distribution/Client/ProjectPlanning.hs | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/cabal-install/Distribution/Client/ProjectPlanning.hs b/cabal-install/Distribution/Client/ProjectPlanning.hs index 02287958b5d..531035823ef 100644 --- a/cabal-install/Distribution/Client/ProjectPlanning.hs +++ b/cabal-install/Distribution/Client/ProjectPlanning.hs @@ -1210,7 +1210,7 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB if null not_per_component_reasons then return comps else do checkPerPackageOk comps not_per_component_reasons - return [elaborateSolverToPackage mapDep spkg g $ + return [elaborateSolverToPackage spkg g $ comps ++ maybeToList setupComponent] Left cns -> dieProgress $ @@ -1276,7 +1276,7 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB fsep (punctuate comma reasons) -- TODO: Maybe exclude Backpack too - elab0 = elaborateSolverToCommon mapDep spkg + elab0 = elaborateSolverToCommon spkg pkgid = elabPkgSourceId elab0 pd = elabPkgDescription elab0 @@ -1501,13 +1501,11 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB Just (Just n) -> display n _ -> "" - elaborateSolverToPackage :: (SolverId -> [ElaboratedPlanPackage]) - -> SolverPackage UnresolvedPkgLoc + elaborateSolverToPackage :: SolverPackage UnresolvedPkgLoc -> ComponentsGraph -> [ElaboratedConfiguredPackage] -> ElaboratedConfiguredPackage elaborateSolverToPackage - mapDep pkg@(SolverPackage (SourcePackage pkgid _gdesc _srcloc _descOverride) _flags _stanzas _deps0 _exe_deps0) compGraph comps = @@ -1516,7 +1514,7 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB -- of the other fields of the elaboratedPackage. elab where - elab0@ElaboratedConfiguredPackage{..} = elaborateSolverToCommon mapDep pkg + elab0@ElaboratedConfiguredPackage{..} = elaborateSolverToCommon pkg elab = elab0 { elabUnitId = newSimpleUnitId pkgInstalledId, elabComponentId = pkgInstalledId, @@ -1613,10 +1611,9 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB (compilerId compiler) pkgInstalledId - elaborateSolverToCommon :: (SolverId -> [ElaboratedPlanPackage]) - -> SolverPackage UnresolvedPkgLoc + elaborateSolverToCommon :: SolverPackage UnresolvedPkgLoc -> ElaboratedConfiguredPackage - elaborateSolverToCommon mapDep + elaborateSolverToCommon pkg@(SolverPackage (SourcePackage pkgid gdesc srcloc descOverride) flags stanzas deps0 _exe_deps0) = elaboratedPackage @@ -1687,10 +1684,9 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB elabRegisterPackageDBStack = buildAndRegisterDbs elabSetupScriptStyle = packageSetupScriptStyle elabPkgDescription - -- Computing the deps here is a little awful - deps = fmap (concatMap (elaborateLibSolverId mapDep)) deps0 - elabSetupScriptCliVersion = packageSetupScriptSpecVersion - elabSetupScriptStyle elabPkgDescription deps + elabSetupScriptCliVersion = + packageSetupScriptSpecVersion + elabSetupScriptStyle elabPkgDescription libDepGraph deps0 elabSetupPackageDBStack = buildAndRegisterDbs buildAndRegisterDbs @@ -2910,31 +2906,34 @@ defaultSetupDeps compiler platform pkg = -- of what the solver picked for us, based on the explicit setup deps or the -- ones added implicitly by 'defaultSetupDeps'. -- -packageSetupScriptSpecVersion :: Package pkg - => SetupScriptStyle +packageSetupScriptSpecVersion :: SetupScriptStyle -> PD.PackageDescription - -> ComponentDeps [pkg] + -> Graph.Graph NonSetupLibDepSolverPlanPackage + -> ComponentDeps [SolverId] -> Version -- We're going to be using the internal Cabal library, so the spec version of -- that is simply the version of the Cabal library that cabal-install has been -- built with. -packageSetupScriptSpecVersion SetupNonCustomInternalLib _ _ = +packageSetupScriptSpecVersion SetupNonCustomInternalLib _ _ _ = cabalVersion -- If we happen to be building the Cabal lib itself then because that -- bootstraps itself then we use the version of the lib we're building. -packageSetupScriptSpecVersion SetupCustomImplicitDeps pkg _ +packageSetupScriptSpecVersion SetupCustomImplicitDeps pkg _ _ | packageName pkg == cabalPkgname = packageVersion pkg -- In all other cases we have a look at what version of the Cabal lib the -- solver picked. Or if it didn't depend on Cabal at all (which is very rare) -- then we look at the .cabal file to see what spec version it declares. -packageSetupScriptSpecVersion _ pkg deps = - case find ((cabalPkgname ==) . packageName) (CD.setupDeps deps) of +packageSetupScriptSpecVersion _ pkg libDepGraph deps = + case find ((cabalPkgname ==) . packageName) setupLibDeps of Just dep -> packageVersion dep Nothing -> PD.specVersion pkg + where + setupLibDeps = map packageId $ fromMaybe [] $ + Graph.closure libDepGraph (CD.setupDeps deps) cabalPkgname, basePkgname :: PackageName From f4f382ae1e43e3eac04bdf8bb300f1da1003fba3 Mon Sep 17 00:00:00 2001 From: Kristen Kozak Date: Sun, 25 Feb 2018 21:45:38 -0800 Subject: [PATCH 2/2] Add a regression test for issue #4288. --- .../PackageTests/NewBuild/T4288/CustomIssue.hs | 3 +++ .../PackageTests/NewBuild/T4288/Setup.hs | 3 +++ .../PackageTests/NewBuild/T4288/T4288.cabal | 16 ++++++++++++++++ .../PackageTests/NewBuild/T4288/cabal.project | 1 + .../PackageTests/NewBuild/T4288/cabal.test.hs | 17 +++++++++++++++++ .../NewBuild/T4288/setup-helper/SetupHelper.hs | 5 +++++ .../T4288/setup-helper/setup-helper.cabal | 9 +++++++++ 7 files changed, 54 insertions(+) create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/CustomIssue.hs create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/Setup.hs create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/T4288.cabal create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/cabal.project create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/cabal.test.hs create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/SetupHelper.hs create mode 100644 cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/setup-helper.cabal diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/CustomIssue.hs b/cabal-testsuite/PackageTests/NewBuild/T4288/CustomIssue.hs new file mode 100644 index 00000000000..b19db66b19c --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/CustomIssue.hs @@ -0,0 +1,3 @@ +module CustomIssue where + +f x = x + 1 diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/Setup.hs b/cabal-testsuite/PackageTests/NewBuild/T4288/Setup.hs new file mode 100644 index 00000000000..c98988c966b --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/Setup.hs @@ -0,0 +1,3 @@ +import SetupHelper (setupHelperDefaultMain) + +main = setupHelperDefaultMain diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/T4288.cabal b/cabal-testsuite/PackageTests/NewBuild/T4288/T4288.cabal new file mode 100644 index 00000000000..eaa6fc71bac --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/T4288.cabal @@ -0,0 +1,16 @@ +name: T4288 +version: 1.0 +build-type: Custom + +-- cabal-version is lower than the version of Cabal that will be chosen for the +-- setup script. +cabal-version: >=1.10 + +-- Setup script only has a transitive dependency on Cabal. +custom-setup + setup-depends: base, setup-helper + +library + exposed-modules: CustomIssue + build-depends: base + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/cabal.project b/cabal-testsuite/PackageTests/NewBuild/T4288/cabal.project new file mode 100644 index 00000000000..2c65bdcb6c8 --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/cabal.project @@ -0,0 +1 @@ +packages: . setup-helper/ diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/cabal.test.hs b/cabal-testsuite/PackageTests/NewBuild/T4288/cabal.test.hs new file mode 100644 index 00000000000..dc15d37a5bc --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/cabal.test.hs @@ -0,0 +1,17 @@ +import Test.Cabal.Prelude + +-- This test is similar to the simplified example in issue #4288. The package's +-- setup script only depends on base and setup-helper. setup-helper exposes a +-- function that is a wrapper for Cabal's defaultMain (similar to +-- cabal-doctest). This test builds the package to check that the flags passed +-- to the setup script are compatible with the version of Cabal that it depends +-- on, even though Cabal is only a transitive dependency. +main = cabalTest $ do + skipUnless =<< hasNewBuildCompatBootCabal + r <- recordMode DoNotRecord $ cabal' "new-build" ["T4288"] + assertOutputContains "This is setup-helper-1.0." r + assertOutputContains + ("In order, the following will be built: " + ++ " - setup-helper-1.0 (lib:setup-helper) (first run) " + ++ " - T4288-1.0 (lib:T4288) (first run)") + r diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/SetupHelper.hs b/cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/SetupHelper.hs new file mode 100644 index 00000000000..0d71dd0570d --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/SetupHelper.hs @@ -0,0 +1,5 @@ +module SetupHelper (setupHelperDefaultMain) where + +import Distribution.Simple + +setupHelperDefaultMain = putStrLn "This is setup-helper-1.0." >> defaultMain diff --git a/cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/setup-helper.cabal b/cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/setup-helper.cabal new file mode 100644 index 00000000000..eb3570e04cd --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T4288/setup-helper/setup-helper.cabal @@ -0,0 +1,9 @@ +name: setup-helper +version: 1.0 +build-type: Simple +cabal-version: >= 1.2 + +library + exposed-modules: SetupHelper + build-depends: base, Cabal + default-language: Haskell2010