From 223f737ee047e0847f4a8029747a3134fd9ddb03 Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Mon, 3 Apr 2023 19:08:53 +0200 Subject: [PATCH 1/8] Abstract `isDistError` There are two places in cabal codebase where `isDistError` is defined (would Hackage refuse the package based on this error?), plus another one in hackage-server (src/Distribution/Server/Packages/Unpack.hs). This commit refactors the definitions into a singel `isHackageDistError` (in Distribution.Client.Check) so everything is in sync. The function is exported, for the benefit of third-party tools. --- Cabal/src/Distribution/PackageDescription/Check.hs | 7 +++++++ Cabal/src/Distribution/Simple/SrcDist.hs | 5 +---- cabal-install/src/Distribution/Client/Check.hs | 7 ++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Cabal/src/Distribution/PackageDescription/Check.hs b/Cabal/src/Distribution/PackageDescription/Check.hs index 5d11072b354..22102253c7f 100644 --- a/Cabal/src/Distribution/PackageDescription/Check.hs +++ b/Cabal/src/Distribution/PackageDescription/Check.hs @@ -28,6 +28,7 @@ module Distribution.PackageDescription.Check ( checkConfiguredPackage, wrapParseWarning, ppPackageCheck, + isHackageDistError, -- ** Checking package contents checkPackageFiles, @@ -846,6 +847,12 @@ data PackageCheck = | PackageDistInexcusable { explanation :: CheckExplanation } deriving (Eq, Ord) +-- | Would Hackage refuse a package because of this error? +isHackageDistError :: PackageCheck -> Bool +isHackageDistError (PackageDistSuspicious {}) = False +isHackageDistError (PackageDistSuspiciousWarn {}) = False +isHackageDistError _ = True + -- | Pretty printing 'PackageCheck'. -- ppPackageCheck :: PackageCheck -> String diff --git a/Cabal/src/Distribution/Simple/SrcDist.hs b/Cabal/src/Distribution/Simple/SrcDist.hs index 99f1bb8166c..e3bf177ac24 100644 --- a/Cabal/src/Distribution/Simple/SrcDist.hs +++ b/Cabal/src/Distribution/Simple/SrcDist.hs @@ -501,10 +501,7 @@ printPackageProblems :: Verbosity -> PackageDescription -> IO () printPackageProblems verbosity pkg_descr = do ioChecks <- checkPackageFiles verbosity pkg_descr "." let pureChecks = checkConfiguredPackage pkg_descr - isDistError (PackageDistSuspicious _) = False - isDistError (PackageDistSuspiciousWarn _) = False - isDistError _ = True - (errors, warnings) = partition isDistError (pureChecks ++ ioChecks) + (errors, warnings) = partition isHackageDistError (pureChecks ++ ioChecks) unless (null errors) $ notice verbosity $ "Distribution quality errors:\n" ++ unlines (map ppPackageCheck errors) diff --git a/cabal-install/src/Distribution/Client/Check.hs b/cabal-install/src/Distribution/Client/Check.hs index b08fa164e5d..1724624aeef 100644 --- a/cabal-install/src/Distribution/Client/Check.hs +++ b/cabal-install/src/Distribution/Client/Check.hs @@ -94,12 +94,9 @@ check verbosity = do warn verbosity "The following errors will cause portability problems on other environments:" printCheckMessages distInexusable - let isDistError (PackageDistSuspicious {}) = False - isDistError (PackageDistSuspiciousWarn {}) = False - isDistError _ = True - isCheckError (PackageDistSuspiciousWarn {}) = False + let isCheckError (PackageDistSuspiciousWarn {}) = False isCheckError _ = True - errors = filter isDistError packageChecks + errors = filter isHackageDistError packageChecks unless (null errors) $ warn verbosity "Hackage would reject this package." From f7080a38e45bd857aebd605ab1a9357ad2b89b7a Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Mon, 3 Apr 2023 19:19:13 +0200 Subject: [PATCH 2/8] Make `check` comply with Hackage requirements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cabal check` exits with 0 or 1. As now the `1` exit is slightly stricter than “Hackage would reject this”, as even PackageDistSuspicious errors will trigger a `1` exit. This patch rectifies this and also specifies the behaviour in the documentation. Notice the usage of “should” and “will”: we cannot be sure Hackage will accept a package (e.g.: maybe there are name clashes) but there are no type I errors: if it fails `cabal check`, Hackage will refuse it. --- cabal-install/src/Distribution/Client/Check.hs | 10 +++++----- cabal-install/src/Distribution/Client/Setup.hs | 4 ++-- doc/cabal-commands.rst | 3 ++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cabal-install/src/Distribution/Client/Check.hs b/cabal-install/src/Distribution/Client/Check.hs index 1724624aeef..f032b148555 100644 --- a/cabal-install/src/Distribution/Client/Check.hs +++ b/cabal-install/src/Distribution/Client/Check.hs @@ -48,7 +48,9 @@ readGenericPackageDescriptionCheck verbosity fpath = do die' verbosity "parse error" Right x -> return (warnings, x) --- | Note: must be called with the CWD set to the directory containing +-- | Checks a packge for common errors. Returns @True@ if the package +-- is fit to upload to Hackage, @False@ otherwise. +-- Note: must be called with the CWD set to the directory containing -- the '.cabal' file. check :: Verbosity -> IO Bool check verbosity = do @@ -94,9 +96,7 @@ check verbosity = do warn verbosity "The following errors will cause portability problems on other environments:" printCheckMessages distInexusable - let isCheckError (PackageDistSuspiciousWarn {}) = False - isCheckError _ = True - errors = filter isHackageDistError packageChecks + let errors = filter isHackageDistError packageChecks unless (null errors) $ warn verbosity "Hackage would reject this package." @@ -104,7 +104,7 @@ check verbosity = do when (null packageChecks) $ notice verbosity "No errors or warnings could be found in the package." - return (not . any isCheckError $ packageChecks) + return (null errors) where printCheckMessages :: [PackageCheck] -> IO () diff --git a/cabal-install/src/Distribution/Client/Setup.hs b/cabal-install/src/Distribution/Client/Setup.hs index 6db91d9cf98..2e890a70ce1 100644 --- a/cabal-install/src/Distribution/Client/Setup.hs +++ b/cabal-install/src/Distribution/Client/Setup.hs @@ -1201,8 +1201,8 @@ checkCommand = CommandUI { "Expects a .cabal package file in the current directory.\n" ++ "\n" ++ "The checks correspond to the requirements to packages on Hackage. " - ++ "If no errors and warnings are reported, Hackage will accept this " - ++ "package.\n", + ++ "If no errors and warnings are reported, Hackage should accept the " + ++ "package. If 1 is returned, Hackage will refuse it.\n", commandNotes = Nothing, commandUsage = usageFlags "check", commandDefaultFlags = toFlag normal, diff --git a/doc/cabal-commands.rst b/doc/cabal-commands.rst index 0ea968b0f5d..d40a6083c7b 100644 --- a/doc/cabal-commands.rst +++ b/doc/cabal-commands.rst @@ -977,7 +977,8 @@ Run ``cabal check`` in the folder where your ``.cabal`` package file is. Set verbosity level (0–3, default is 1). ``cabal check`` mimics Hackage's requirements: if no error or warning -is reported, Hackage should accept your package. +is reported, Hackage should accept your package. If ``cabal check`` exits +with ``1``, Hackage will refuse it. cabal sdist ^^^^^^^^^^^ From 0cc8cd76a815844dee03fa6f39e5b15b8b495397 Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Tue, 4 Apr 2023 08:15:15 +0200 Subject: [PATCH 3/8] Make testsuite pass All the modified tests returned 1 on exit, now they return 0. --- .../Check/ConfiguredPackage/COptions/CxxOs/cabal.test.hs | 2 +- .../CabalVersion/DefaultExtension/cabal.test.hs | 2 +- .../CabalVersion/DefaultLanguage/cabal.test.hs | 2 +- .../CabalVersion/ExtraDynamicLibraryFlavour/cabal.test.hs | 2 +- .../Check/ConfiguredPackage/CabalVersion/Mixins/cabal.test.hs | 2 +- .../Check/ConfiguredPackage/CabalVersion/Sources/cabal.test.hs | 2 +- .../ConfiguredPackage/CabalVersion/VirtualModules/cabal.test.hs | 2 +- .../ConfiguredPackage/Fields/DeprecatedExtension/cabal.test.hs | 2 +- .../Check/ConfiguredPackage/Fields/NoCategory/cabal.test.hs | 2 +- .../Check/ConfiguredPackage/Fields/NoDescription/cabal.test.hs | 2 +- .../Check/ConfiguredPackage/Fields/NoMaintainer/cabal.test.hs | 2 +- .../Check/ConfiguredPackage/Fields/NoSynopsis/cabal.test.hs | 2 +- .../ConfiguredPackage/Fields/ShortDescription/cabal.test.hs | 2 +- .../ConfiguredPackage/GHCOptions/GHCSharedOptions/cabal.test.hs | 2 +- .../ConfiguredPackage/License/NoFileSpecified/cabal.test.hs | 2 +- .../ConfiguredPackage/License/SuspiciousLicense/cabal.test.hs | 2 +- .../ConfiguredPackage/License/SuspiciousVersion/cabal.test.hs | 2 +- .../License/WarnAllRightsReserved/cabal.test.hs | 2 +- .../ConfiguredPackage/Sanity/VersionSignatures/cabal.test.hs | 2 +- .../Check/NonConfCheck/DevOnlyFlags/Profiling/cabal.test.hs | 2 +- .../PackageTests/Check/NonConfCheck/UnusedFlags/cabal.test.hs | 2 +- .../PackageTests/Check/PackageFiles/VCSInfo/cabal.test.hs | 2 +- 22 files changed, 22 insertions(+), 22 deletions(-) diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/COptions/CxxOs/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/COptions/CxxOs/cabal.test.hs index 9706be30598..af9d416967c 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/COptions/CxxOs/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/COptions/CxxOs/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `cxx-options`, do not use `-O1`. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultExtension/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultExtension/cabal.test.hs index 558722a1e75..ed1e8720e0c 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultExtension/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultExtension/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `default-extensions` need ≥1.10. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultLanguage/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultLanguage/cabal.test.hs index 9b2fb76f2fe..aefcb6e899e 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultLanguage/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/DefaultLanguage/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `default-language` need ≥1.10. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/ExtraDynamicLibraryFlavour/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/ExtraDynamicLibraryFlavour/cabal.test.hs index d6de120a48b..ea93456bba3 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/ExtraDynamicLibraryFlavour/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/ExtraDynamicLibraryFlavour/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `extra-dynamic-library-flavour` need ≥3.0. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Mixins/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Mixins/cabal.test.hs index 59b761f8008..b1d092438d8 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Mixins/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Mixins/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `mixins` need ≥2.0. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Sources/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Sources/cabal.test.hs index 4b34bdad916..5b8c31c0b66 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Sources/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/Sources/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `cmm-sources` and friends need ≥3.0. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/VirtualModules/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/VirtualModules/cabal.test.hs index 4672147a484..7a92518d401 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/VirtualModules/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/CabalVersion/VirtualModules/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `virtual-modules` need ≥2.2. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/DeprecatedExtension/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/DeprecatedExtension/cabal.test.hs index 1d31f4c583e..73f0a75bdee 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/DeprecatedExtension/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/DeprecatedExtension/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Deprecated extension. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoCategory/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoCategory/cabal.test.hs index 484b17857ac..50fb012514d 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoCategory/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoCategory/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- No category. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoDescription/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoDescription/cabal.test.hs index 388d77437df..ee2c3ac0018 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoDescription/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoDescription/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- No description. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoMaintainer/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoMaintainer/cabal.test.hs index 6a615a7ee67..832b74a21a3 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoMaintainer/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoMaintainer/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- No maintainer. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoSynopsis/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoSynopsis/cabal.test.hs index ea88909bb5d..f2ee05944ac 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoSynopsis/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/NoSynopsis/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- No synopsis. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/ShortDescription/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/ShortDescription/cabal.test.hs index 9974626c7a6..723a2dca2dc 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/ShortDescription/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Fields/ShortDescription/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Description should be longer than synopsis. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/GHCOptions/GHCSharedOptions/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/GHCOptions/GHCSharedOptions/cabal.test.hs index 8974491ac0d..672a005ed41 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/GHCOptions/GHCSharedOptions/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/GHCOptions/GHCSharedOptions/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Tricky option in `ghc-shared-options`. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/NoFileSpecified/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/NoFileSpecified/cabal.test.hs index 65cef73d457..7a81a247159 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/NoFileSpecified/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/NoFileSpecified/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `licence-file` missing. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousLicense/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousLicense/cabal.test.hs index 086abf34304..37ee44418e0 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousLicense/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousLicense/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Suspicious license BSD4. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousVersion/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousVersion/cabal.test.hs index ae6610af429..9ba2f98306d 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousVersion/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/SuspiciousVersion/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Suspicious license version. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/WarnAllRightsReserved/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/WarnAllRightsReserved/cabal.test.hs index f618ac6fb3d..7719c9ffd9e 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/WarnAllRightsReserved/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/License/WarnAllRightsReserved/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Dubious AllRightsReserved. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Sanity/VersionSignatures/cabal.test.hs b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Sanity/VersionSignatures/cabal.test.hs index 2ef34c34581..da01864a554 100644 --- a/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Sanity/VersionSignatures/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/ConfiguredPackage/Sanity/VersionSignatures/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- `signatures` field used with cabal-version < 2.0 main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/NonConfCheck/DevOnlyFlags/Profiling/cabal.test.hs b/cabal-testsuite/PackageTests/Check/NonConfCheck/DevOnlyFlags/Profiling/cabal.test.hs index ad9c8f4bb76..ac7dc0a3ef8 100644 --- a/cabal-testsuite/PackageTests/Check/NonConfCheck/DevOnlyFlags/Profiling/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/NonConfCheck/DevOnlyFlags/Profiling/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Profiling flags unsuited for distribution. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/NonConfCheck/UnusedFlags/cabal.test.hs b/cabal-testsuite/PackageTests/Check/NonConfCheck/UnusedFlags/cabal.test.hs index 7bbbaf8e4ce..15c29338ada 100644 --- a/cabal-testsuite/PackageTests/Check/NonConfCheck/UnusedFlags/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/NonConfCheck/UnusedFlags/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Unused flag. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/PackageFiles/VCSInfo/cabal.test.hs b/cabal-testsuite/PackageTests/Check/PackageFiles/VCSInfo/cabal.test.hs index 0d6ac3d42f4..a2321d77b22 100644 --- a/cabal-testsuite/PackageTests/Check/PackageFiles/VCSInfo/cabal.test.hs +++ b/cabal-testsuite/PackageTests/Check/PackageFiles/VCSInfo/cabal.test.hs @@ -2,4 +2,4 @@ import Test.Cabal.Prelude -- Missing VCS info. main = cabalTest $ - fails $ cabal "check" [] + cabal "check" [] From 83345968566ec554d70b6e59b58a5c88918d91da Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Tue, 4 Apr 2023 17:55:53 +0200 Subject: [PATCH 4/8] Add changelog for #8897 --- changelog.d/pr-8897 | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 changelog.d/pr-8897 diff --git a/changelog.d/pr-8897 b/changelog.d/pr-8897 new file mode 100644 index 00000000000..a89023dc416 --- /dev/null +++ b/changelog.d/pr-8897 @@ -0,0 +1,14 @@ +synopsis: Make check comply with Hackage requirements +packages: Cabal cabal-install +prs: #8897 + +description: { + +- `cabal check` will only return exitcode 1 when the package is not fit + for Hackage. E.g. it will not error anymore when your `synopsis:` is + larger than `description:`, just emit a warning. +- Cabal: Distribution.Client.Check now exports `isHackageDistError`, for + third-party tools to know if a specific error will preclude a package + from being uploaded to Hacakge. + +} From a341f74fd6a0ca981f5b9514fbb917bf7deaf1aa Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Thu, 6 Apr 2023 20:06:40 +0200 Subject: [PATCH 5/8] Expand catchall into constructors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (Andreas’ review) For `isHackageDistError`, so that in the future, when constructors are added, the compiler will warn about missing them missing and a thoughtful choice will be made. --- Cabal/src/Distribution/PackageDescription/Check.hs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Cabal/src/Distribution/PackageDescription/Check.hs b/Cabal/src/Distribution/PackageDescription/Check.hs index 22102253c7f..4064e0db3c1 100644 --- a/Cabal/src/Distribution/PackageDescription/Check.hs +++ b/Cabal/src/Distribution/PackageDescription/Check.hs @@ -849,9 +849,11 @@ data PackageCheck = -- | Would Hackage refuse a package because of this error? isHackageDistError :: PackageCheck -> Bool +isHackageDistError (PackageBuildImpossible {}) = True +isHackageDistError (PackageBuildWarning {}) = True +isHackageDistError (PackageDistInexcusable {}) = True isHackageDistError (PackageDistSuspicious {}) = False isHackageDistError (PackageDistSuspiciousWarn {}) = False -isHackageDistError _ = True -- | Pretty printing 'PackageCheck'. -- From 8fffe3217c16acd5eee2919e95449046ea94ea68 Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 15 Apr 2023 16:55:51 +0200 Subject: [PATCH 6/8] Use LambdaCase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (Artem’s review) Saves typing `isHackageDistError` four times more. --- Cabal/src/Distribution/PackageDescription/Check.hs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Cabal/src/Distribution/PackageDescription/Check.hs b/Cabal/src/Distribution/PackageDescription/Check.hs index 4064e0db3c1..e602a9a6ee3 100644 --- a/Cabal/src/Distribution/PackageDescription/Check.hs +++ b/Cabal/src/Distribution/PackageDescription/Check.hs @@ -1,3 +1,5 @@ +{-# LANGUAGE LambdaCase #-} + ----------------------------------------------------------------------------- -- | -- Module : Distribution.PackageDescription.Check @@ -849,11 +851,12 @@ data PackageCheck = -- | Would Hackage refuse a package because of this error? isHackageDistError :: PackageCheck -> Bool -isHackageDistError (PackageBuildImpossible {}) = True -isHackageDistError (PackageBuildWarning {}) = True -isHackageDistError (PackageDistInexcusable {}) = True -isHackageDistError (PackageDistSuspicious {}) = False -isHackageDistError (PackageDistSuspiciousWarn {}) = False +isHackageDistError = \case + (PackageBuildImpossible {}) -> True + (PackageBuildWarning {}) -> True + (PackageDistInexcusable {}) -> True + (PackageDistSuspicious {}) -> False + (PackageDistSuspiciousWarn {}) -> False -- | Pretty printing 'PackageCheck'. -- From d2a830dba59c6f19301396d7ee0ac3511682abd6 Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 15 Apr 2023 17:12:07 +0200 Subject: [PATCH 7/8] Reword help strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (Artem’s review) Make clear that errors will make `check` return 1. --- cabal-install/src/Distribution/Client/Setup.hs | 3 ++- doc/cabal-commands.rst | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cabal-install/src/Distribution/Client/Setup.hs b/cabal-install/src/Distribution/Client/Setup.hs index 2e890a70ce1..10244ea97c2 100644 --- a/cabal-install/src/Distribution/Client/Setup.hs +++ b/cabal-install/src/Distribution/Client/Setup.hs @@ -1202,7 +1202,8 @@ checkCommand = CommandUI { ++ "\n" ++ "The checks correspond to the requirements to packages on Hackage. " ++ "If no errors and warnings are reported, Hackage should accept the " - ++ "package. If 1 is returned, Hackage will refuse it.\n", + ++ "package. If errors are present, `check` exits with 1 and Hackage " + ++ "will refuse the package.\n", commandNotes = Nothing, commandUsage = usageFlags "check", commandDefaultFlags = toFlag normal, diff --git a/doc/cabal-commands.rst b/doc/cabal-commands.rst index d40a6083c7b..a4f793c6799 100644 --- a/doc/cabal-commands.rst +++ b/doc/cabal-commands.rst @@ -977,8 +977,8 @@ Run ``cabal check`` in the folder where your ``.cabal`` package file is. Set verbosity level (0–3, default is 1). ``cabal check`` mimics Hackage's requirements: if no error or warning -is reported, Hackage should accept your package. If ``cabal check`` exits -with ``1``, Hackage will refuse it. +is reported, Hackage should accept your package. If errors are present +``cabal check`` exits with ``1`` and Hackage will will refuse the package. cabal sdist ^^^^^^^^^^^ From 5c3b107580db35ae7906824de24456a653d1324d Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 15 Apr 2023 18:20:48 +0200 Subject: [PATCH 8/8] Fix typo --- doc/cabal-commands.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/cabal-commands.rst b/doc/cabal-commands.rst index a4f793c6799..3ce853429ce 100644 --- a/doc/cabal-commands.rst +++ b/doc/cabal-commands.rst @@ -978,7 +978,7 @@ Run ``cabal check`` in the folder where your ``.cabal`` package file is. ``cabal check`` mimics Hackage's requirements: if no error or warning is reported, Hackage should accept your package. If errors are present -``cabal check`` exits with ``1`` and Hackage will will refuse the package. +``cabal check`` exits with ``1`` and Hackage will refuse the package. cabal sdist ^^^^^^^^^^^