cpphs 1.20.5 broken when using GHC 8.2.1 on Windows

[asr]

On Linux, Installing Agda with cpphs 1.20.4, I got a large number of messages of the following shape:

$ cabal get Agda
$ cd Agda
$ cabal install
...
Warning: Can't find file "/usr/local/stow/ghc-8.0.2/lib/ghc-8.0.2/include/ghcversion.h" in directories

  Asked for by: src/full/Agda/Auto/Auto.hs  at line 2 col 1

I didn't get those warning when using cpphs 1.20.3.

Blocking https://github.com/agda/agda/issues/2496.

[domenkozar]

What version of directory do you have? Try with 1.3.1.0

[asr]

What version of directory do you have?

GHC 8.0.2 and directory 1.3.0.0.

Try with 1.3.1.0

I got the same warning after installing cpphs 1.20.4 with directory 1.3.1.0.

[asr]

A MWE:

$ cat Test.hs

module Main where

#define FOO 1

#ifdef FOO
foo = 42
#endif

main = print foo

$ ghc -cpp -pgmPcpphs -optP--cpp Test.hs 
Warning: Can't find file "/usr/local/stow/ghc-8.0.2/lib/ghc-8.0.2/include/ghcversion.h" in directories

  Asked for by: Test.hs  at line 1 col 1

[jmitchell]

Due to issue #9, the most recent suggestion is to apply this patch https://github.com/jmitchell/cpphs-blackbox/blob/master/avoid-using-findfile_-ghc-7_4-doesn_t-include-it_.dpatch.

@asr, for the next few days I'm unable to do any testing, but you could try filing a PR to https://github.com/jmitchell/cpphs-blackbox if you want to test particular behaviors of the proposed patch.

[jmitchell]

Currently awaiting final approval and merge from @malcolmwallace. See https://github.com/malcolmwallace/cpphs/issues/9#issuecomment-285122025.

[asr]

Due to issue #9, the most recent suggestion is to apply this patch https://github.com/jmitchell/cpphs-blackbox/blob/master/avoid-using-findfile_-ghc-7_4-doesn_t-include-it_.dpatch.

I tested the above patch on Linux using GHC 8.0.2 and it also fix this issue. Thanks @jmitchell.

[RyanGlScott]

This is an especially serious problem on GHC 8.2.1, since include/ghcversions.h is the only place where __GLASGOW_HASKELL__ is defined. In other words, using GHC 8.2.1 and cpphs-1.20.4 together on any code that relies on preprocessing __GLASGOW_HASKELL__ is totally broken at the moment. See https://ghc.haskell.org/trac/ghc/ticket/13553

I've confirmed that applying @jmitchell's patch in https://github.com/malcolmwallace/cpphs/issues/11#issuecomment-288493485 makes GHC 8.2.1 work with cpphs-1.20.4 again.

@malcolmwallace, can we please get another release of cpphs with this fix? Without it, it is impossible to build many libraries (including Agda) using GHC 8.2.1.

[asr]

Blocking https://github.com/agda/agda/issues/2539.

[malcolmwallace]

I would like to withdraw cpphs-1.20.4 from Hackage, rolling back to 1.20.3 as the latest version. However, I can't work out a way to do that.

[RyanGlScott]

@malcolmwallace: You can deprecate releases on Hackage on going to the cpphs page, clicking "edit package information" (about halfway towards the bottom), then clicking "Preferred versions", and then clicking the button corresponding to the 1.20.4 release under "Deprecated versions" and clicking "Set status".

But even if you do that, I still urge you to make a new release with the patch in https://github.com/malcolmwallace/cpphs/issues/11#issuecomment-288493485. If you roll back to cpphs-1.20.3, then this GHC 8.2.1 issue will still be present, but it will be on Windows only (as originally reported in #8).

[malcolmwallace]

Excellent. Thanks for the simple guide to deprecating a package release on Hackage. I did also make a fresh release 1.20.5, which is identical to 1.20.3 in all but version number.

[domenkozar]

@malcolmwallace would you be willing to migrate to github so we can setup linux/windows/macos CI? To prevent such issues in the future.

[RyanGlScott]

@malcolmwallace: As I feared, cpphs-1.20.5 is completely broken when using GHC 8.2.1 (or HEAD) on Windows. Here is the error you get when trying to compile this file:

{-# LANGUAGE CPP #-}
module Bug where

foo :: Int
#if __GLASGOW_HASKELL__ >= 800
foo = 42
#else
foo = "wrong type"
#endif

with cpphs-1.20.5 on GHC HEAD on Windows:

$ ../../../Software/ghc/inplace/bin/ghc-stage2.exe Bug.hs -pgmP cpphs -optP --cpp
Warning: Can't find file "C:/Users/RyanGlScott/Software/ghc/includes\ghcversion.h" in directories
        .
        C:\Users\RyanGlScott\Software\ghc\libraries\base\include
        C:\Users\RyanGlScott\Software\ghc\libraries\integer-gmp\include
        C:/Users/RyanGlScott/Software/ghc/rts/dist/build
        C:/Users/RyanGlScott/Software/ghc/includes
        C:/Users/RyanGlScott/Software/ghc/includes/dist-derivedconstants/header
  Asked for by: Bug.hs  at line 1 col 1
[1 of 1] Compiling Bug              ( Bug.hs, Bug.o )

Bug.hs:8:7: error:
    * Couldn't match expected type `Int' with actual type `[Char]'
    * In the expression: "wrong type"
      In an equation for `foo': foo = "wrong type"
  |
8 | foo = "wrong type"
  |       ^^^^^^^^^^^^

[RyanGlScott]

You can fix the issue by applying the following changes to cpphs-1.20.5:

diff --git a/Language/Preprocessor/Cpphs/ReadFirst.hs b/Language/Preprocessor/Cpphs/ReadFirst.hs
index 4cfdeb3..3a608c2 100644
--- a/Language/Preprocessor/Cpphs/ReadFirst.hs
+++ b/Language/Preprocessor/Cpphs/ReadFirst.hs
@@ -19,6 +19,7 @@ module Language.Preprocessor.Cpphs.ReadFirst

 import System.IO
 import System.Directory (doesFileExist)
+import System.FilePath  (isRelative, (</>))
 import Data.List        (intersperse)
 import Control.Exception as E
 import Control.Monad    (when)
@@ -36,27 +37,35 @@ readFirst :: String             -- ^ filename
               , String
               )                 -- ^ discovered filepath, and file contents

-readFirst name demand path warn =
-    case name of
-       nm@(c:':':_) -> try nm [""]  -- Windows absolute path
-       '/':nm -> try nm   [""]
-       _      -> try name (cons dd (".":path))
+readFirst name demand paths warn = do
+  case (isRelative name, directory demand) of
+    (False, _) -> try name []
+    (_, "") -> try name (".":paths)
+    (_, dd) -> try name (dd:".":paths)
   where
-    dd = directory demand
-    cons x xs = if null x then xs else x:xs
-    try name [] = do
-        when warn $
+    try name ds = do
+      file <- findFile ds name
+      case file of
+        Just f -> do
+          content <- readFileUTF8 f
+          return (f,content)
+        Nothing -> do
           hPutStrLn stderr ("Warning: Can't find file \""++name
                            ++"\" in directories\n\t"
-                           ++concat (intersperse "\n\t" (cons dd (".":path)))
+                           ++concat (intersperse "\n\t" ds)
                            ++"\n  Asked for by: "++show demand)
-        return ("missing file: "++name,"")
-    try name (p:ps) = do
-        let file = cleanPath p++'/':cleanPath name
-        ok <- doesFileExist file
-        if not ok then try name ps
-          else do content <- readFileUTF8 file
-                  return (file,content)
+          return ("missing file: "++name,"")
+    findFile [] fname = do
+      exists <- doesFileExist fname
+      if (not $ isRelative fname) && exists
+        then do return $ Just fname
+        else do return Nothing
+    findFile (d:ds) fname = do
+      let path = d </> fname
+      exists <- doesFileExist path
+      if exists
+        then do return $ Just path
+        else findFile ds fname

 readFileUTF8 :: FilePath -> IO String
 readFileUTF8 file = do
diff --git a/cpphs.cabal b/cpphs.cabal
index 4aa45f0..85c3ecb 100644
--- a/cpphs.cabal
+++ b/cpphs.cabal
@@ -24,7 +24,7 @@ Build-type: Simple
 Extra-Source-Files: README, LICENCE-GPL, LICENCE-commercial, CHANGELOG, docs/cpphs.1, docs/index.html

 Library
-    Build-Depends: base>3&&<6, old-locale, old-time, directory, polyparse>=1.9
+    Build-Depends: base>3&&<6, old-locale, old-time, directory, filepath, polyparse>=1.9
     Exposed-Modules:
         Language.Preprocessor.Cpphs
         Language.Preprocessor.Unlit
@@ -40,7 +40,7 @@ Library
         Language.Preprocessor.Cpphs.Tokenise

 Executable cpphs
-    Build-Depends: base>3&&<6, old-locale, old-time, directory, polyparse>=1.9
+    Build-Depends: base>3&&<6, old-locale, old-time, directory, filepath, polyparse>=1.9
     Main-Is: cpphs.hs
     Other-Modules:
         Language.Preprocessor.Cpphs

I wasn't able to make a darcs patch, since the code located at http://code.haskell.org/cpphs/ is out of date (it's still on version 1.20.4).

[RyanGlScott]

Any update on this, @malcolmwallace? cpphs is still completely broken for me on GHC 8.2 because of this issue.

[malcolmwallace]

OK, so I am deeply confused about what exactly is the issue here. I do not have ghc-8.2, and for the sake of clarity, I think the best characterisation of the bug report would be a minimal single-module example, with a literal #include that fails. I tried the MWE example above, but it works for me with ghc-7.6.1 and cpphs-1.20.6, possibly because the example has no #include and relies on ghc to supply one.

[RyanGlScott]

OK, so there are a couple of factors here.

This issue is only present on GHC 8.2 or later, ever since this GHC commit dropped a redundant (re)definition of the __GLASGOW_HASKELL__ macro. Now, the only place it is defined is in include/ghcversion.h (or include\ghcversion.h on Windows), so it is crucial that cpphs be able to find this file.

Now on Linux, this works still happens to work with GHC 8.2. But on Windows it doesn't, because findFile isn't properly designed to handle Windows-style path separators, which is why all of these warnings mention include\ghcversion.h with a backslash, not a forward slash. (Before, it only worked on Windows due to a fluke, since __GLASGOW_HASKELL__ happened to be defined twice, and cpphs miraculously picked it up. But in GHC 8.2, you can't rely on that fluke anymore.)

So again, this is in fact a minimal working example:

{-# LANGUAGE CPP #-}
module Bug where

foo :: Int
#if __GLASGOW_HASKELL__ >= 800
foo = 42
#else
foo = "wrong type"
#endif

But it will only fail with Windows on GHC 8.2.

The fix is to simply patch findFile to be Windows-aware, which I describe how to do in https://github.com/malcolmwallace/cpphs/issues/11#issuecomment-294299458.

[RyanGlScott]

Unfortunately, I don't think it's possible to create a test case which reproduces the issue on Linux, since if you put #include "include\ghcversion.h" in a file and pass it to cpphs, then it will simply be normalized to include/ghcversion.h (obviously, this doesn't happen on Windows).

[malcolmwallace]

I'm still a bit confused, because cpphs does not use findFile. However, I think I managed to come up with a simpler example that fails with any ghc, and involves Windows drive letters:

include "C:/workspace/ext/ghc-7.8.4/lib/include/ghcconfig.h"

The problem was that my readFirst function was inserting a leading slash in front of the drive letter. I have pushed a fix which seems to work for me, as cpphs-1.20.7. Please give it a try.

[RyanGlScott]

That still doesn't work for me:

$ ~/Software/ghc-8.2.1-rc2/bin/ghc Bug.hs -pgmP cpphs -optP --cpp
Warning: Can't find file "\Users\RyanGlScott\Software\ghc-8.2.1-rc2\lib/include\ghcversion.h" in directories
        .
        C:\Users\RyanGlScott\Software\ghc-8.2.1-rc2\lib\base-4.10.0.0\include
        C:\Users\RyanGlScott\Software\ghc-8.2.1-rc2\lib\integer-gmp-1.0.0.1\include
        C:\Users\RyanGlScott\Software\ghc-8.2.1-rc2\lib/include
  Asked for by: Bug.hs  at line 1 col 1
[1 of 1] Compiling Bug              ( Bug.hs, Bug.o )

Bug.hs:8:7: error:
    * Couldn't match expected type `Int' with actual type `[Char]'
    * In the expression: "wrong type"
      In an equation for `foo': foo = "wrong type"
  |
8 | foo = "wrong type"
  |       ^^^^^^^^^^^^

$ cpphs --version
cpphs.exe 1.20.7

The only difference is now it warns about \Users\RyanGlScott\Software\ghc-8.2.1-rc2\lib/include\ghcversion.h. instead of C:/Users/RyanGlScott/Software/ghc/includes\ghcversion.h.

I'm still a bit confused, because cpphs does not use findFile.

Sorry, I meant to refer to readFirst there. I mentioned findFile since the solution to this problem is to backport the findFile function that was introduced in a fairly recent version of filepath and use it in readFirst, as shown in https://github.com/malcolmwallace/cpphs/issues/11#issuecomment-294299458.

[malcolmwallace]

RIght. I forgot a case (leading backslash). Try 1.20.8?

[RyanGlScott]

Things appear to be working with cpphs-1.20.8. (I also built Agda successfully with it, just to be sure.)

Thanks, @malcolmwallace!

[asr]

I also built Agda on Linux without problems. Thanks @malcolmwallace and @RyanGlScott.

[asr]

Why not marked as deprecated versions 1.20.5, 1.20.6 and 1.20.7 in Hackage? It would avoid to report some unnecessary issues here and everywhere.