Parse list fields in cabal config file

[gbaz]

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.
• If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

This changes parsing of cabal config files to be more uniform with parsing of cabal project files.

Specifically, "extra-include-dirs", "extra-lib-dirs", "extra-framework-dirs", "extra-prog-path", and "configure-options" are now parsed as comma separated fields rather than single strings. This should resolve #5420 by making the syntax for this portable across systems. It is an alternative approach to #6155. Rather than writing multiple lines in config files, it brings the parsing and printing into accord by allowing parsing of comma separation (which the emitting already assumes is possible).

The entire config parser stuff is pretty antiquated and needs a full rewrite. In the meantime, this just "patches" the parsing after the fact. It makes use of the same parsers as for cabal project files, to not introduce yet another duplicate path. To do so we had to move certain "local" combinators from the cabal project file parser into their "proper" home (where the comments indicated they already should be moved as a further cleanup step). I audited the use cases for when they replace existing combinators there, and they're strictly more correct, so we should be fine.

Tested against the full suite and added a test specifically against this as well.

[randen]

Part of #5420 is that when cabal-install forks processes and sets environment variables (specifically, %PATH%), the concatenation of the paths used commas, rather than semi-colons, as required by Windows %PATH%. I have tested the changes in #6155 and the generated "extra-prog-path" appears to be properly comma-separated, and on a single line.

However, on the back-end of this, when cabal-install is making use of this extra-prog-path, to append/pre-pend those additional path(s) onto the PATH environment variable, is there a platform-specific PATH separator used? If so, then yes, this PR (and #6155) should fix #5420.

[gbaz]

@randen it would be good if you can check that empirically as well. my understanding of observed past behavior and the code is that it should work. cf this region of code:

cabal/Cabal/Distribution/Simple.hs

Lines 748 to 761 in 7a02cda

	let extraPath = fromNubList $ configProgramPathExtra flags
	let cflagsEnv = maybe (unwords ccFlags) (++ (" " ++ unwords ccFlags))
	$ lookup "CFLAGS" env
	spSep = [searchPathSeparator]
	pathEnv = maybe (intercalate spSep extraPath)
	((intercalate spSep extraPath ++ spSep)++) $ lookup "PATH" env
	overEnv = ("CFLAGS", Just cflagsEnv) :
	[("PATH", Just pathEnv) | not (null extraPath)]
	hp = hostPlatform lbi
	maybeHostFlag = if hp == buildPlatform then [] else ["--host=" ++ show (pretty hp)]
	args' = configureFile':args ++ ["CC=" ++ ccProgShort] ++ maybeHostFlag
	shProg = simpleProgram "sh"
	progDb = modifyProgramSearchPath
	(\p -> map ProgramSearchPathDir extraPath ++ p) emptyProgramDb

In particular, in the past if we kept each entry on a distinct line it would work. The problem wasn't that it was using the wrong character to join them -- it was just treating the entire list as a single list and not joining things at all. (Note this also really screwed up the internal progdb search path, which always expected a full list afaik, and not a value-separated string)

[gbaz]

(nb the one test failure appears due to a travis timeout)

[phadej]

so if Windows user wants to write extra-prog-path they should use comma, semicolon is not accepted?

[gbaz]

Yes. This patch is to allow the parser to handle comma separated fields. Semicolon was never accepted anywhere. It "just happened" to work on windows (and actually still does because this patch won't change it) because it reads in "foo; bar" as a single value, and appends that single value to the path, but the syntax matches. I.e., it works by doing an deliberate "injection" attack :-).

[phadej]

Can we change parser to recognise ; as a separator too for that field. Than I think I'll be happy.

[phadej]

... and the entry to changelog.

[gbaz]

do you want it as a separator for all fields or just to special case this one? i'd like to keep uniform syntax...

[gbaz]

As I mentioned on irc, changing the parser for filepath lists to handle ; seems unnecessary to fix this -- it just extends the already convoluted parser code still further, adding special cases not only for certain lists (we already have like four distinct list types even without this) but also tokens.

[gbaz]

i.e. the docs already say

so this patch as is doesn't change the grammar. it just fixes the fact that the docs claimed a feature that was only partially implemented.

[gbaz]

(and because, on unix and mac, the comma thing 'accidentally' worked, people didn't notice)

[gbaz]

@23Skidoo can you backport into the 3.0 branch [assuming that's what's getting released with the new ghc]? (if you prefer, you can ask me to do it).

[23Skidoo]

Cherry-picked to 3.0.