diff --git a/Cabal/src/Distribution/Simple/Configure.hs b/Cabal/src/Distribution/Simple/Configure.hs index 56d8517b3b9..9cea0fda39f 100644 --- a/Cabal/src/Distribution/Simple/Configure.hs +++ b/Cabal/src/Distribution/Simple/Configure.hs @@ -114,6 +114,7 @@ import Distribution.Types.MissingDependencyReason (MissingDependencyReason (..)) import Distribution.Types.PackageVersionConstraint import Distribution.Utils.LogProgress import Distribution.Utils.NubList +import Distribution.Utils.String (trim) import Distribution.Verbosity import Distribution.Version @@ -2397,7 +2398,6 @@ configurePkgconfigPackages verbosity pkg_descr progdb enabled pkgconfig ["--modversion", pkg] `catchIO` (\_ -> dieWithException verbosity $ PkgConfigNotFound pkg versionRequirement) `catchExit` (\_ -> dieWithException verbosity $ PkgConfigNotFound pkg versionRequirement) - let trim = dropWhile isSpace . dropWhileEnd isSpace let v = PkgconfigVersion (toUTF8BS $ trim version) if not (withinPkgconfigVersionRange v range) then dieWithException verbosity $ BadVersion pkg versionRequirement v diff --git a/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs b/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs index 84375b0f4de..a1709a0cf9f 100644 --- a/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs +++ b/cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs @@ -14,11 +14,13 @@ module Distribution.Solver.Types.ProjectConfigPath , docProjectConfigPath , docProjectConfigFiles , cyclicalImportMsg + , untrimmedUriImportMsg , docProjectConfigPathFailReason -- * Checks and Normalization , isCyclicConfigPath , isTopLevelConfigPath + , isUntrimmedUriConfigPath , canonicalizeConfigPath ) where @@ -34,6 +36,7 @@ import System.FilePath import qualified Data.List.NonEmpty as NE import Distribution.Solver.Modular.Version (VR) import Distribution.Pretty (prettyShow) +import Distribution.Utils.String (trim) import Text.PrettyPrint import Distribution.Simple.Utils (ordNub) @@ -98,9 +101,13 @@ instance Structured ProjectConfigPath -- >>> render . docProjectConfigPath $ ProjectConfigPath $ "D.config" :| ["C.config", "B.config", "A.project"] -- "D.config\n imported by: C.config\n imported by: B.config\n imported by: A.project" docProjectConfigPath :: ProjectConfigPath -> Doc -docProjectConfigPath (ProjectConfigPath (p :| [])) = text p -docProjectConfigPath (ProjectConfigPath (p :| ps)) = vcat $ - text p : [ text " " <+> text "imported by:" <+> text l | l <- ps ] +docProjectConfigPath (ProjectConfigPath (p :| [])) = quoteUntrimmed p +docProjectConfigPath (ProjectConfigPath (p :| ps)) = vcat $ quoteUntrimmed p : + [ text " " <+> text "imported by:" <+> quoteUntrimmed l | l <- ps ] + +-- | If the path has leading or trailing spaces then show it quoted. +quoteUntrimmed :: FilePath -> Doc +quoteUntrimmed s = if trim s /= s then quotes (text s) else text s -- | Renders the paths as a list without showing which path imports another, -- like this; @@ -150,6 +157,14 @@ cyclicalImportMsg path@(ProjectConfigPath (duplicate :| _)) = , nest 2 (docProjectConfigPath path) ] +-- | A message for an import that has leading or trailing spaces. +untrimmedUriImportMsg :: Doc -> ProjectConfigPath -> Doc +untrimmedUriImportMsg intro path = + vcat + [ intro <+> text "import has leading or trailing whitespace" <> semi + , nest 2 (docProjectConfigPath path) + ] + docProjectConfigPathFailReason :: VR -> ProjectConfigPath -> Doc docProjectConfigPathFailReason vr pcp | ProjectConfigPath (p :| []) <- pcp = @@ -178,6 +193,11 @@ nullProjectConfigPath = ProjectConfigPath $ "unused" :| [] isCyclicConfigPath :: ProjectConfigPath -> Bool isCyclicConfigPath (ProjectConfigPath p) = length p /= length (NE.nub p) +-- | Check if the last segment of the path (root or importee) is a URI that has +-- leading or trailing spaces. +isUntrimmedUriConfigPath :: ProjectConfigPath -> Bool +isUntrimmedUriConfigPath (ProjectConfigPath (p :| _)) = let p' = trim p in p' /= p && isURI p' + -- | Check if the project config path is top-level, meaning it was not included by -- some other project config. isTopLevelConfigPath :: ProjectConfigPath -> Bool @@ -196,7 +216,7 @@ unconsProjectConfigPath ps = fmap ProjectConfigPath <$> NE.uncons (coerce ps) makeRelativeConfigPath :: FilePath -> ProjectConfigPath -> ProjectConfigPath makeRelativeConfigPath dir (ProjectConfigPath p) = ProjectConfigPath - $ (\segment -> (if isURI segment then segment else makeRelative dir segment)) + $ (\segment@(trim -> trimSegment) -> (if isURI trimSegment then trimSegment else makeRelative dir segment)) <$> p -- | Normalizes and canonicalizes a path removing '.' and '..' indirections. @@ -273,11 +293,25 @@ makeRelativeConfigPath dir (ProjectConfigPath p) = -- return $ expected == render (docProjectConfigPath p) ++ "\n" -- :} -- True +-- +-- "A string is a valid URL potentially surrounded by spaces if, after stripping leading and trailing whitespace from it, it is a valid URL." +-- [W3C/HTML5/URLs](https://www.w3.org/TR/2010/WD-html5-20100624/urls.html) +-- +-- Trailing spaces for @ProjectConfigPath@ URLs are trimmed. +-- +-- >>> p <- canonicalizeConfigPath "" (ProjectConfigPath $ ("https://www.stackage.org/nightly-2024-12-05/cabal.config ") :| []) +-- >>> render $ docProjectConfigPath p +-- "https://www.stackage.org/nightly-2024-12-05/cabal.config" +-- +-- >>> let d = testDir +-- >>> p <- canonicalizeConfigPath d (ProjectConfigPath $ ("https://www.stackage.org/nightly-2024-12-05/cabal.config ") :| [d "cabal.project"]) +-- >>> render $ docProjectConfigPath p +-- "https://www.stackage.org/nightly-2024-12-05/cabal.config\n imported by: cabal.project" canonicalizeConfigPath :: FilePath -> ProjectConfigPath -> IO ProjectConfigPath canonicalizeConfigPath d (ProjectConfigPath p) = do - xs <- sequence $ NE.scanr (\importee -> (>>= \importer -> - if isURI importee - then pure importee + xs <- sequence $ NE.scanr (\importee@(trim -> trimImportee) -> (>>= \importer@(trim -> trimImporter) -> + if isURI trimImportee || isURI trimImporter + then pure trimImportee else canonicalizePath $ d takeDirectory importer importee)) (pure ".") p return . makeRelativeConfigPath d . ProjectConfigPath . NE.fromList $ NE.init xs diff --git a/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs b/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs index 49720fdd8ea..1b6357c335b 100644 --- a/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs +++ b/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs @@ -126,6 +126,7 @@ import Distribution.Simple.Setup import Distribution.Simple.Utils ( debug , lowercase + , noticeDoc ) import Distribution.Types.CondTree ( CondBranch (..) @@ -141,6 +142,7 @@ import Distribution.Utils.NubList , overNubList , toNubList ) +import Distribution.Utils.String (trim) import Distribution.Client.HttpUtils import Distribution.Client.ParseUtils @@ -274,6 +276,9 @@ parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (Project if isCyclicConfigPath normLocPath then pure . parseFail $ ParseUtils.FromString (render $ cyclicalImportMsg normLocPath) Nothing else do + when + (isUntrimmedUriConfigPath importLocPath) + (noticeDoc verbosity $ untrimmedUriImportMsg (Disp.text "Warning:") importLocPath) 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 @@ -342,7 +347,7 @@ parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (Project fetch pci fetch :: FilePath -> IO BS.ByteString - fetch pci = case parseURI pci of + fetch pci = case parseURI $ trim pci of Just uri -> do let fp = cacheDir map (\x -> if isPathSeparator x then '_' else x) (makeValid $ show uri) createDirectoryIfMissing True cacheDir diff --git a/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/cabal.out b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/cabal.out new file mode 100644 index 00000000000..94ff8af283a --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/cabal.out @@ -0,0 +1,30 @@ +# checking project import with trailing space +# cabal v2-build +Warning: import has leading or trailing whitespace; + 'https://www.stackage.org/nightly-2024-12-05/cabal.config ' + imported by: trailing-space.project +Configuration is affected by the following files: +- trailing-space.project +- with-ghc.config + imported by: trailing-space.project +- https://www.stackage.org/nightly-2024-12-05/cabal.config + imported by: trailing-space.project +Resolving dependencies... +Build profile: -w ghc- -O1 +In order, the following would be built: + - my-0.1 (lib:my) (first run) +# checking project import with tabs and spaces +# cabal v2-build +Warning: import has leading or trailing whitespace; + 'https://www.stackage.org/nightly-2024-12-05/cabal.config ' + imported by: tabs-and-spaces.project +Configuration is affected by the following files: +- tabs-and-spaces.project +- with-ghc.config + imported by: tabs-and-spaces.project +- https://www.stackage.org/nightly-2024-12-05/cabal.config + imported by: tabs-and-spaces.project +Resolving dependencies... +Build profile: -w ghc- -O1 +In order, the following would be built: + - my-0.1 (lib:my) (first run) diff --git a/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/cabal.test.hs b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/cabal.test.hs new file mode 100644 index 00000000000..5c27d9172ec --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/cabal.test.hs @@ -0,0 +1,14 @@ +import Test.Cabal.Prelude + +main = cabalTest . recordMode RecordMarked $ do + let log = recordHeader . pure + + log "checking project import with trailing space" + trailing <- cabal' "v2-build" [ "--dry-run", "--project-file=trailing-space.project" ] + assertOutputContains "import has leading or trailing whitespace" trailing + assertOutputContains "'https://www.stackage.org/nightly-2024-12-05/cabal.config '" trailing + + log "checking project import with tabs and spaces" + cabal "v2-build" [ "--dry-run", "--project-file=tabs-and-spaces.project" ] + + return () diff --git a/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/my.cabal b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/my.cabal new file mode 100644 index 00000000000..b1b36c1e620 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/my.cabal @@ -0,0 +1,9 @@ +name: my +version: 0.1 +license: BSD3 +cabal-version: >= 1.2 +build-type: Simple + +library + exposed-modules: Foo + build-depends: base diff --git a/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/tabs-and-spaces.project b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/tabs-and-spaces.project new file mode 100644 index 00000000000..667f4474622 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/tabs-and-spaces.project @@ -0,0 +1,3 @@ +packages: . +import: https://www.stackage.org/nightly-2024-12-05/cabal.config +import: with-ghc.config diff --git a/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/trailing-space.project b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/trailing-space.project new file mode 100644 index 00000000000..5923d9f7156 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/trailing-space.project @@ -0,0 +1,3 @@ +packages: . +import: https://www.stackage.org/nightly-2024-12-05/cabal.config +import: with-ghc.config diff --git a/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/with-ghc.config b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/with-ghc.config new file mode 100644 index 00000000000..140a00be1b9 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/with-ghc.config @@ -0,0 +1,7 @@ +-- WARNING: Override the `with-compiler: ghc-x.y.z` of the stackage import, of +-- https://www.stackage.org/nightly-yyyy-mm-dd/cabal.config. Otherwise tests +-- will fail with: +-- -Error: [Cabal-5490] +-- -Cannot find the program 'ghc'. User-specified path 'ghc-x.y.z' does not +-- refer to an executable and the program is not on the system path. +with-compiler: ghc diff --git a/changelog.d/pr-10629 b/changelog.d/pr-10629 new file mode 100644 index 00000000000..2d8259b45ef --- /dev/null +++ b/changelog.d/pr-10629 @@ -0,0 +1,13 @@ +--- +synopsis: "Report trailing spaces in project import URIs" +packages: [cabal-install, cabal-install-solver] +prs: 10629 +issues: 10622 +--- + +> A string is a valid URL potentially surrounded by spaces if, after stripping +> leading and trailing whitespace from it, it is a valid URL." +> SOURCE: [W3C/HTML5/URLs](https://www.w3.org/TR/2010/WD-html5-20100624/urls.html) + +Fixes a problem of mistaking a URI for a file path when it has trailing spaces +and warn about such trailing spaces. diff --git a/fix-whitespace.yaml b/fix-whitespace.yaml index 28b62b12ac5..fc3605d136c 100644 --- a/fix-whitespace.yaml +++ b/fix-whitespace.yaml @@ -95,6 +95,8 @@ excluded-files: - Cabal-tests/tests/ParserTests/warnings/tab.cabal - Cabal-tests/tests/ParserTests/warnings/utf8.cabal - cabal-testsuite/PackageTests/Regression/T8507/pkg.cabal + - cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/trailing-space.project + - cabal-testsuite/PackageTests/ProjectImport/UntrimmedImport/tabs-and-spaces.project # These also contain tabs that affect the golden value: # Could be removed from exceptions, but then the tab warning