Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ exAvSrcPkg ex =
-- solver allows unknown extensions/languages when the compiler
-- supports them.
let checks = C.checkPackage (srcpkgDescription package) Nothing
in filter (not . isUnknownLangExt) checks
in filter (not . isMissingUpperBound) $ filter (not . isUnknownLangExt) checks
in if null pkgCheckErrors
then package
else error $ "invalid GenericPackageDescription for package "
Expand Down Expand Up @@ -671,6 +671,10 @@ exAvSrcPkg ex =
C.UnknownExtensions {} -> True
C.UnknownLanguages {} -> True
_ -> False
isMissingUpperBound :: C.PackageCheck -> Bool
isMissingUpperBound pc = case C.explanation pc of
C.MissingUpperBounds {} -> True
_ -> False


mkSimpleVersion :: ExamplePkgVersion -> C.Version
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