From ea830d70144db14175d20f139973aa2865337def Mon Sep 17 00:00:00 2001
From: Artem Pelenitsyn <a.pelenitsyn@gmail.com>
Date: Wed, 25 Aug 2021 20:13:24 -0400
Subject: [PATCH] Rebase of #5370 (fix for #4683) (#7409)
* Introduce checks for upper dependencies in setup.
setup depends are now checked for having upper dependencies.
Upper bounds are mandatory for base and Cabal libraries, and
are optional (but emit warning) for other libraries.
Implicit dependencies are not being checked.
* Add tests.
* Fix tests.
* add changelog
* fix test for the newer cabal_raw'
* scale down alerts: no warnings on any package, only errors on base+Cabal
* address review comments
* typo
* improve changelog (add Cabal)
* remove boundedAbove and use Distribution.Version.hasUpperBound instead
Co-authored-by: Alexander Vershilov <alexander.vershilov@gmail.com>
Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
Co-authored-by: Mikolaj Konarski <mikolaj@well-typed.com>
---
.../Distribution/PackageDescription/Check.hs | 77 ++++++++++++-------
.../PackageTests/CheckSetup/LICENSE | 1 +
.../PackageTests/CheckSetup/MyLibrary.hs | 1 +
.../PackageTests/CheckSetup/Setup.hs | 2 +
.../PackageTests/CheckSetup/my.cabal | 25 ++++++
.../PackageTests/CheckSetup/setup.test.hs | 20 +++++
changelog.d/issue-4683 | 4 +
7 files changed, 104 insertions(+), 26 deletions(-)
create mode 100644 cabal-testsuite/PackageTests/CheckSetup/LICENSE
create mode 100644 cabal-testsuite/PackageTests/CheckSetup/MyLibrary.hs
create mode 100644 cabal-testsuite/PackageTests/CheckSetup/Setup.hs
create mode 100644 cabal-testsuite/PackageTests/CheckSetup/my.cabal
create mode 100644 cabal-testsuite/PackageTests/CheckSetup/setup.test.hs
create mode 100644 changelog.d/issue-4683
diff --git a/Cabal/src/Distribution/PackageDescription/Check.hs b/Cabal/src/Distribution/PackageDescription/Check.hs
index 680a9ab118..fc1c196f06 100644
--- a/Cabal/src/Distribution/PackageDescription/Check.hs
+++ b/Cabal/src/Distribution/PackageDescription/Check.hs
@@ -154,6 +154,7 @@ checkPackage gpkg mpkg =
++ checkUnusedFlags gpkg
++ checkUnicodeXFields gpkg
++ checkPathsModuleExtensions pkg
+ ++ checkSetupVersions gpkg
where
pkg = fromMaybe (flattenPackageDescription gpkg) mpkg
@@ -1416,7 +1417,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' "
@@ -1431,21 +1432,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
@@ -1455,18 +1442,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 [
@@ -2151,6 +2129,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
-- ------------------------------------------------------------
@@ -2384,3 +2391,21 @@ isGoodRelativeDirectoryPath = state0
-- | x <= CSlash -> 1
-- | otherwise -> 4
-- @
+
+--
+-- 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)
+ []
diff --git a/cabal-testsuite/PackageTests/CheckSetup/LICENSE b/cabal-testsuite/PackageTests/CheckSetup/LICENSE
new file mode 100644
index 0000000000..6b1d0bfabc
--- /dev/null
+++ b/cabal-testsuite/PackageTests/CheckSetup/LICENSE
@@ -0,0 +1 @@
+LICENSE
diff --git a/cabal-testsuite/PackageTests/CheckSetup/MyLibrary.hs b/cabal-testsuite/PackageTests/CheckSetup/MyLibrary.hs
new file mode 100644
index 0000000000..a51c414bcd
--- /dev/null
+++ b/cabal-testsuite/PackageTests/CheckSetup/MyLibrary.hs
@@ -0,0 +1 @@
+module MyLibrary () where
diff --git a/cabal-testsuite/PackageTests/CheckSetup/Setup.hs b/cabal-testsuite/PackageTests/CheckSetup/Setup.hs
new file mode 100644
index 0000000000..9a994af677
--- /dev/null
+++ b/cabal-testsuite/PackageTests/CheckSetup/Setup.hs
@@ -0,0 +1,2 @@
+import Distribution.Simple
+main = defaultMain
diff --git a/cabal-testsuite/PackageTests/CheckSetup/my.cabal b/cabal-testsuite/PackageTests/CheckSetup/my.cabal
new file mode 100644
index 0000000000..538e637514
--- /dev/null
+++ b/cabal-testsuite/PackageTests/CheckSetup/my.cabal
@@ -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
diff --git a/cabal-testsuite/PackageTests/CheckSetup/setup.test.hs b/cabal-testsuite/PackageTests/CheckSetup/setup.test.hs
new file mode 100644
index 0000000000..96ed439578
--- /dev/null
+++ b/cabal-testsuite/PackageTests/CheckSetup/setup.test.hs
@@ -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 ()
diff --git a/changelog.d/issue-4683 b/changelog.d/issue-4683
new file mode 100644
index 0000000000..9fc7de4533
--- /dev/null
+++ b/changelog.d/issue-4683
@@ -0,0 +1,4 @@
+synopsis: 'cabal check' to fail when no upper bounds for base or Cabal are present in setup dependencies
+packages: Cabal, cabal-install
+issues: #4683
+prs: #5370, #7409
--
GitLab