malcolmwallace / cpphs

The C pre-processor, implemented in Haskell.
2 stars 0 forks source link

Warning: Can't find file "C:\.../include\ghcversion.h" in directories #8

Closed domenkozar closed 7 years ago

domenkozar commented 7 years ago

I'm not sure yet where this is coming from, but I'd like to report an unnecessary warning on Windows:

[00:03:39] Preprocessing library cardano-sl-0.1.0.0...
[00:03:39] Warning: Can't find file "C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib/include\ghcversion.h" in directories
[00:03:39]  src/Pos
[00:03:39]  .
[00:03:39]  .stack-work\dist\b7fec021\build
[00:03:39]  .stack-work\dist\b7fec021\build
[00:03:39]  .stack-work\dist\b7fec021\build\autogen
[00:03:39]  .stack-work\dist\b7fec021\build
[00:03:39]  C:\OpenSSL-Win64\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\msys2-20150512\mingw64\include
[00:03:39]  C:\projects\pos-haskell-prototype\rocksdb\include
[00:03:39]  C:\OpenSSL-Win64\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\msys2-20150512\mingw64\include
[00:03:39]  C:\projects\pos-haskell-prototype\rocksdb\include
[00:03:39]  C:\sr\snapshots\a78c6a89\lib\x86_64-windows-ghc-8.0.1\vector-algorithms-0.7.0.1-8R8UpWgvBC926XMxBjYPpx\include
[00:03:39]  C:\sr\snapshots\a78c6a89\lib\x86_64-windows-ghc-8.0.1\zlib-0.6.1.2-4CWLN1T27kOJhNvXgy46ZV\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\process-1.4.2.0\include
[00:03:39]  C:\sr\snapshots\a78c6a89\lib\x86_64-windows-ghc-8.0.1\vector-0.11.0.0-BEDZb5o2QOhGbIm6ky7rl6\include
[00:03:39]  C:\sr\snapshots\a78c6a89\lib\x86_64-windows-ghc-8.0.1\old-time-1.1.0.3-IcvdkJUsE9M8t3io8peAEp\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\directory-1.2.6.2\include
[00:03:39]  C:\sr\snapshots\a78c6a89\lib\x86_64-windows-ghc-8.0.1\primitive-0.6.1.0-Ip44DqhfCp21tTUYbecwa\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\time-1.6.0.1\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\Win32-2.3.1.1\include
[00:03:39]  C:\sr\snapshots\a78c6a89\lib\x86_64-windows-ghc-8.0.1\network-2.6.3.1-nK9qnsiJR03CWuPIGMmX\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\bytestring-0.10.8.1\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\base-4.9.0.0\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib\integer-gmp-1.0.0.1\include
[00:03:39]  C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib/include
[00:03:39]   Asked for by: src/Pos/CLI.hs  at line 2 col 1

It appears to trigger for each module using cpphs. The file is present, so I suspect the problem is in unix path character in C:\Users\appveyor\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.1\lib/include\ghcversion.h.

EDIT: I suspect that this bug comes from the packaging and cpphs is just called with --include by stack/cabal. Need to find a way to see how cpphs is invoked.

domenkozar commented 7 years ago

Reopening since it seems to actually be a bug in readFirst function within cpphs, according to https://ghc.haskell.org/trac/ghc/ticket/13166

Mistuke commented 7 years ago

Yes, the error is in this part:

readFirst name demand path warn =
    case name of
       '/':nm -> try nm   [""]
       _      -> try name (cons dd (".":path))

for Unix path it assumes that when / is used the path is rooted and then doesn't add a prefix to the path. This function however does not account for Windows paths. It then only tries paths with prefixes, so the file is never found. Possible fix can be to use System.FilePath.hasDrive for this detection instead of checking for /. Although this entire function can just as well be replaced by System.Directory.findFile or findFileWith which correctly abstract away the platform differences.

domenkozar commented 7 years ago

cc @malcolmwallace thoughts?

Mistuke commented 7 years ago

If no response after a while then perhaps emailing the maintainer directly would produce better results. He may not be monitoring github, seeing as the source is still in darcs.

malcolmwallace commented 7 years ago

Can you try the freshly released cpphs-1.20.3, and let me know if it fixes the problem?

domenkozar commented 7 years ago

@malcolmwallace doesn't appear to fix the issue (page takes a while to load due to log highlighting): https://ci.appveyor.com/project/jagajaga/pos-haskell-prototype/build/0.1.1964#L22890

Mistuke commented 7 years ago

Yeah, further down try invalidates the path again

    try name (p:ps) = do
        let file = cleanPath p++'/':cleanPath name
        ok <- doesFileExist file

This makes C:\... into /C:\..

domenkozar commented 7 years ago

Indeed, that line is redundant on Windows.

@malcolmwallace thoughts? I fail to understand what's the purpose of prepending a slash.

Mistuke commented 7 years ago

It's attempting to reconstruct a path from the prefix and the include path. It just so happens that when the prefix is "/" on unix you end up with // and it still works

jmitchell commented 7 years ago

I've created a patch that seems to work well in both Windows and Linux. To test it out:

darcs get http://code.haskell.org/cpphs
darcs apply windows-path-support.dpatch
stack init
stack install
cpphs --include=<file_name> -I<include_dir1> -I<include-dir2> ...

@Mistuke, thank you for your tip about System.Directory.findFile. I reimplemented readFirst in terms of it, while (hopefully) preserving existing side effects and first-directories-to-try logic.

domenkozar commented 7 years ago

@jmitchell could you create a plain patch? darcs patch fails for me:

C:\pdfname\cpphs>darcs apply patch.txt
darcs: bug at src\Darcs\Patch\Depends.hs:374 compiled Feb  1 2017 19:09:24
Failed to commute common patches:
patch f450def45c7a19836570a16a0537b852d7523f8c
Author: Malcolm.Wallace@me.com
Date:   Mon Feb  6 18:55:48 Coordinated Universal Time 2017
domenkozar commented 7 years ago

I applied the patch manually and can confirm it fixes the issue on Windows for pdfname package, no more warnings.

@malcolmwallace could we get cpphs-1.20.4 out with the patch above? Thanks!

domenkozar commented 7 years ago

:pray:

jmitchell commented 7 years ago

Are there any concerns that need to be addressed before this is integrated? Happy to take feedback.

malcolmwallace commented 7 years ago

Patch applied. cpphs-1.20.4 released. Thanks!

domenkozar commented 7 years ago

Strange, using the patch worked, but using cpphs-1.20-4 I'm still getting the warning:

C:\pdfname>stack build --test
pdfname-0.1: configure (exe)
Configuring pdfname-0.1...
pdfname-0.1: build (exe)
Preprocessing executable 'pdfname' for pdfname-0.1...
Warning: Can't find file "C:\Users\Administrator\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.2\lib/include\ghcversion.h" in directories

  Asked for by: src\\\\Main.hs  at line 2 col 1
[1 of 1] Compiling Main             ( src\Main.hs, .stack-work\dist\ca59d0ab\build\pdfname\pdfname-tmp\Main.o )
Warning: Can't find file "C:\Users\Administrator\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.2\lib/include\ghcversion.h" in directories

  Asked for by: src\\\\Main.hs  at line 2 col 1
Linking .stack-work\dist\ca59d0ab\build\pdfname\pdfname.exe ...
pdfname-0.1: copy/register
Installing executable(s) in C:\pdfname\.stack-work\install\401d7a24\bin

I'll double check it's not using the old cpphs.

domenkozar commented 7 years ago

@jmitchell

C:\pdfname>cpphs Setup.hs --include=C:\Users\Administrator\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.2\lib\include\ghcversion.h
Warning: Can't find file "C:\Users\Administrator\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.2\lib\include\ghcversion.h" in directories

  Asked for by: Setup.hs  at line 1 col 1
#line 1 "Setup.hs"
#line 1 "missing file: C:\\Users\\Administrator\\AppData\\Local\\Programs\\stack\\x86_64-windows\\ghc-8.0.2\\lib\\include\\ghcversion.h"
#line 2 "Setup.hs"
#line 1 "Setup.hs"
import Distribution.Simple
main = defaultMain

C:\pdfname>dir C:\Users\Administrator\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.2\lib\include\ghcversion.h
 Volume in drive C has no label.
 Volume Serial Number is EA7A-F1C6

 Directory of C:\Users\Administrator\AppData\Local\Programs\stack\x86_64-windows\ghc-8.0.2\lib\include

01/12/2017  08:21 PM               560 ghcversion.h
               1 File(s)            560 bytes
               0 Dir(s)  67,110,219,776 bytes free

C:\pdfname>cpphs --version
cpphs 1.20.4
jmitchell commented 7 years ago

@domenkozar, thanks for catching this! Based on initial experiments it seems absolute paths in Windows aren't working. I recall testing \some\path\file.xyz, but must have forgot to try C:\some\path\file.xyz. My mistake. I'll make a fix.

@malcolmwallace, thank you for taking the time to make a release. I'll test the next patch more extensively.

jmitchell commented 7 years ago

I've filed an issue with System.Directory to address the surprising behavior of findFile on absolute paths in Windows.

Whether they make any changes or not, the following patch fixes the issue by moving the drive letter to the head of the directories to search and stripping it from the filename. It's admittedly not elegant, but it resolves the issue @domenkozar discovered.

C:\x\pdfname\3rdparty\cpphs>darcs whatsnew
hunk ./Language/Preprocessor/Cpphs/ReadFirst.hs 22
-import System.FilePath  (isRelative)
+import System.FilePath  (dropDrive,hasDrive,isRelative,takeDrive)
hunk ./Language/Preprocessor/Cpphs/ReadFirst.hs 48
-      file <- findFile ds name
+      file <-
+        if hasDrive name then
+          findFile (takeDrive name:ds) (dropDrive name)
+        else
+          findFile ds name
jmitchell commented 7 years ago

@malcolmwallace, the above change has been tested when --include is an absolute path (C:\tmp\Test.hs), a rooted directory without the drive (\tmp\Test.hs), and with relative paths (Test.hs, tmp\Test.hs, and ..\tmp\Test.hs).

domenkozar commented 7 years ago

directory-1.3.1.0 and cpphs-1.20.4 fix the issue, thanks @jmitchell @malcolmwallace

asr commented 7 years ago

It seems the fix to this Windows issue generated the same problem on Linux, see #11.