Skip to content

Commit 564b4fe

Browse files
bacchanaliaMikolaj
authored andcommitted
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
1 parent a6b99b8 commit 564b4fe

File tree

11 files changed

+54
-11
lines changed

11 files changed

+54
-11
lines changed

cabal-install/src/Distribution/Client/ScriptUtils.hs

+2-2
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,9 @@ withContextAndSelectors noTargets kind flags@NixStyleFlags{..} targetStrings glo
305305
(withGlobalConfig verbosity globalConfigFlag $ withoutProject mkTmpDir)
306306

307307
(tc', ctx', sels) <- case targetStrings of
308-
-- Only script targets may contain spaces and or end with ':'.
308+
-- Only script targets may end with ':'.
309309
-- Trying to readTargetSelectors such a target leads to a parse error.
310-
[target] | any (\c -> isSpace c) target || ":" `isSuffixOf` target -> do
310+
[target] | ":" `isSuffixOf` target -> do
311311
scriptOrError target [TargetSelectorNoScript $ TargetString1 target]
312312
_ -> do
313313
-- In the case where a selector is both a valid target and script, assume it is a target,

cabal-install/src/Distribution/Client/TargetSelector.hs

+8-6
Original file line numberDiff line numberDiff line change
@@ -324,21 +324,21 @@ parseTargetString =
324324
parseTargetApprox :: Parse.ReadP r TargetString
325325
parseTargetApprox =
326326
( do
327-
a <- tokenQ
327+
a <- tokenQEnd
328328
return (TargetString1 a)
329329
)
330330
+++ ( do
331331
a <- tokenQ0
332332
_ <- Parse.char ':'
333-
b <- tokenQ
333+
b <- tokenQEnd
334334
return (TargetString2 a b)
335335
)
336336
+++ ( do
337337
a <- tokenQ0
338338
_ <- Parse.char ':'
339339
b <- tokenQ
340340
_ <- Parse.char ':'
341-
c <- tokenQ
341+
c <- tokenQEnd
342342
return (TargetString3 a b c)
343343
)
344344
+++ ( do
@@ -348,7 +348,7 @@ parseTargetString =
348348
_ <- Parse.char ':'
349349
c <- tokenQ
350350
_ <- Parse.char ':'
351-
d <- tokenQ
351+
d <- tokenQEnd
352352
return (TargetString4 a b c d)
353353
)
354354
+++ ( do
@@ -360,7 +360,7 @@ parseTargetString =
360360
_ <- Parse.char ':'
361361
d <- tokenQ
362362
_ <- Parse.char ':'
363-
e <- tokenQ
363+
e <- tokenQEnd
364364
return (TargetString5 a b c d e)
365365
)
366366
+++ ( do
@@ -376,14 +376,16 @@ parseTargetString =
376376
_ <- Parse.char ':'
377377
f <- tokenQ
378378
_ <- Parse.char ':'
379-
g <- tokenQ
379+
g <- tokenQEnd
380380
return (TargetString7 a b c d e f g)
381381
)
382382

383383
token = Parse.munch1 (\x -> not (isSpace x) && x /= ':')
384384
tokenQ = parseHaskellString <++ token
385385
token0 = Parse.munch (\x -> not (isSpace x) && x /= ':')
386386
tokenQ0 = parseHaskellString <++ token0
387+
tokenEnd = Parse.munch1 (/= ':')
388+
tokenQEnd = parseHaskellString <++ tokenEnd
387389
parseHaskellString :: Parse.ReadP r String
388390
parseHaskellString = Parse.readS_to_P reads
389391

cabal-install/tests/IntegrationTests2.hs

+5-3
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,14 @@ testTargetSelectors reportSubCase = do
260260
":pkg:q:lib:q:file:Q.y"
261261
, "app/Main.hs", "p:app/Main.hs", "exe:ppexe:app/Main.hs", "p:ppexe:app/Main.hs",
262262
":pkg:p:exe:ppexe:file:app/Main.hs"
263+
, "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",
264+
":pkg:p:exe:pppexe:file:a p p/Main.hs"
263265
]
264266
ts @?= replicate 5 (TargetComponent "p-0.1" (CLibName LMainLibName) (FileTarget "P"))
265267
++ replicate 5 (TargetComponent "q-0.1" (CLibName LMainLibName) (FileTarget "QQ"))
266268
++ replicate 5 (TargetComponent "q-0.1" (CLibName LMainLibName) (FileTarget "Q"))
267269
++ replicate 5 (TargetComponent "p-0.1" (CExeName "ppexe") (FileTarget ("app" </> "Main.hs")))
270+
++ replicate 5 (TargetComponent "p-0.1" (CExeName "pppexe") (FileTarget ("a p p" </> "Main.hs")))
268271
-- Note there's a bit of an inconsistency here: for the single-part
269272
-- syntax the target has to point to a file that exists, whereas for
270273
-- all the other forms we don't require that.
@@ -278,9 +281,8 @@ testTargetSelectors reportSubCase = do
278281
testTargetSelectorBadSyntax :: Assertion
279282
testTargetSelectorBadSyntax = do
280283
(_, _, _, localPackages, _) <- configureProject testdir config
281-
let targets = [ "foo bar", " foo"
282-
, "foo:", "foo::bar"
283-
, "foo: ", "foo: :bar"
284+
let targets = [ "foo:", "foo::bar"
285+
, " :foo", "foo: :bar"
284286
, "a:b:c:d:e:f", "a:b:c:d:e:f:g:h" ]
285287
Left errs <- readTargetSelectors localPackages Nothing targets
286288
zipWithM_ (@?=) errs (map TargetSelectorUnrecognised targets)

cabal-install/tests/IntegrationTests2/targets/simple/a p p/Main.hs

Whitespace-only changes.

cabal-install/tests/IntegrationTests2/targets/simple/p.cabal

+5
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,8 @@ executable ppexe
1515
main-is: Main.hs
1616
hs-source-dirs: app
1717
other-modules: PMain
18+
19+
executable pppexe
20+
main-is: Main.hs
21+
hs-source-dirs: "a p p"
22+
other-modules: PMain
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
cabal-version: 3.0
2+
name: T8875
3+
version: 0.1.0.0
4+
5+
executable foo
6+
main-is: Main.hs
7+
build-depends: base
8+
hs-source-dirs: "a app"
9+
default-language: Haskell2010
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
main = return ()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# cabal v2-build
2+
Resolving dependencies...
3+
Build profile: -w ghc-<GHCVER> -O1
4+
In order, the following will be built:
5+
- T8875-0.1.0.0 (exe:foo) (first run)
6+
Configuring executable 'foo' for T8875-0.1.0.0...
7+
Preprocessing executable 'foo' for T8875-0.1.0.0...
8+
Building executable 'foo' for T8875-0.1.0.0...
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
packages: .
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import Test.Cabal.Prelude
2+
3+
main = cabalTest . void $ do
4+
-- Building a target that contains whitespace
5+
cabal' "v2-build" ["a app/Main.hs"]

changelog.d/issue-8875

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
synopsis: Allow whitespace in targets
2+
packages: cabal-install
3+
prs: #10032
4+
issues: #8875
5+
6+
description: {
7+
Allow spaces in the final component of target selectors. This resolves an issue
8+
where using absolute paths in selectors can fail if there is whitespace in the
9+
parent directories of the project.
10+
}

0 commit comments

Comments
 (0)