diff --git a/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs b/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs index b98d493656c..16fbcf51610 100644 --- a/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs +++ b/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs @@ -12,6 +12,7 @@ module Distribution.Solver.Types.ProjectConfigPath , docProjectConfigPath , docProjectConfigPaths , cyclicalImportMsg + , duplicateImportMsg , docProjectConfigPathFailReason -- * Checks and Normalization @@ -102,13 +103,24 @@ docProjectConfigPaths :: [ProjectConfigPath] -> Doc docProjectConfigPaths ps = vcat [ text "-" <+> text p | ProjectConfigPath (p :| _) <- ps ] --- | A message for a cyclical import, assuming the head of the path is the --- duplicate. +-- | A message for a cyclical import, a "cyclical import of". cyclicalImportMsg :: ProjectConfigPath -> Doc -cyclicalImportMsg path@(ProjectConfigPath (duplicate :| _)) = +cyclicalImportMsg path@(ProjectConfigPath (duplicate :| _)) = seenImportMsg "cyclical" duplicate path [] + +-- | A message for a duplicate import, a "duplicate import of". +duplicateImportMsg :: FilePath -> ProjectConfigPath -> [(FilePath, ProjectConfigPath)] -> Doc +duplicateImportMsg = seenImportMsg "duplicate" + +seenImportMsg :: String -> FilePath -> ProjectConfigPath -> [(FilePath, ProjectConfigPath)] -> Doc +seenImportMsg seen duplicate path seenImportsBy = vcat - [ text "cyclical import of" <+> text duplicate <> semi + [ text seen <+> text "import of" <+> text duplicate <> semi , nest 2 (docProjectConfigPath path) + , nest 2 $ + vcat + [ docProjectConfigPath dib + | (_, dib) <- filter ((duplicate ==) . fst) seenImportsBy + ] ] docProjectConfigPathFailReason :: VR -> ProjectConfigPath -> Doc diff --git a/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs b/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs index 49720fdd8ea..72f527cf35d 100644 --- a/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs +++ b/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs @@ -1,6 +1,7 @@ {-# LANGUAGE ConstraintKinds #-} {-# LANGUAGE DataKinds #-} {-# LANGUAGE DeriveGeneric #-} +{-# LANGUAGE MultiWayIf #-} {-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE RecordWildCards #-} {-# LANGUAGE ScopedTypeVariables #-} @@ -33,6 +34,7 @@ module Distribution.Client.ProjectConfig.Legacy ) where import Data.Coerce (coerce) +import Data.IORef import Distribution.Client.Compat.Prelude import Distribution.Types.Flag (FlagName, parsecFlagAssignment) @@ -137,7 +139,8 @@ import Distribution.Types.CondTree ) import Distribution.Types.SourceRepo (RepoType) import Distribution.Utils.NubList - ( fromNubList + ( NubList + , fromNubList , overNubList , toNubList ) @@ -244,13 +247,16 @@ parseProject parseProject rootPath cacheDir httpTransport verbosity configToParse = do let (dir, projectFileName) = splitFileName rootPath projectDir <- makeAbsolute dir - projectPath <- canonicalizeConfigPath projectDir (ProjectConfigPath $ projectFileName :| []) - parseProjectSkeleton cacheDir httpTransport verbosity projectDir projectPath configToParse + projectPath@(ProjectConfigPath (canonicalRoot :| _)) <- canonicalizeConfigPath projectDir (ProjectConfigPath $ projectFileName :| []) + importsBy <- newIORef $ toNubList [(canonicalRoot, projectPath)] + parseProjectSkeleton cacheDir httpTransport verbosity importsBy projectDir projectPath configToParse parseProjectSkeleton :: FilePath -> HttpTransport -> Verbosity + -> IORef (NubList (FilePath, ProjectConfigPath)) + -- ^ The imports seen so far, useful for reporting on duplicates and to detect duplicates that are not cycles -> FilePath -- ^ The directory of the project configuration, typically the directory of cabal.project -> ProjectConfigPath @@ -258,7 +264,7 @@ parseProjectSkeleton -> ProjectConfigToParse -- ^ The contents of the file to parse -> IO (ParseResult ProjectConfigSkeleton) -parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (ProjectConfigToParse bs) = +parseProjectSkeleton cacheDir httpTransport verbosity importsBy projectDir source (ProjectConfigToParse bs) = (sanityWalkPCS False =<<) <$> liftPR (go []) (ParseUtils.readFields bs) where go :: [ParseUtils.Field] -> [ParseUtils.Field] -> IO (ParseResult ProjectConfigSkeleton) @@ -266,19 +272,26 @@ parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (Project (ParseUtils.F _ "import" importLoc) -> do let importLocPath = importLoc `consProjectConfigPath` source - -- Once we canonicalize the import path, we can check for cyclical imports - normLocPath <- canonicalizeConfigPath projectDir importLocPath + -- Once we canonicalize the import path, we can check for cyclical and duplicate imports + normLocPath@(ProjectConfigPath (uniqueImport :| _)) <- canonicalizeConfigPath projectDir importLocPath + seenImportsBy@(fmap fst -> seenImports) <- fromNubList <$> atomicModifyIORef' importsBy (\ibs -> (toNubList [(uniqueImport, normLocPath)] <> ibs, ibs)) debug verbosity $ "\nimport path, normalized\n=======================\n" ++ render (docProjectConfigPath normLocPath) - - if isCyclicConfigPath normLocPath - then pure . parseFail $ ParseUtils.FromString (render $ cyclicalImportMsg normLocPath) Nothing - else do - normSource <- canonicalizeConfigPath projectDir source - let fs = (\z -> CondNode z [normLocPath] mempty) <$> fieldsToConfig normSource (reverse acc) - res <- parseProjectSkeleton cacheDir httpTransport verbosity projectDir importLocPath . ProjectConfigToParse =<< fetchImportConfig normLocPath - rest <- go [] xs - pure . fmap mconcat . sequence $ [fs, res, rest] + debug verbosity "\nseen unique paths\n=================" + mapM_ (debug verbosity) seenImports + debug verbosity "\n" + + if + | isCyclicConfigPath normLocPath -> + pure . parseFail $ ParseUtils.FromString (render $ cyclicalImportMsg normLocPath) Nothing + | uniqueImport `elem` seenImports -> do + pure . parseFail $ ParseUtils.FromString (render $ duplicateImportMsg uniqueImport normLocPath seenImportsBy) Nothing + | otherwise -> do + normSource <- canonicalizeConfigPath projectDir source + let fs = (\z -> CondNode z [normLocPath] mempty) <$> fieldsToConfig normSource (reverse acc) + res <- parseProjectSkeleton cacheDir httpTransport verbosity importsBy projectDir importLocPath . ProjectConfigToParse =<< fetchImportConfig normLocPath + rest <- go [] xs + pure . fmap mconcat . sequence $ [fs, res, rest] (ParseUtils.Section l "if" p xs') -> do subpcs <- go [] xs' let fs = singletonProjectConfigSkeleton <$> fieldsToConfig source (reverse acc) diff --git a/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.out b/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.out index c2690ee4366..df9786ec44c 100644 --- a/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.out +++ b/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.out @@ -254,131 +254,17 @@ Could not resolve dependencies: (constraint from oops-0.project requires ==1.4.3.0) [__1] fail (backjumping, conflict set: hashable, oops) After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble fulfilling: hashable (3), oops (2) -# checking if we detect when the same config is imported via many different paths (we don't) +# checking that we detect when the same config is imported via many different paths # cabal v2-build -Configuration is affected by the following files: -- yops-0.project -- yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops-4.config - imported by: yops/yops-3.config - imported by: yops-0.project -- yops-4.config - imported by: yops/yops-3.config - imported by: yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops-6.config - imported by: yops/yops-5.config - imported by: yops-0.project -- yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-0.project -- yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops-8.config - imported by: yops/yops-7.config - imported by: yops-0.project -- yops-8.config - imported by: yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-0.project -- yops-8.config - imported by: yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-0.project -- yops-8.config - imported by: yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops/yops-1.config - imported by: yops-0.project -- yops/yops-3.config - imported by: yops-0.project -- yops/yops-3.config - imported by: yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops/yops-5.config - imported by: yops-0.project -- yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-0.project -- yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops/yops-7.config - imported by: yops-0.project -- yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-0.project -- yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-0.project -- yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config - imported by: yops-2.config - imported by: yops/yops-1.config - imported by: yops-0.project -- yops/yops-9.config - imported by: yops-0.project -- yops/yops-9.config - imported by: yops-8.config - imported by: yops/yops-7.config - imported by: yops-0.project -- yops/yops-9.config - imported by: yops-8.config - imported by: yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-0.project -- yops/yops-9.config - imported by: yops-8.config - imported by: yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config +Error: [Cabal-7090] +Error parsing project file /yops-0.project: +duplicate import of yops/yops-3.config; + yops/yops-3.config imported by: yops-0.project -- yops/yops-9.config - imported by: yops-8.config - imported by: yops/yops-7.config - imported by: yops-6.config - imported by: yops/yops-5.config - imported by: yops-4.config - imported by: yops/yops-3.config + yops/yops-3.config imported by: yops-2.config imported by: yops/yops-1.config imported by: yops-0.project -Up to date # checking bad conditional # cabal v2-build Error: [Cabal-7090] diff --git a/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.test.hs b/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.test.hs index 67118d362c0..d404658ae93 100644 --- a/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.test.hs +++ b/cabal-testsuite/PackageTests/ConditionalAndImport/cabal.test.hs @@ -252,11 +252,9 @@ main = cabalTest . withRepo "repo" . recordMode RecordMarked $ do -- +-- yops-8.config -- +-- yops/yops-9.config (no further imports) -- +-- yops/yops-9.config (no further imports) - -- - -- We don't check and don't error or warn on the same config being imported - -- via many different paths. - log "checking if we detect when the same config is imported via many different paths (we don't)" - yopping <- cabal' "v2-build" [ "--project-file=yops-0.project" ] + log "checking that we detect when the same config is imported via many different paths" + yopping <- fails $ cabal' "v2-build" [ "--project-file=yops-0.project" ] + assertOutputContains (normalizeWindowsOutput "duplicate import of yops/yops-3.config") yopping log "checking bad conditional" badIf <- fails $ cabal' "v2-build" [ "--project-file=bad-conditional.project" ] diff --git a/changelog.d/pr-9933 b/changelog.d/pr-9933 new file mode 100644 index 00000000000..d8a6c1d2337 --- /dev/null +++ b/changelog.d/pr-9933 @@ -0,0 +1,23 @@ +synopsis: Detect non-cyclical duplicate project imports +description: + Detect and report on duplicate imports that are non-cyclical and expand the + detail in cyclical import reporting, being more explicit and consistent with + non-cyclical duplicate reporting. + + ``` + $ cabal build --project-file=yops-0.project + ... + Error: [Cabal-7090] + Error parsing project file yops-0.project: + duplicate import of yops/yops-3.config; + yops/yops-3.config + imported by: yops-0.project + yops/yops-3.config + imported by: yops-2.config + imported by: yops/yops-1.config + imported by: yops-0.project + ``` + +packages: cabal-install-solver cabal-install +prs: #9578 #9933 +issues: #9562