-
Notifications
You must be signed in to change notification settings - Fork 725
Force Cabal >= 1.24 dep when there's no custom-setup stanza. #3337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,7 +97,7 @@ module Distribution.PackageDescription ( | |
| GenericPackageDescription(..), | ||
| Flag(..), FlagName(..), FlagAssignment, | ||
| CondTree(..), ConfVar(..), Condition(..), | ||
| cNot, | ||
| cNot, cAnd, cOr, | ||
|
|
||
| -- * Source repositories | ||
| SourceRepo(..), | ||
|
|
@@ -111,7 +111,7 @@ module Distribution.PackageDescription ( | |
|
|
||
| import Distribution.Compat.Binary | ||
| import qualified Distribution.Compat.Semigroup as Semi ((<>)) | ||
| import Distribution.Compat.Semigroup as Semi (Monoid(..), Semigroup, gmempty, gmappend) | ||
| import Distribution.Compat.Semigroup as Semi (Monoid(..), Semigroup, gmempty) | ||
| import qualified Distribution.Compat.ReadP as Parse | ||
| import Distribution.Compat.ReadP ((<++)) | ||
| import Distribution.Package | ||
|
|
@@ -308,18 +308,24 @@ instance Text BuildType where | |
| -- options authors can specify to just Haskell package dependencies. | ||
|
|
||
| data SetupBuildInfo = SetupBuildInfo { | ||
| setupDepends :: [Dependency] | ||
| setupDepends :: [Dependency], | ||
| defaultSetupDepends :: Bool | ||
| -- ^ Is this a default 'custom-setup' section added by the cabal-install | ||
| -- code (as opposed to user-provided)? This field is only used | ||
| -- internally, and doesn't correspond to anything in the .cabal | ||
| -- file. See #3199. | ||
| } | ||
| deriving (Generic, Show, Eq, Read, Typeable, Data) | ||
|
|
||
| instance Binary SetupBuildInfo | ||
|
|
||
| instance Semi.Monoid SetupBuildInfo where | ||
| mempty = gmempty | ||
| mempty = SetupBuildInfo [] False | ||
| mappend = (Semi.<>) | ||
|
|
||
| instance Semigroup SetupBuildInfo where | ||
| (<>) = gmappend | ||
| a <> b = SetupBuildInfo (setupDepends a Semi.<> setupDepends b) | ||
| (defaultSetupDepends a || defaultSetupDepends b) | ||
|
|
||
| -- --------------------------------------------------------------------------- | ||
| -- Module renaming | ||
|
|
@@ -1193,11 +1199,32 @@ data Condition c = Var c | |
| | CAnd (Condition c) (Condition c) | ||
| deriving (Show, Eq, Typeable, Data, Generic) | ||
|
|
||
| -- | Boolean negation of a 'Condition' value. | ||
| cNot :: Condition a -> Condition a | ||
| cNot (Lit b) = Lit (not b) | ||
| cNot (CNot c) = c | ||
| cNot c = CNot c | ||
|
|
||
| -- | Boolean AND of two 'Condtion' values. | ||
| cAnd :: Condition a -> Condition a -> Condition a | ||
| cAnd (Lit False) _ = Lit False | ||
| cAnd _ (Lit False) = Lit False | ||
| cAnd (Lit True) x = x | ||
| cAnd x (Lit True) = x | ||
| cAnd x y = CAnd x y | ||
|
|
||
| -- | Boolean OR of two 'Condition' values. | ||
| cOr :: Eq v => Condition v -> Condition v -> Condition v | ||
| cOr (Lit True) _ = Lit True | ||
| cOr _ (Lit True) = Lit True | ||
| cOr (Lit False) x = x | ||
| cOr x (Lit False) = x | ||
| cOr c (CNot d) | ||
| | c == d = Lit True | ||
| cOr (CNot c) d | ||
| | c == d = Lit True | ||
| cOr x y = COr x y | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot to mention that I added a case to my snippet: Without it, the function testing whether #3199 applies doesn't ignore this use of buildable:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indeed, it's kinda odd that symmetry was broken for the "x or (not x)" case...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @grayjay Updated. |
||
|
|
||
| instance Functor Condition where | ||
| f `fmap` Var c = Var (f c) | ||
| _ `fmap` Lit c = Lit c | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,16 +92,14 @@ import Distribution.Package | |
| , Package(..), packageName, packageVersion | ||
| , UnitId, Dependency(Dependency)) | ||
| import qualified Distribution.PackageDescription as PD | ||
| ( PackageDescription(..), SetupBuildInfo(..) | ||
| , GenericPackageDescription(..) | ||
| , Flag(flagName), FlagName(..) ) | ||
| import qualified Distribution.PackageDescription.Configuration as PD | ||
| import Distribution.PackageDescription.Configuration | ||
| ( finalizePackageDescription ) | ||
| import Distribution.Client.PackageUtils | ||
| ( externalBuildDepends ) | ||
| import Distribution.Version | ||
| ( VersionRange, anyVersion, thisVersion, withinRange | ||
| , simplifyVersionRange ) | ||
| ( VersionRange, Version(..), anyVersion, orLaterVersion, thisVersion | ||
| , withinRange, simplifyVersionRange ) | ||
| import Distribution.Compiler | ||
| ( CompilerInfo(..) ) | ||
| import Distribution.System | ||
|
|
@@ -122,7 +120,7 @@ import Distribution.Verbosity | |
| import Data.List | ||
| ( foldl', sort, sortBy, nubBy, maximumBy, intercalate, nub ) | ||
| import Data.Function (on) | ||
| import Data.Maybe (fromMaybe) | ||
| import Data.Maybe (fromMaybe) | ||
| import qualified Data.Map as Map | ||
| import qualified Data.Set as Set | ||
| import Data.Set (Set) | ||
|
|
@@ -392,7 +390,7 @@ removeUpperBounds allowNewer params = | |
| -- 'addSourcePackages'. Otherwise, the packages inserted by | ||
| -- 'addSourcePackages' won't have upper bounds in dependencies relaxed. | ||
| -- | ||
| addDefaultSetupDependencies :: (SourcePackage -> [Dependency]) | ||
| addDefaultSetupDependencies :: (SourcePackage -> Maybe [Dependency]) | ||
| -> DepResolverParams -> DepResolverParams | ||
| addDefaultSetupDependencies defaultSetupDeps params = | ||
| params { | ||
|
|
@@ -408,9 +406,12 @@ addDefaultSetupDependencies defaultSetupDeps params = | |
| PD.setupBuildInfo = | ||
| case PD.setupBuildInfo pkgdesc of | ||
| Just sbi -> Just sbi | ||
| Nothing -> Just PD.SetupBuildInfo { | ||
| PD.setupDepends = defaultSetupDeps srcpkg | ||
| } | ||
| Nothing -> case defaultSetupDeps srcpkg of | ||
| Nothing -> Nothing | ||
| Just deps -> Just PD.SetupBuildInfo { | ||
| PD.defaultSetupDepends = True, | ||
| PD.setupDepends = deps | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -449,12 +450,41 @@ standardInstallPolicy | |
| . hideInstalledPackagesSpecificBySourcePackageId | ||
| [ packageId pkg | SpecificSourcePackage pkg <- pkgSpecifiers ] | ||
|
|
||
| . addDefaultSetupDependencies mkDefaultSetupDeps | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So make this function conditional on the build-type Custom, and then we don't need the previous patch to apply that conditionality within |
||
| . addSourcePackages | ||
| [ pkg | SpecificSourcePackage pkg <- pkgSpecifiers ] | ||
|
|
||
| $ basicDepResolverParams | ||
| installedPkgIndex sourcePkgIndex | ||
|
|
||
| where | ||
| -- Force Cabal >= 1.24 dep when the package is affected by #3199. | ||
| mkDefaultSetupDeps :: SourcePackage -> Maybe [Dependency] | ||
| mkDefaultSetupDeps srcpkg | affected = | ||
| Just [Dependency (PackageName "Cabal") | ||
| (orLaterVersion $ Version [1,24] [])] | ||
| | otherwise = Nothing | ||
| where | ||
| gpkgdesc = packageDescription srcpkg | ||
| pkgdesc = PD.packageDescription gpkgdesc | ||
| bt = fromMaybe PD.Custom (PD.buildType pkgdesc) | ||
| affected = bt == PD.Custom && hasBuildableFalse gpkgdesc | ||
|
|
||
| -- Does this package contain any components with non-empty 'build-depends' | ||
| -- and a 'buildable' field that could potentially be set to 'False'? False | ||
| -- positives are possible. | ||
| hasBuildableFalse :: PD.GenericPackageDescription -> Bool | ||
| hasBuildableFalse gpkg = | ||
| not (all alwaysTrue (zipWith PD.cOr buildableConditions noDepConditions)) | ||
| where | ||
| buildableConditions = PD.extractConditions PD.buildable gpkg | ||
| noDepConditions = PD.extractConditions | ||
| (null . PD.targetBuildDepends) gpkg | ||
| alwaysTrue (PD.Lit True) = True | ||
| alwaysTrue _ = False | ||
|
|
||
|
|
||
| applySandboxInstallPolicy :: SandboxPackageInfo | ||
| -> DepResolverParams | ||
| -> DepResolverParams | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, cunning. I went to some lengths in the new-build code to do this without changing the underlying type, but yes it is rather easier if we can just stash the info here. I might switch the new-build code over to use that, rather than keeping an extra Set around.