Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow whitespace in target selectors (backport #10032) #10083

Merged
merged 1 commit into from
Jun 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -300,9 +300,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 @@ -327,21 +327,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 @@ -351,7 +351,7 @@ parseTargetString =
_ <- Parse.char ':'
c <- tokenQ
_ <- Parse.char ':'
d <- tokenQ
d <- tokenQEnd
return (TargetString4 a b c d)
)
+++ ( do
Expand All @@ -363,7 +363,7 @@ parseTargetString =
_ <- Parse.char ':'
d <- tokenQ
_ <- Parse.char ':'
e <- tokenQ
e <- tokenQEnd
return (TargetString5 a b c d e)
)
+++ ( do
Expand All @@ -379,14 +379,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.
}
Loading