Skip to content

Commit

Permalink
Merge pull request #10032 from bacchanalia/8875-whitespace-in-target
Browse files Browse the repository at this point in the history
Allow whitespace in target selectors
  • Loading branch information
mergify[bot] authored May 26, 2024
2 parents a6b99b8 + 564b4fe commit 94277d1
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 11 deletions.
4 changes: 2 additions & 2 deletions cabal-install/src/Distribution/Client/ScriptUtils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 8 additions & 6 deletions cabal-install/src/Distribution/Client/TargetSelector.hs
Original file line number Diff line number Diff line change
Expand Up @@ -324,21 +324,21 @@ 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
a <- tokenQ0
_ <- Parse.char ':'
b <- tokenQ
_ <- Parse.char ':'
c <- tokenQ
c <- tokenQEnd
return (TargetString3 a b c)
)
+++ ( do
Expand All @@ -348,7 +348,7 @@ parseTargetString =
_ <- Parse.char ':'
c <- tokenQ
_ <- Parse.char ':'
d <- tokenQ
d <- tokenQEnd
return (TargetString4 a b c d)
)
+++ ( do
Expand All @@ -360,7 +360,7 @@ parseTargetString =
_ <- Parse.char ':'
d <- tokenQ
_ <- Parse.char ':'
e <- tokenQ
e <- tokenQEnd
return (TargetString5 a b c d e)
)
+++ ( do
Expand All @@ -376,14 +376,16 @@ parseTargetString =
_ <- Parse.char ':'
f <- tokenQ
_ <- Parse.char ':'
g <- tokenQ
g <- tokenQEnd
return (TargetString7 a b c d e f g)
)

token = Parse.munch1 (\x -> not (isSpace x) && x /= ':')
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

Expand Down
8 changes: 5 additions & 3 deletions cabal-install/tests/IntegrationTests2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
Empty file.
5 changes: 5 additions & 0 deletions cabal-install/tests/IntegrationTests2/targets/simple/p.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions cabal-testsuite/PackageTests/NewBuild/T8875/T8875.cabal
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/NewBuild/T8875/a app/Main.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
main = return ()
8 changes: 8 additions & 0 deletions cabal-testsuite/PackageTests/NewBuild/T8875/cabal.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# cabal v2-build
Resolving dependencies...
Build profile: -w ghc-<GHCVER> -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...
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/NewBuild/T8875/cabal.project
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: .
5 changes: 5 additions & 0 deletions cabal-testsuite/PackageTests/NewBuild/T8875/cabal.test.hs
Original file line number Diff line number Diff line change
@@ -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"]
10 changes: 10 additions & 0 deletions changelog.d/issue-8875
Original file line number Diff line number Diff line change
@@ -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.
}

0 comments on commit 94277d1

Please sign in to comment.