From 564b4fe6dea7a577c48ebdccbc8f319de1450fcc Mon Sep 17 00:00:00 2001 From: Zoe Zuser Date: Sat, 18 May 2024 22:39:09 -0400 Subject: [PATCH] Allow whitespace in target selectors Disallowing whitespace while parsing target selectors incorrectly disallows file targets with whitespace in the paths, which can issues when users pass absolute paths. fixes #8875 --- .../src/Distribution/Client/ScriptUtils.hs | 4 ++-- .../src/Distribution/Client/TargetSelector.hs | 14 ++++++++------ cabal-install/tests/IntegrationTests2.hs | 8 +++++--- .../IntegrationTests2/targets/simple/a p p/Main.hs | 0 .../tests/IntegrationTests2/targets/simple/p.cabal | 5 +++++ .../PackageTests/NewBuild/T8875/T8875.cabal | 9 +++++++++ .../PackageTests/NewBuild/T8875/a app/Main.hs | 1 + .../PackageTests/NewBuild/T8875/cabal.out | 8 ++++++++ .../PackageTests/NewBuild/T8875/cabal.project | 1 + .../PackageTests/NewBuild/T8875/cabal.test.hs | 5 +++++ changelog.d/issue-8875 | 10 ++++++++++ 11 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 cabal-install/tests/IntegrationTests2/targets/simple/a p p/Main.hs create mode 100644 cabal-testsuite/PackageTests/NewBuild/T8875/T8875.cabal create mode 100644 cabal-testsuite/PackageTests/NewBuild/T8875/a app/Main.hs create mode 100644 cabal-testsuite/PackageTests/NewBuild/T8875/cabal.out create mode 100644 cabal-testsuite/PackageTests/NewBuild/T8875/cabal.project create mode 100644 cabal-testsuite/PackageTests/NewBuild/T8875/cabal.test.hs create mode 100644 changelog.d/issue-8875 diff --git a/cabal-install/src/Distribution/Client/ScriptUtils.hs b/cabal-install/src/Distribution/Client/ScriptUtils.hs index aeae4eaf459..d4f152a4557 100644 --- a/cabal-install/src/Distribution/Client/ScriptUtils.hs +++ b/cabal-install/src/Distribution/Client/ScriptUtils.hs @@ -305,9 +305,9 @@ withContextAndSelectors noTargets kind flags@NixStyleFlags{..} targetStrings glo (withGlobalConfig verbosity globalConfigFlag $ withoutProject mkTmpDir) (tc', ctx', sels) <- case targetStrings of - -- Only script targets may contain spaces and or end with ':'. + -- Only script targets may end with ':'. -- Trying to readTargetSelectors such a target leads to a parse error. - [target] | any (\c -> isSpace c) target || ":" `isSuffixOf` target -> do + [target] | ":" `isSuffixOf` target -> do scriptOrError target [TargetSelectorNoScript $ TargetString1 target] _ -> do -- In the case where a selector is both a valid target and script, assume it is a target, diff --git a/cabal-install/src/Distribution/Client/TargetSelector.hs b/cabal-install/src/Distribution/Client/TargetSelector.hs index 856168103d1..f287400e1e1 100644 --- a/cabal-install/src/Distribution/Client/TargetSelector.hs +++ b/cabal-install/src/Distribution/Client/TargetSelector.hs @@ -324,13 +324,13 @@ parseTargetString = parseTargetApprox :: Parse.ReadP r TargetString parseTargetApprox = ( do - a <- tokenQ + a <- tokenQEnd return (TargetString1 a) ) +++ ( do a <- tokenQ0 _ <- Parse.char ':' - b <- tokenQ + b <- tokenQEnd return (TargetString2 a b) ) +++ ( do @@ -338,7 +338,7 @@ parseTargetString = _ <- Parse.char ':' b <- tokenQ _ <- Parse.char ':' - c <- tokenQ + c <- tokenQEnd return (TargetString3 a b c) ) +++ ( do @@ -348,7 +348,7 @@ parseTargetString = _ <- Parse.char ':' c <- tokenQ _ <- Parse.char ':' - d <- tokenQ + d <- tokenQEnd return (TargetString4 a b c d) ) +++ ( do @@ -360,7 +360,7 @@ parseTargetString = _ <- Parse.char ':' d <- tokenQ _ <- Parse.char ':' - e <- tokenQ + e <- tokenQEnd return (TargetString5 a b c d e) ) +++ ( do @@ -376,7 +376,7 @@ parseTargetString = _ <- Parse.char ':' f <- tokenQ _ <- Parse.char ':' - g <- tokenQ + g <- tokenQEnd return (TargetString7 a b c d e f g) ) @@ -384,6 +384,8 @@ parseTargetString = tokenQ = parseHaskellString <++ token token0 = Parse.munch (\x -> not (isSpace x) && x /= ':') tokenQ0 = parseHaskellString <++ token0 + tokenEnd = Parse.munch1 (/= ':') + tokenQEnd = parseHaskellString <++ tokenEnd parseHaskellString :: Parse.ReadP r String parseHaskellString = Parse.readS_to_P reads diff --git a/cabal-install/tests/IntegrationTests2.hs b/cabal-install/tests/IntegrationTests2.hs index 6579b2ddcc2..3598b357d69 100644 --- a/cabal-install/tests/IntegrationTests2.hs +++ b/cabal-install/tests/IntegrationTests2.hs @@ -260,11 +260,14 @@ testTargetSelectors reportSubCase = do ":pkg:q:lib:q:file:Q.y" , "app/Main.hs", "p:app/Main.hs", "exe:ppexe:app/Main.hs", "p:ppexe:app/Main.hs", ":pkg:p:exe:ppexe:file:app/Main.hs" + , "a p p/Main.hs", "p:a p p/Main.hs", "exe:pppexe:a p p/Main.hs", "p:pppexe:a p p/Main.hs", + ":pkg:p:exe:pppexe:file:a p p/Main.hs" ] ts @?= replicate 5 (TargetComponent "p-0.1" (CLibName LMainLibName) (FileTarget "P")) ++ replicate 5 (TargetComponent "q-0.1" (CLibName LMainLibName) (FileTarget "QQ")) ++ replicate 5 (TargetComponent "q-0.1" (CLibName LMainLibName) (FileTarget "Q")) ++ replicate 5 (TargetComponent "p-0.1" (CExeName "ppexe") (FileTarget ("app" "Main.hs"))) + ++ replicate 5 (TargetComponent "p-0.1" (CExeName "pppexe") (FileTarget ("a p p" "Main.hs"))) -- Note there's a bit of an inconsistency here: for the single-part -- syntax the target has to point to a file that exists, whereas for -- all the other forms we don't require that. @@ -278,9 +281,8 @@ testTargetSelectors reportSubCase = do testTargetSelectorBadSyntax :: Assertion testTargetSelectorBadSyntax = do (_, _, _, localPackages, _) <- configureProject testdir config - let targets = [ "foo bar", " foo" - , "foo:", "foo::bar" - , "foo: ", "foo: :bar" + let targets = [ "foo:", "foo::bar" + , " :foo", "foo: :bar" , "a:b:c:d:e:f", "a:b:c:d:e:f:g:h" ] Left errs <- readTargetSelectors localPackages Nothing targets zipWithM_ (@?=) errs (map TargetSelectorUnrecognised targets) diff --git a/cabal-install/tests/IntegrationTests2/targets/simple/a p p/Main.hs b/cabal-install/tests/IntegrationTests2/targets/simple/a p p/Main.hs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/cabal-install/tests/IntegrationTests2/targets/simple/p.cabal b/cabal-install/tests/IntegrationTests2/targets/simple/p.cabal index 75b24e0eadc..33ffc0d3a5f 100644 --- a/cabal-install/tests/IntegrationTests2/targets/simple/p.cabal +++ b/cabal-install/tests/IntegrationTests2/targets/simple/p.cabal @@ -15,3 +15,8 @@ executable ppexe main-is: Main.hs hs-source-dirs: app other-modules: PMain + +executable pppexe + main-is: Main.hs + hs-source-dirs: "a p p" + other-modules: PMain diff --git a/cabal-testsuite/PackageTests/NewBuild/T8875/T8875.cabal b/cabal-testsuite/PackageTests/NewBuild/T8875/T8875.cabal new file mode 100644 index 00000000000..9372c1977f9 --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T8875/T8875.cabal @@ -0,0 +1,9 @@ +cabal-version: 3.0 +name: T8875 +version: 0.1.0.0 + +executable foo + main-is: Main.hs + build-depends: base + hs-source-dirs: "a app" + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/NewBuild/T8875/a app/Main.hs b/cabal-testsuite/PackageTests/NewBuild/T8875/a app/Main.hs new file mode 100644 index 00000000000..b3549c2fe3d --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T8875/a app/Main.hs @@ -0,0 +1 @@ +main = return () diff --git a/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.out b/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.out new file mode 100644 index 00000000000..7bb94dd545c --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.out @@ -0,0 +1,8 @@ +# cabal v2-build +Resolving dependencies... +Build profile: -w ghc- -O1 +In order, the following will be built: + - T8875-0.1.0.0 (exe:foo) (first run) +Configuring executable 'foo' for T8875-0.1.0.0... +Preprocessing executable 'foo' for T8875-0.1.0.0... +Building executable 'foo' for T8875-0.1.0.0... diff --git a/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.project b/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.project new file mode 100644 index 00000000000..e6fdbadb439 --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.project @@ -0,0 +1 @@ +packages: . diff --git a/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.test.hs b/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.test.hs new file mode 100644 index 00000000000..a9120447cb9 --- /dev/null +++ b/cabal-testsuite/PackageTests/NewBuild/T8875/cabal.test.hs @@ -0,0 +1,5 @@ +import Test.Cabal.Prelude + +main = cabalTest . void $ do + -- Building a target that contains whitespace + cabal' "v2-build" ["a app/Main.hs"] diff --git a/changelog.d/issue-8875 b/changelog.d/issue-8875 new file mode 100644 index 00000000000..6642609a0e6 --- /dev/null +++ b/changelog.d/issue-8875 @@ -0,0 +1,10 @@ +synopsis: Allow whitespace in targets +packages: cabal-install +prs: #10032 +issues: #8875 + +description: { +Allow spaces in the final component of target selectors. This resolves an issue +where using absolute paths in selectors can fail if there is whitespace in the +parent directories of the project. +}