Incorrect special character handling in Haddock response files

[snoyberg]

This has been causing us quite some trouble with our Stackage builds (see: commercialhaskell/stackage#1018). I recommend considering this a major issue worthy of an emergency patch, since the following combination seems to always cause failed Haddock builds:

• Newest released Cabal (1.22.6.0)
• Upcoming Haddock release (> 2.16.1)
• Any cabal file with a newline in the synopsis

When generating a response file for the Haddock arguments, the current code in Cabal simply calls hPutStrLn on the argument without any escaping. When an argument has a newline (like the synopsis), anything after the newline is treated as its own argument.

Solution: use response file escaping to pass in all arguments. As an example, that's how I implemented response file support in GHC, see: ghc/ghc@296bc70#diff-782eb6a039b579ec019e5a4009f45058R1281

[ezyang]

Reproduced.

[ezyang]

If I understand correctly, we can't fix this until haskell/haddock#379 is fixed, because Haddock does not yet support the standard GCC response file format.

[23Skidoo]

/cc @randen

[ezyang]

If you like, we can disable response files for now until Haddock fixes this.

[23Skidoo]

Yes, needs to be fixed in Haddock first.

[ezyang]

The relevant code to fix is Distribution.Simple.Haddock, in renderArgs. Here's a hokey patch that doesn't work:

--- a/Cabal/Distribution/Simple/Haddock.hs
+++ b/Cabal/Distribution/Simple/Haddock.hs
@@ -497,7 +497,7 @@ renderArgs verbosity tmpFileOpts version comp args k = do
                  withTempFileEx tmpFileOpts outputDir "haddock-response.txt" $
                     \responseFileName hf -> do
                          when haddockSupportsUTF8 (hSetEncoding hf utf8)
-                         mapM_ (hPutStrLn hf) renderedArgs
+                         mapM_ (hPutStrLn hf . escape) renderedArgs
                          hClose hf
                          let respFile = "@" ++ responseFileName
                          k ([respFile], result)
@@ -515,6 +515,19 @@ renderArgs verbosity tmpFileOpts version comp args k = do
               pkgstr = display $ packageName pkgid
               pkgid = arg argPackageName
       arg f = fromFlag $ f args
+      -- This was cribbed from compiler/main/Systools.hs
+      escape x = concat
+          [ "\""
+          , concatMap
+              (\c ->
+                  case c of
+                      '\\' -> "\\\\"
+                      '\n' -> "\\n"
+                      '\"' -> "\\\""
+                      _    -> [c])
+              x
+          , "\""
+          ]

[randen]

Arrrgv! Right, so even with cabal doing the escaping, e.g., snoyberg's ghc patch for response-file-generation, the called program must also be aware of the escaping and do the unescaping. Haddock > 2.16.1 was only used in building the docs for the recent HP releases on exactly one machine, so that is why this problem has laid dormant until we are now post-7.10.3.

Is escaping backslashes, newlines, and double-quote sufficient? When supported, we encode into UTF-8 in the above quoted code, but is anyone aware of any other caveats that are not obvious to me?

Short of reading the gcc code (e.g., the 2005 gcc patch referenced by https://gcc.gnu.org/wiki/Response_Files), is anyone aware of an actual "spec" for the " gcc -compatible syntax" of a response file?

edit: If we disable response file generation in cabal, as a work-around, it has not been depended upon by anyone, yet (except the HP build).

[23Skidoo]

Short of reading the gcc code is anyone aware of an actual "spec" for the " gcc -compatible syntax" of a response file?

This part of gcc's source is actually quite readable and well-commented, so I suggest doing exactly that.

[randen]

This may sound gross, but what if we do something like base64-encode each argument chunk, separating each by newlines? No escaping necessary (other than whatever the receiving program expects for its own argument parameters, such as whether it allows newlines, etc.). Not "standard" (like there is a standard for this) but should be completely reliable and robust. Since it is only for cabal-haddock interaction, it is rather limited. Would humans build such a response file? Since getting all the escaping right manually would likely involve a tool of some kind to be reliable and repeatable (no one is going to hand-create a response file and get it right the first time), it is not so onerous to have the user use a base64-encoding of each argument chunk. Also, the manual use case is probably not really the requirement here, is it? Anyway, I'm working on this problem, the gcc-spec-way, but please consider this other alternative for fun.

[23Skidoo]

I'd much prefer to solve this the standard way. People may want to inspect Cabal-generated response files; other tools/scripts may want to generate them.

[23Skidoo]

For anyone following this ticket: the issue is fixed in Cabal >= 1.22.7.0 and Haddock >= 2.16.2. Specifically, the version of Haddock that comes with GHC 8 RC2 (2.17.0) includes the fix.

[geraldus]

@23Skidoo do you mean

Haddock >= 2.16.2 … GHC 8 RC2 (2.17.0) …

?

[23Skidoo]

Thanks, fixed.

[randen]

To complete the story for archeologists (including myself--I wasn't able to find it immediately), I should have linked the PR that was used for the Haddock part of this: haskell/haddock/pull/470