Skip to content

Commit

Permalink
Use default-package-bounds in check and cabal-install
Browse files Browse the repository at this point in the history
  • Loading branch information
jasagredo committed Jun 26, 2024
1 parent de41943 commit c2aaf71
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 8 deletions.
27 changes: 26 additions & 1 deletion Cabal/src/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ checkGenericPackageDescription
packageDescription_
_gpdScannedVersion_
genPackageFlags_
genDefaultPackageBounds_
condLibrary_
condSubLibraries_
condForeignLibs_
Expand Down Expand Up @@ -272,6 +273,8 @@ checkGenericPackageDescription
checkPackageDescription packageDescription_
-- Flag names.
mapM_ checkFlagName genPackageFlags_
-- default package bounds
mapM_ checkDefaultBounds genDefaultPackageBounds_

-- § Feature checks.
checkSpecVer
Expand All @@ -294,9 +297,11 @@ checkGenericPackageDescription
. pnPackageId
. ccNames
)

let ads =
maybe [] ((: []) . extractAssocDeps pName) condLibrary_
++ map (uncurry extractAssocDeps) condSubLibraries_
++ maybe [] ((: []) . Left . defaultTargetBuildDepends) genDefaultPackageBounds_

case condLibrary_ of
Just cl ->
Expand Down Expand Up @@ -362,6 +367,26 @@ checkGenericPackageDescription
(invalidFlagName fn)
(PackageDistInexcusable $ SuspiciousFlagName [fn])

checkDefaultBounds :: Monad m => DefaultBounds -> CheckM m ()
checkDefaultBounds (DefaultBounds db dbt) =
let noBound = isAnyVersion . depVerRange
noExeBound = isAnyVersion . exeDepVerRange
in do
mapM_
( \b ->
checkP
(noBound b)
(PackageDistInexcusable $ DefaultBoundsNoBound [unPackageName $ depPkgName b])
)
db
mapM_
( \b ->
checkP
(noExeBound b)
(PackageDistInexcusable $ DefaultBoundsNoBound [unPackageName $ exeDepPkgName b])
)
dbt

decFlags :: Set.Set FlagName
decFlags = toSetOf (L.genPackageFlags . traverse . L.flagName) gpd

Expand Down Expand Up @@ -916,7 +941,7 @@ extractAssocDeps n ct =
in -- Merging is fine here, remember the specific
-- library dependencies will be checked branch
-- by branch.
(n, snd a)
Right (n, snd a)

-- | August 2022: this function is an oddity due to the historical
-- GenericPackageDescription/PackageDescription split (check
Expand Down
17 changes: 12 additions & 5 deletions Cabal/src/Distribution/PackageDescription/Check/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ checkCustomField (n, _) =

-- A library name / dependencies association list. Ultimately to be
-- fed to PVP check.
type AssocDep = (UnqualComponentName, [Dependency])
type AssocDep = Either [Dependency] (UnqualComponentName, [Dependency])

-- Convenience function to partition important dependencies by name. To
-- be used together with checkPVP. Important: usually “base” or “Cabal”,
Expand All @@ -67,7 +67,7 @@ type AssocDep = (UnqualComponentName, [Dependency])
partitionDeps
:: Monad m
=> [AssocDep] -- Possibly inherited dependencies, i.e.
-- dependencies from internal/main libs.
-- dependencies from internal/main libs
-> [UnqualComponentName] -- List of package names ("base", "Cabal"…)
-> [Dependency] -- Dependencies to check.
-> CheckM m ([Dependency], [Dependency])
Expand All @@ -77,18 +77,25 @@ partitionDeps ads ns ds = do
-- names of our dependencies
dqs = map unqualName ds
-- shared targets that match
fads = filter (flip elem dqs . fst) ads
fads =
filter
( \ad -> case ad of
Left{} -> True
Right (x, _) -> elem x dqs
)
ads
-- the names of such targets
inNam = nub $ map fst fads :: [UnqualComponentName]
inNam = nub $ mapMaybe (either (const Nothing) (Just . fst)) fads :: [UnqualComponentName]
-- the dependencies of such targets
inDep = concatMap snd fads :: [Dependency]
inDep = concatMap (either id snd) fads :: [Dependency]

-- We exclude from checks:
-- 1. dependencies which are shared with main library / a
-- sublibrary; and of course
-- 2. the names of main library / sub libraries themselves.
--
-- So in myPackage.cabal
--
-- library
-- build-depends: text < 5
--
Expand Down
5 changes: 5 additions & 0 deletions Cabal/src/Distribution/PackageDescription/Check/Target.hs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ module Distribution.PackageDescription.Check.Target
, checkExecutable
, checkTestSuite
, checkBenchmark
-- Utils
, mergeDependencies
) where

import Distribution.Compat.Prelude
Expand Down Expand Up @@ -336,6 +338,7 @@ checkBuildInfo cet ams ads bi = do
ads
[mkUnqualComponentName "base"]
(mergeDependencies $ targetBuildDepends bi)

let ick = const (PackageDistInexcusable BaseNoUpperBounds)
rck = PackageDistSuspiciousWarn . MissingUpperBounds cet
checkPVP ick ids
Expand Down Expand Up @@ -760,6 +763,7 @@ isInternalTarget (CETExecutable{}) = False
isInternalTarget (CETTest{}) = True
isInternalTarget (CETBenchmark{}) = True
isInternalTarget (CETSetup{}) = False
isInternalTarget (CETDefaultPackageBounds{}) = True

-- ------------------------------------------------------------
-- Options
Expand All @@ -776,6 +780,7 @@ cet2bit (CETExecutable{}) = BITOther
cet2bit (CETTest{}) = BITTestBench
cet2bit (CETBenchmark{}) = BITTestBench
cet2bit CETSetup = BITOther
cet2bit CETDefaultPackageBounds = BITOther

-- General check on all options (ghc, C, C++, …) for common inaccuracies.
checkBuildInfoOptions :: Monad m => BITarget -> BuildInfo -> CheckM m ()
Expand Down
13 changes: 13 additions & 0 deletions Cabal/src/Distribution/PackageDescription/Check/Warning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ data CheckExplanation
| UnknownCompiler [String]
| BaseNoUpperBounds
| MissingUpperBounds CEType [String]
| DefaultBoundsNoBound [String]
| SuspiciousFlagName [String]
| DeclaredUsedFlags (Set.Set FlagName) (Set.Set FlagName)
| NonASCIICustomField [String]
Expand Down Expand Up @@ -417,6 +418,7 @@ data CheckExplanationID
| CIUnknownCompiler
| CIBaseNoUpperBounds
| CIMissingUpperBounds
| CIDefaultBoundsNoBound
| CISuspiciousFlagName
| CIDeclaredUsedFlags
| CINonASCIICustomField
Expand Down Expand Up @@ -558,6 +560,7 @@ checkExplanationId (UnknownArch{}) = CIUnknownArch
checkExplanationId (UnknownCompiler{}) = CIUnknownCompiler
checkExplanationId (BaseNoUpperBounds{}) = CIBaseNoUpperBounds
checkExplanationId (MissingUpperBounds{}) = CIMissingUpperBounds
checkExplanationId (DefaultBoundsNoBound{}) = CIDefaultBoundsNoBound
checkExplanationId (SuspiciousFlagName{}) = CISuspiciousFlagName
checkExplanationId (DeclaredUsedFlags{}) = CIDeclaredUsedFlags
checkExplanationId (NonASCIICustomField{}) = CINonASCIICustomField
Expand Down Expand Up @@ -704,6 +707,7 @@ ppCheckExplanationId CIUnknownArch = "unknown-arch"
ppCheckExplanationId CIUnknownCompiler = "unknown-compiler"
ppCheckExplanationId CIBaseNoUpperBounds = "missing-bounds-important"
ppCheckExplanationId CIMissingUpperBounds = "missing-upper-bounds"
ppCheckExplanationId CIDefaultBoundsNoBound = "default-bounds-no-bound"
ppCheckExplanationId CISuspiciousFlagName = "suspicious-flag"
ppCheckExplanationId CIDeclaredUsedFlags = "unused-flag"
ppCheckExplanationId CINonASCIICustomField = "non-ascii"
Expand Down Expand Up @@ -746,6 +750,7 @@ data CEType
| CETTest UnqualComponentName
| CETBenchmark UnqualComponentName
| CETSetup
| CETDefaultPackageBounds
deriving (Eq, Ord, Show)

-- | Pretty printing `CEType`.
Expand All @@ -757,6 +762,7 @@ ppCET cet = case cet of
CETTest n -> "test suite" ++ qn n
CETBenchmark n -> "benchmark" ++ qn n
CETSetup -> "custom-setup"
CETDefaultPackageBounds -> "default package bounds"
where
qn :: UnqualComponentName -> String
qn wn = (" " ++) . quote . prettyShow $ wn
Expand Down Expand Up @@ -1302,6 +1308,13 @@ ppExplanation (MissingUpperBounds ct names) =
++ List.intercalate separator names
++ "\n"
++ "Please add them. There is more information at https://pvp.haskell.org/"
ppExplanation (DefaultBoundsNoBound names) =
let separator = "\n - "
in "These default-package-bounds declarations are missing bounds:"
++ separator
++ List.intercalate separator names
++ "\n"
++ "It doesn't make sense to have empty bounds on default-package-bounds. Please add them."
ppExplanation (SuspiciousFlagName invalidFlagNames) =
"Suspicious flag names: "
++ unwords invalidFlagNames
Expand Down
5 changes: 3 additions & 2 deletions cabal-install/src/Distribution/Client/Dependency.hs
Original file line number Diff line number Diff line change
Expand Up @@ -554,13 +554,14 @@ relaxPackageDeps
-> PD.GenericPackageDescription
-> PD.GenericPackageDescription
relaxPackageDeps _ rd gpd | not (isRelaxDeps rd) = gpd -- subsumed by no-op case in 'removeBounds'
relaxPackageDeps relKind RelaxDepsAll gpd = PD.transformAllBuildDepends relaxAll gpd
relaxPackageDeps relKind RelaxDepsAll gpd =
PD.transformDefaultBuildDepends relaxAll $ PD.transformAllBuildDepends relaxAll gpd
where
relaxAll :: Dependency -> Dependency
relaxAll (Dependency pkgName verRange cs) =
Dependency pkgName (removeBound relKind RelaxDepModNone verRange) cs
relaxPackageDeps relKind (RelaxDepsSome depsToRelax0) gpd =
PD.transformAllBuildDepends relaxSome gpd
PD.transformDefaultBuildDepends relaxSome $ PD.transformAllBuildDepends relaxSome gpd
where
thisPkgName = packageName gpd
thisPkgId = packageId gpd
Expand Down
1 change: 1 addition & 0 deletions cabal-install/src/Distribution/Client/IndexUtils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,7 @@ packageListFromCache verbosity mkPkg hnd Cache{..} = accum mempty [] mempty cach
}
, gpdScannedVersion = Just specVer -- tells index scanner to skip this file.
, genPackageFlags = []
, genDefaultPackageBounds = Nothing
, condLibrary = Nothing
, condSubLibraries = []
, condForeignLibs = []
Expand Down
1 change: 1 addition & 0 deletions cabal-install/tests/IntegrationTests2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ testTargetSelectorAmbiguous reportSubCase = do
packageDescription = emptyPackageDescription { package = pkgid },
gpdScannedVersion = Nothing,
genPackageFlags = [],
genDefaultPackageBounds = Nothing,
condLibrary = Nothing,
condSubLibraries = [],
condForeignLibs = [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ exAvSrcPkg ex =
}
, C.gpdScannedVersion = Nothing
, C.genPackageFlags = flags
, C.genDefaultPackageBounds = Nothing
, C.condLibrary =
let mkLib v bi = mempty{C.libVisibility = v, C.libBuildInfo = bi}
-- Avoid using the Monoid instance for [a] when getting
Expand Down

0 comments on commit c2aaf71

Please sign in to comment.