diff --git a/cabal-install/src/Distribution/Client/Errors.hs b/cabal-install/src/Distribution/Client/Errors.hs index 3b80b50dd94..b5201aaf939 100644 --- a/cabal-install/src/Distribution/Client/Errors.hs +++ b/cabal-install/src/Distribution/Client/Errors.hs @@ -70,7 +70,7 @@ data CabalInstallException | ReportTargetProblems String | ListBinTargetException String | ResolveWithoutDependency String - | CannotReadCabalFile FilePath + | CannotReadCabalFile FilePath FilePath | ErrorUpdatingIndex FilePath IOException | InternalError FilePath | ReadIndexCache FilePath @@ -390,7 +390,11 @@ exceptionMessageCabalInstall e = case e of ReportTargetProblems problemsMsg -> problemsMsg ListBinTargetException errorStr -> errorStr ResolveWithoutDependency errorStr -> errorStr - CannotReadCabalFile file -> "Cannot read .cabal file inside " ++ file + CannotReadCabalFile expect file -> + "Failed to read " + ++ expect + ++ " from archive " + ++ file ErrorUpdatingIndex name ioe -> "Error while updating index for " ++ name ++ " repository " ++ show ioe InternalError msg -> "internal error when reading package index: " diff --git a/cabal-install/src/Distribution/Client/IndexUtils.hs b/cabal-install/src/Distribution/Client/IndexUtils.hs index 705c62d62d1..13ef8c6456d 100644 --- a/cabal-install/src/Distribution/Client/IndexUtils.hs +++ b/cabal-install/src/Distribution/Client/IndexUtils.hs @@ -66,6 +66,10 @@ import Distribution.Client.Types import Distribution.Parsec (simpleParsecBS) import Distribution.Verbosity +import Distribution.Client.ProjectConfig + ( CabalFileParseError + , readSourcePackageCabalFile' + ) import Distribution.Client.Setup ( RepoContext (..) ) @@ -97,6 +101,7 @@ import Distribution.Simple.Utils , fromUTF8LBS , info , warn + , warnError ) import Distribution.Types.Dependency import Distribution.Types.PackageName (PackageName) @@ -880,14 +885,22 @@ withIndexEntries verbosity (RepoIndex _repoCtxt (RepoLocalNoIndex (LocalRepo nam where cabalPath = prettyShow pkgid ++ ".cabal" Just pkgId -> do + let tarFile = localDir file -- check for the right named .cabal file in the compressed tarball - tarGz <- BS.readFile (localDir file) + tarGz <- BS.readFile tarFile let tar = GZip.decompress tarGz entries = Tar.read tar + expectFilename = prettyShow pkgId FilePath. prettyShow (packageName pkgId) ++ ".cabal" - case Tar.foldEntries (readCabalEntry pkgId) Nothing (const Nothing) entries of + tarballPackageDescription <- + Tar.foldEntries + (readCabalEntry tarFile expectFilename) + (pure Nothing) + (handleTarFormatError tarFile) + entries + case tarballPackageDescription of Just ce -> return (Just ce) - Nothing -> dieWithException verbosity $ CannotReadCabalFile file + Nothing -> dieWithException verbosity $ CannotReadCabalFile expectFilename tarFile let (prefs, gpds) = partitionEithers $ @@ -918,16 +931,55 @@ withIndexEntries verbosity (RepoIndex _repoCtxt (RepoLocalNoIndex (LocalRepo nam stripSuffix sfx str = fmap reverse (stripPrefix (reverse sfx) (reverse str)) - -- look for /.cabal inside the tarball - readCabalEntry :: PackageIdentifier -> Tar.Entry -> Maybe NoIndexCacheEntry -> Maybe NoIndexCacheEntry - readCabalEntry pkgId entry Nothing - | filename == Tar.entryPath entry - , Tar.NormalFile contents _ <- Tar.entryContent entry = - let bs = BS.toStrict contents - in ((`CacheGPD` bs) <$> parseGenericPackageDescriptionMaybe bs) - where - filename = prettyShow pkgId FilePath. prettyShow (packageName pkgId) ++ ".cabal" - readCabalEntry _ _ x = x + handleTarFormatError :: FilePath -> Tar.FormatError -> IO (Maybe NoIndexCacheEntry) + handleTarFormatError tarFile formatError = do + -- This is printed at warning-level because malformed `.tar.gz`s in + -- indexes are unexpected and we want users to tell us about them. + warnError verbosity $ + "Failed to parse " + <> tarFile + <> ": " + <> displayException formatError + pure Nothing + + -- look for `expectFilename` inside the tarball + readCabalEntry + :: FilePath + -> FilePath + -> Tar.Entry + -> IO (Maybe NoIndexCacheEntry) + -> IO (Maybe NoIndexCacheEntry) + readCabalEntry tarFile expectFilename entry previous' = do + previous <- previous' + case previous of + Just _entry -> pure previous + Nothing -> do + if expectFilename /= Tar.entryPath entry + then pure Nothing + else case Tar.entryContent entry of + Tar.NormalFile contents _fileSize -> do + let bytes = BS.toStrict contents + maybePackageDescription + :: Either CabalFileParseError GenericPackageDescription <- + -- Warnings while parsing `.cabal` files are only shown in + -- verbose mode because people are allowed to upload packages + -- with warnings to Hackage (and we may _add_ warnings by + -- shipping new versions of Cabal). You probably don't care + -- if there's a warning in some random package in your + -- transitive dependency tree, as long as it's not causing + -- issues. If it is causing issues, you can add `-v` to see + -- the warnings! + try $ readSourcePackageCabalFile' (info verbosity) expectFilename bytes + case maybePackageDescription of + Left exception -> do + -- Here we show the _failure_ to parse the `.cabal` file as + -- a warning. This will impact which versions/packages are + -- available in your index, so users should know! + warn verbosity $ "In " <> tarFile <> ": " <> displayException exception + pure Nothing + Right genericPackageDescription -> + pure $ Just $ CacheGPD genericPackageDescription bytes + _ -> pure Nothing withIndexEntries verbosity index callback _ = do -- non-secure repositories withFile (indexFile index) ReadMode $ \h -> do diff --git a/cabal-install/src/Distribution/Client/ProjectConfig.hs b/cabal-install/src/Distribution/Client/ProjectConfig.hs index 5f31dc0fab5..478b8771c09 100644 --- a/cabal-install/src/Distribution/Client/ProjectConfig.hs +++ b/cabal-install/src/Distribution/Client/ProjectConfig.hs @@ -38,6 +38,9 @@ module Distribution.Client.ProjectConfig , writeProjectConfigFile , commandLineFlagsToProjectConfig , onlyTopLevelProvenance + , readSourcePackageCabalFile + , readSourcePackageCabalFile' + , CabalFileParseError (..) -- * Packages within projects , ProjectPackageLocation (..) @@ -174,7 +177,6 @@ import Distribution.Simple.Setup import Distribution.Simple.Utils ( createDirectoryIfMissingVerbose , dieWithException - , info , maybeExit , notice , rawSystemIOWithEnv @@ -1612,10 +1614,22 @@ readSourcePackageCabalFile -> BS.ByteString -> IO GenericPackageDescription readSourcePackageCabalFile verbosity pkgfilename content = + readSourcePackageCabalFile' (warn verbosity) pkgfilename content + +-- | Like `readSourcePackageCabalFile`, but the `warn` function is an argument. +-- +-- This is used when reading @.cabal@ files in indexes, where warnings should +-- generally be ignored. +readSourcePackageCabalFile' + :: (String -> IO ()) + -> FilePath + -> BS.ByteString + -> IO GenericPackageDescription +readSourcePackageCabalFile' logWarnings pkgfilename content = case runParseResult (parseGenericPackageDescription content) of (warnings, Right pkg) -> do unless (null warnings) $ - info verbosity (formatWarnings warnings) + logWarnings (formatWarnings warnings) return pkg (warnings, Left (mspecVersion, errors)) -> throwIO $ CabalFileParseError pkgfilename content errors mspecVersion warnings diff --git a/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/pkg/pkg.cabal b/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/pkg/pkg.cabal index f6ca5619b77..7e0d2a55f8a 100644 --- a/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/pkg/pkg.cabal +++ b/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/pkg/pkg.cabal @@ -1,7 +1,7 @@ name: pkg version: 1.0 build-type: Custom -cabal-version: >= 1.20 +cabal-version: 1.20 custom-setup setup-depends: base, Cabal, setup-dep == 2.* diff --git a/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/repo/setup-dep-1.0/setup-dep.cabal b/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/repo/setup-dep-1.0/setup-dep.cabal index 7e3e890b092..0a8ee4af5b0 100644 --- a/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/repo/setup-dep-1.0/setup-dep.cabal +++ b/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/repo/setup-dep-1.0/setup-dep.cabal @@ -1,7 +1,7 @@ name: setup-dep version: 1.0 build-type: Simple -cabal-version: >= 1.20 +cabal-version: 1.20 library build-depends: base diff --git a/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/setup-dep/setup-dep.cabal b/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/setup-dep/setup-dep.cabal index 47910b2eed7..6cfa8c075fd 100644 --- a/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/setup-dep/setup-dep.cabal +++ b/cabal-testsuite/PackageTests/BuildTargets/UseLocalPackageForSetup/setup-dep/setup-dep.cabal @@ -1,7 +1,7 @@ name: setup-dep version: 2.0 build-type: Simple -cabal-version: >= 1.20 +cabal-version: 1.20 library build-depends: base diff --git a/cabal-testsuite/PackageTests/IndexCabalFileParseError/cabal.out b/cabal-testsuite/PackageTests/IndexCabalFileParseError/cabal.out new file mode 100644 index 00000000000..8d891bd421e --- /dev/null +++ b/cabal-testsuite/PackageTests/IndexCabalFileParseError/cabal.out @@ -0,0 +1,19 @@ +# cabal v2-update +Downloading the latest package list from test-local-repo +Warning: In /cabal.dist/repo/my-lib-1.0.tar.gz: Errors encountered when parsing cabal file my-lib-1.0/my-lib.cabal: + +my-lib-1.0/my-lib.cabal:4:22: error: +unexpected Unknown SPDX license identifier: 'puppy' + + 3 | version: 1.0 + 4 | license: puppy license :) + | ^ + +my-lib-1.0/my-lib.cabal:5:1: warning: +Unknown field: "puppy" + + 4 | license: puppy license :) + 5 | puppy: teehee! + | ^ +Error: [Cabal-7046] +Failed to read my-lib-1.0/my-lib.cabal from archive /cabal.dist/repo/my-lib-1.0.tar.gz diff --git a/cabal-testsuite/PackageTests/IndexCabalFileParseError/cabal.test.hs b/cabal-testsuite/PackageTests/IndexCabalFileParseError/cabal.test.hs new file mode 100644 index 00000000000..05a6bfc4934 --- /dev/null +++ b/cabal-testsuite/PackageTests/IndexCabalFileParseError/cabal.test.hs @@ -0,0 +1,4 @@ +import Test.Cabal.Prelude + +main = cabalTest $ withRepoNoUpdate "repo" $ do + fails $ cabal "v2-update" [] diff --git a/cabal-testsuite/PackageTests/IndexCabalFileParseError/repo/my-lib-1.0/my-lib.cabal b/cabal-testsuite/PackageTests/IndexCabalFileParseError/repo/my-lib-1.0/my-lib.cabal new file mode 100644 index 00000000000..f37d57df406 --- /dev/null +++ b/cabal-testsuite/PackageTests/IndexCabalFileParseError/repo/my-lib-1.0/my-lib.cabal @@ -0,0 +1,5 @@ +cabal-version: 3.0 +name: my-lib +version: 1.0 +license: puppy license :) +puppy: teehee! diff --git a/cabal-testsuite/PackageTests/NewBuild/CmdRun/Script/cabal.out b/cabal-testsuite/PackageTests/NewBuild/CmdRun/Script/cabal.out index 075d530474d..54fa7f05e1c 100644 --- a/cabal-testsuite/PackageTests/NewBuild/CmdRun/Script/cabal.out +++ b/cabal-testsuite/PackageTests/NewBuild/CmdRun/Script/cabal.out @@ -1,4 +1,5 @@ # cabal v2-run +Warning: The package description file ./script.cabal has warnings: script.cabal:0:0: A package using 'cabal-version: >=1.10' must use section syntax. See the Cabal user guide for details. Configuration is affected by the following files: - cabal.project Resolving dependencies... diff --git a/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptBad/cabal.out b/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptBad/cabal.out index aa977554b57..076b432b019 100644 --- a/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptBad/cabal.out +++ b/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptBad/cabal.out @@ -1,4 +1,5 @@ # cabal v2-run +Warning: The package description file ./script.cabal has warnings: script.cabal:0:0: A package using 'cabal-version: >=1.10' must use section syntax. See the Cabal user guide for details. Configuration is affected by the following files: - cabal.project Error: [Cabal-7121] diff --git a/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptLiterate/cabal.out b/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptLiterate/cabal.out index 2c4abd1b407..5ebd9d91f5c 100644 --- a/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptLiterate/cabal.out +++ b/cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptLiterate/cabal.out @@ -1,4 +1,5 @@ # cabal v2-run +Warning: The package description file ./script.cabal has warnings: script.cabal:0:0: A package using 'cabal-version: >=1.10' must use section syntax. See the Cabal user guide for details. Configuration is affected by the following files: - cabal.project Resolving dependencies... diff --git a/cabal-testsuite/PackageTests/NewConfigure/ConfigFile/cabal.out b/cabal-testsuite/PackageTests/NewConfigure/ConfigFile/cabal.out index 1e95b7b237b..90a93d8bd2f 100644 --- a/cabal-testsuite/PackageTests/NewConfigure/ConfigFile/cabal.out +++ b/cabal-testsuite/PackageTests/NewConfigure/ConfigFile/cabal.out @@ -1,4 +1,5 @@ # cabal v2-configure +Warning: The package description file ./ConfigFile.cabal has warnings: ConfigFile.cabal:0:0: A package using 'cabal-version: >=1.10' must use section syntax. See the Cabal user guide for details. Configuration is affected by the following files: - cabal.project Config file not written due to flag(s). diff --git a/cabal-testsuite/PackageTests/NewFreeze/BuildTools/new_freeze.out b/cabal-testsuite/PackageTests/NewFreeze/BuildTools/new_freeze.out index 9674cd0cc8b..2cc44b9016b 100644 --- a/cabal-testsuite/PackageTests/NewFreeze/BuildTools/new_freeze.out +++ b/cabal-testsuite/PackageTests/NewFreeze/BuildTools/new_freeze.out @@ -1,6 +1,7 @@ # cabal v2-update Downloading the latest package list from test-local-repo # cabal v2-build +Warning: The package description file ./my-local-package.cabal has warnings: my-local-package.cabal:3:23: Packages with 'cabal-version: 1.12' or later should specify a specific version of the Cabal spec of the form 'cabal-version: x.y'. Use 'cabal-version: 1.20'. Configuration is affected by the following files: - cabal.project Resolving dependencies... @@ -16,6 +17,7 @@ Configuration is affected by the following files: Resolving dependencies... Wrote freeze file: /cabal.project.freeze # cabal v2-build +Warning: The package description file ./my-local-package.cabal has warnings: my-local-package.cabal:3:23: Packages with 'cabal-version: 1.12' or later should specify a specific version of the Cabal spec of the form 'cabal-version: x.y'. Use 'cabal-version: 1.20'. Configuration is affected by the following files: - cabal.project - cabal.project.freeze diff --git a/cabal-testsuite/PackageTests/Regression/T7234/Fail/cabal.out b/cabal-testsuite/PackageTests/Regression/T7234/Fail/cabal.out index d19a9b23a47..572c32fde7a 100644 --- a/cabal-testsuite/PackageTests/Regression/T7234/Fail/cabal.out +++ b/cabal-testsuite/PackageTests/Regression/T7234/Fail/cabal.out @@ -1,4 +1,5 @@ # cabal v2-build +Warning: The package description file ./issue7234.cabal has warnings: issue7234.cabal:13:3: The field "other-extensions" is available only since the Cabal specification version 1.10. Configuration is affected by the following files: - cabal.project Resolving dependencies... diff --git a/cabal-testsuite/PackageTests/Regression/T7234/Success/cabal.out b/cabal-testsuite/PackageTests/Regression/T7234/Success/cabal.out index 5accd75362d..2598efd4917 100644 --- a/cabal-testsuite/PackageTests/Regression/T7234/Success/cabal.out +++ b/cabal-testsuite/PackageTests/Regression/T7234/Success/cabal.out @@ -1,4 +1,5 @@ # cabal v2-build +Warning: The package description file ./issue7234.cabal has warnings: issue7234.cabal:14:3: The field "other-extensions" is available only since the Cabal specification version 1.10. Configuration is affected by the following files: - cabal.project Resolving dependencies... diff --git a/cabal-testsuite/PackageTests/Regression/T9640/cabal.out b/cabal-testsuite/PackageTests/Regression/T9640/cabal.out index e0d2968a2e3..a2b4d7da6f0 100644 --- a/cabal-testsuite/PackageTests/Regression/T9640/cabal.out +++ b/cabal-testsuite/PackageTests/Regression/T9640/cabal.out @@ -1,6 +1,7 @@ # cabal v2-update Downloading the latest package list from test-local-repo # cabal build +Warning: The package description file ./depend-on-custom-with-exe.cabal has warnings: depend-on-custom-with-exe.cabal:16:1: Ignoring trailing fields after sections: "ghc-options" Configuration is affected by the following files: - cabal.project Resolving dependencies... diff --git a/cabal-testsuite/src/Test/Cabal/Prelude.hs b/cabal-testsuite/src/Test/Cabal/Prelude.hs index d8cee954d83..6455dbb87b1 100644 --- a/cabal-testsuite/src/Test/Cabal/Prelude.hs +++ b/cabal-testsuite/src/Test/Cabal/Prelude.hs @@ -564,11 +564,9 @@ src `archiveTo` dst = do infixr 4 `archiveTo` --- | Given a directory (relative to the 'testCurrentDir') containing --- a series of directories representing packages, generate an --- external repository corresponding to all of these packages -withRepo :: FilePath -> TestM a -> TestM a -withRepo repo_dir m = do +-- | Like 'withRepo', but doesn't run @cabal update@. +withRepoNoUpdate :: FilePath -> TestM a -> TestM a +withRepoNoUpdate repo_dir m = do env <- getTestEnv -- 1. Initialize repo directory @@ -606,11 +604,7 @@ withRepo repo_dir m = do liftIO $ print $ testUserCabalConfigFile env liftIO $ print =<< readFile (testUserCabalConfigFile env) - -- 4. Update our local index - -- Note: this doesn't do anything for file+noindex repositories. - cabal "v2-update" ["-z"] - - -- 5. Profit + -- 4. Profit withReaderT (\env' -> env' { testHaveRepo = True }) m -- TODO: Arguably should undo everything when we're done... where @@ -620,6 +614,17 @@ withRepo repo_dir m = do _ -> x) else id) (testRepoDir env) +-- | Given a directory (relative to the 'testCurrentDir') containing +-- a series of directories representing packages, generate an +-- external repository corresponding to all of these packages +withRepo :: FilePath -> TestM a -> TestM a +withRepo repo_dir m = do + withRepoNoUpdate repo_dir $ do + -- Update our local index + -- Note: this doesn't do anything for file+noindex repositories. + cabal "v2-update" ["-z"] + m + -- | Given a directory (relative to the 'testCurrentDir') containing -- a series of directories representing packages, generate an -- remote repository corresponding to all of these packages @@ -798,17 +803,31 @@ recordMode mode = withReaderT (\env -> env { assertOutputContains :: MonadIO m => WithCallStack (String -> Result -> m ()) assertOutputContains needle result = withFrozenCallStack $ - unless (needle `isInfixOf` (concatOutput output)) $ + unless (needle `isInfixOf` concatOutput output) $ assertFailure $ " expected: " ++ needle where output = resultOutput result assertOutputDoesNotContain :: MonadIO m => WithCallStack (String -> Result -> m ()) assertOutputDoesNotContain needle result = withFrozenCallStack $ - when (needle `isInfixOf` (concatOutput output)) $ + when (needle `isInfixOf` concatOutput output) $ assertFailure $ "unexpected: " ++ needle where output = resultOutput result +assertOutputMatches :: MonadIO m => WithCallStack (String -> Result -> m ()) +assertOutputMatches regex result = + withFrozenCallStack $ + unless (concatOutput output =~ regex) $ + assertFailure $ "expected regex match: " ++ regex + where output = resultOutput result + +assertOutputDoesNotMatch :: MonadIO m => WithCallStack (String -> Result -> m ()) +assertOutputDoesNotMatch regex result = + withFrozenCallStack $ + when (concatOutput output =~ regex) $ + assertFailure $ "unexpected regex match: " ++ regex + where output = resultOutput result + assertFindInFile :: MonadIO m => WithCallStack (String -> FilePath -> m ()) assertFindInFile needle path = withFrozenCallStack $ diff --git a/changelog.d/pr-10647 b/changelog.d/pr-10647 new file mode 100644 index 00000000000..4a87939f8b6 --- /dev/null +++ b/changelog.d/pr-10647 @@ -0,0 +1,10 @@ +--- +synopsis: 'Improve "Cannot read .cabal file inside ..." errors' +packages: [cabal-install] +prs: 10647 +--- + +The error message printed when Cabal fails to read a `.cabal` file inside an +index (like the Hackage index) has been greatly improved. Additionally, +warnings in `.cabal` files in indexes are printed instead of being silently +discarded.