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

Rebase of #5370 (fix for #4683) #7409

Merged
merged 12 commits into from
Aug 26, 2021
77 changes: 51 additions & 26 deletions Cabal/src/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ checkPackage gpkg mpkg =
++ checkUnusedFlags gpkg
++ checkUnicodeXFields gpkg
++ checkPathsModuleExtensions pkg
++ checkSetupVersions gpkg
where
pkg = fromMaybe (flattenPackageDescription gpkg) mpkg

Expand Down Expand Up @@ -1405,7 +1406,7 @@ checkPackageVersions pkg =
-- 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 (boundedAbove baseDependency)) $
check (not (hasUpperBound baseDependency)) $
PackageDistInexcusable $
"The dependency 'build-depends: base' does not specify an upper "
++ "bound on the version number. Each major release of the 'base' "
Expand All @@ -1420,21 +1421,7 @@ checkPackageVersions pkg =

]
where
-- TODO: What we really want to do is test if there exists any
-- configuration in which the base version is unbounded above.
-- However that's a bit tricky because there are many possible
-- configurations. As a cheap easy and safe approximation we will
-- pick a single "typical" configuration and check if that has an
-- open upper bound. To get a typical configuration we finalise
-- using no package index and the current platform.
finalised = finalizePD
mempty defaultComponentRequestedSpec (const True)
buildPlatform
(unknownCompilerInfo
(CompilerId buildCompilerFlavor nullVersion)
NoAbiTag)
[] pkg
baseDependency = case finalised of
baseDependency = case typicalPkg pkg of
Right (pkg', _) | not (null baseDeps) ->
foldr intersectVersionRanges anyVersion baseDeps
where
Expand All @@ -1444,18 +1431,9 @@ checkPackageVersions pkg =

-- 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 boundedAbove noVersion = True
-- then we will just skip the check, since hasUpperBound noVersion = True
_ -> noVersion

-- TODO: move to Distribution.Version
boundedAbove :: VersionRange -> Bool
boundedAbove vr = case asVersionIntervals vr of
[] -> True -- this is the inconsistent version range.
(x:xs) -> case last (x:|xs) of
VersionInterval _ UpperBound {} -> True
VersionInterval _ NoUpperBound -> False


checkConditionals :: GenericPackageDescription -> [PackageCheck]
checkConditionals pkg =
catMaybes [
Expand Down Expand Up @@ -2125,6 +2103,35 @@ checkGlobFiles verbosity pkg root =
++ " directory by that name."
]

-- | Check that setup dependencies, have proper bounds.
-- In particular, @base@ and @Cabal@ upper bounds are mandatory.
checkSetupVersions :: GenericPackageDescription -> [PackageCheck]
checkSetupVersions pkg =
[ emitError nameStr
| (name, vr) <- Map.toList deps
, not (hasUpperBound vr)
, let nameStr = unPackageName name
, nameStr `elem` criticalPkgs
]
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
emitError nm =
PackageDistInexcusable $
"The dependency 'setup-depends: '"++nm++"' does not specify an "
++ "upper bound on the version number. Each major release of the "
++ "'"++nm++"' package changes the API in various ways and most "
++ "packages will need some changes to compile with it. If you are "
++ "not sure what upper bound to use then use the next major "
++ "version."

-- ------------------------------------------------------------
-- * Utils
-- ------------------------------------------------------------
Expand Down Expand Up @@ -2394,3 +2401,21 @@ isGoodRelativeDirectoryPath = state0
-- | otherwise -> 4
-- 6 -> \_ -> 6 -- black hole
-- @

--
-- TODO: What we really want to do is test if there exists any
-- configuration in which the base version is unbounded above.
-- However that's a bit tricky because there are many possible
-- configurations. As a cheap easy and safe approximation we will
-- pick a single "typical" configuration and check if that has an
-- open upper bound. To get a typical configuration we finalise
-- using no package index and the current platform.
typicalPkg :: GenericPackageDescription
-> Either [Dependency] (PackageDescription, FlagAssignment)
typicalPkg = finalizePD
mempty defaultComponentRequestedSpec (const True)
buildPlatform
(unknownCompilerInfo
(CompilerId buildCompilerFlavor nullVersion)
NoAbiTag)
[]
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/CheckSetup/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENSE
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/CheckSetup/MyLibrary.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module MyLibrary () where
2 changes: 2 additions & 0 deletions cabal-testsuite/PackageTests/CheckSetup/Setup.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import Distribution.Simple
main = defaultMain
25 changes: 25 additions & 0 deletions cabal-testsuite/PackageTests/CheckSetup/my.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: CheckSetup
version: 0.1
license: BSD3
license-file: LICENSE
author: Alexander Vershilov
maintainer: Alexander Vershilov
synopsis: Check setup
category: PackageTests
build-type: Custom
cabal-version: 2.0

description:
Check that Cabal recognizes problems with setup module.

custom-setup
setup-depends:
base,
Cabal,
bytestring

Library
default-language: Haskell2010
build-depends: base <5.0
exposed-modules:
MyLibrary
20 changes: 20 additions & 0 deletions cabal-testsuite/PackageTests/CheckSetup/setup.test.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import Test.Cabal.Prelude

-- Test that setup shows all the 'autogen-modules' warnings.
main = cabalTest $ do

checkResult <- fails $ cabal_raw' ["check"] Nothing

-- Package check messages.
let libError1 =
"The dependency 'setup-depends: 'Cabal' does not specify "
++ "an upper bound on the version number"
libError2 =
"The dependency 'setup-depends: 'base' does not specify "
++ "an upper bound on the version number"

-- Asserts for the desired check messages after configure.
assertOutputContains libError1 checkResult
assertOutputContains libError2 checkResult

return ()
4 changes: 4 additions & 0 deletions changelog.d/issue-4683
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
synopsis: 'cabal check' to fail when no upper bounds for base or Cabal are present in setup dependencies
Mikolaj marked this conversation as resolved.
Show resolved Hide resolved
packages: Cabal, cabal-install
issues: #4683
prs: #5370, #7409