Skip to content
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

Add check for upper bound on any dependency in cabal check #8339

Closed
1 change: 1 addition & 0 deletions Cabal-tests/tests/CheckTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ checkTests = testGroup "regressions"
, checkTest "assoc-cpp-options.cabal"
, checkTest "public-multilib-1.cabal"
, checkTest "public-multilib-2.cabal"
, checkTest "public-multilib-3.cabal"
, checkTest "issue-6288-a.cabal"
, checkTest "issue-6288-b.cabal"
, checkTest "issue-6288-c.cabal"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ library
default-language: Haskell2010
build-depends:
, base ^>=4.14
, somelib:internal
, somelib:internal ^>=1.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm uncertain what this syntax somelib:internal means, I assumed somelib is a dependency.
There appears no docs on this syntax (note that this is the other multilib failing now, not mulitlib 1, which I fixed).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think somelib is a dep, internal is its component.


exposed-modules: Foo
14 changes: 14 additions & 0 deletions Cabal-tests/tests/ParserTests/regressions/public-multilib-3.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
cabal-version: 3.0
name: public-multilib3
version: 0
synopsis: public-multilibs
category: Tests
license: MIT

library
default-language: Haskell2010
build-depends:
, base ^>=4.14
, somelib

exposed-modules: Foo
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
No 'maintainer' field.
No 'description' field.
These packages miss upper bounds 'somelib' please add them with with `cabal gen-bounds`. For more information see: https://www.parsonsmatt.org/2020/05/07/on_pvp_restrictive_bounds.html
71 changes: 41 additions & 30 deletions Cabal/src/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ data CheckExplanation =
| UnknownArch [String]
| UnknownCompiler [String]
| BaseNoUpperBounds
| MissingUpperBounds [PackageName]
| SuspiciousFlagName [String]
| DeclaredUsedFlags (Set FlagName) (Set FlagName)
| NonASCIICustomField [String]
Expand Down Expand Up @@ -669,6 +670,12 @@ ppExplanation (UnknownArch unknownArches) =
"Unknown architecture name " ++ commaSep (map quote unknownArches)
ppExplanation (UnknownCompiler unknownImpls) =
"Unknown compiler name " ++ commaSep (map quote unknownImpls)
ppExplanation (MissingUpperBounds names) =
"These packages miss upper bounds '"
++ (intercalate "','" (unPackageName <$> names)) ++ "'"
++ " please add them with with `cabal gen-bounds`."
++ " For more information see: "
++ " https://www.parsonsmatt.org/2020/05/07/on_pvp_restrictive_bounds.html"
ppExplanation BaseNoUpperBounds =
"The dependency 'build-depends: base' does not specify an upper "
++ "bound on the version number. Each major release of the 'base' "
Expand Down Expand Up @@ -1814,29 +1821,22 @@ checkCabalVersion pkg =
--
checkPackageVersions :: GenericPackageDescription -> [PackageCheck]
checkPackageVersions pkg =
catMaybes [

-- Check that the version of base is bounded above.
-- For example this bans "build-depends: base >= 3".
-- It should probably be "build-depends: base >= 3 && < 4"
-- which is the same as "build-depends: base == 3.*"
check (not (hasUpperBound baseDependency)) $
PackageDistInexcusable BaseNoUpperBounds

]
if length others > 0
then
PackageDistSuspiciousWarn (MissingUpperBounds others) : baseErrors
else
baseErrors
where
baseDependency = case typicalPkg pkg of
Right (pkg', _) | not (null baseDeps) ->
foldr intersectVersionRanges anyVersion baseDeps
where
baseDeps =
[ vr | Dependency pname vr _ <- allBuildDepends pkg'
, pname == mkPackageName "base" ]

-- Just in case finalizePD fails for any reason,
-- or if the package doesn't depend on the base package at all,
-- then we will just skip the check, since hasUpperBound noVersion = True
_ -> noVersion
baseErrors = (PackageDistInexcusable BaseNoUpperBounds <$ bases)
deps = toDependencyVersionsMap allBuildDepends pkg
-- base gets special treatment (it's more critical)
(bases, others) = partition (("base" ==) . unPackageName) $ do
(name, vr) <- Map.toList deps
-- Check that the version of a package is bounded above.
-- For example this bans "build-depends: base >= 3".
-- It should probably be "build-depends: base >= 3 && < 4"
-- which is the same as "build-depends: base == 3.*"
if hasUpperBound vr then [] else pure name -- emit for the error

checkConditionals :: GenericPackageDescription -> [PackageCheck]
checkConditionals pkg =
Expand Down Expand Up @@ -2410,14 +2410,7 @@ checkSetupVersions pkg =
]
where
criticalPkgs = ["Cabal", "base"]
deps = case typicalPkg pkg of
Right (pkgs', _) ->
Map.fromListWith intersectVersionRanges
[ (pname, vr)
| sbi <- maybeToList $ setupBuildInfo pkgs'
, Dependency pname vr _ <- setupDepends sbi
]
_ -> Map.empty
deps = toDependencyVersionsMap ((=<<) setupDepends . maybeToList . setupBuildInfo) pkg
emitError nm =
PackageDistInexcusable (UpperBoundSetup nm)

Expand Down Expand Up @@ -2456,6 +2449,24 @@ checkDuplicateModules pkg =
-- * Utils
-- ------------------------------------------------------------

toDependencyVersionsMap :: (PackageDescription -> [Dependency]) -> GenericPackageDescription -> Map PackageName VersionRange
toDependencyVersionsMap selectDependencies pkg = case typicalPkg pkg of
Right (pkgs', _) ->
let
self :: PackageName
self = pkgName $ package pkgs'
in
Map.fromListWith intersectVersionRanges $ do
Dependency pname vr _ <- selectDependencies pkgs'
if pname == self then
[]
else [(pname, vr)]
-- Just in case finalizePD fails for any reason,
-- or if the package doesn't depend on the base package at all,
-- no deps is no checks.
_ -> Map.empty


quote :: String -> String
quote s = "'" ++ s ++ "'"

Expand Down
4 changes: 4 additions & 0 deletions changelog.d/pr-8339
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
synopsis: Add check for upper bound on any dependency in cabal check
report, list, init, fetch, info, upload, get.
prs: #8339
issues: #8291