Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use stdout to communicate arguments #90

Merged
merged 5 commits into from
Nov 18, 2019
Merged

Conversation

mpickering
Copy link
Collaborator

Now all the cradles communicate by reading the value of HIE_BIOS_OUTPUT
which is a filepath, and writing the arguments to that temporary file.
This makes all the cradles more robust about the build tools spurting
things to stdout.

@mpickering
Copy link
Collaborator Author

Can someone please test on windows? Also need to test the stack cradle. It should be sufficient to run hie-bios debug in order to test that the right arguments are returned.

I locally tested both wrappers (cabal and cabal.hs) and also the bios cradle.

@fendor fendor mentioned this pull request Nov 15, 2019
14 tasks
@mpickering
Copy link
Collaborator Author

@jinwoo I have changed the bios cradle in this PR so you have to output the options to a file given on the HIE_BIOS_OUTPUT env var. Is this interface ok for you?

@jneira
Copy link
Member

jneira commented Nov 16, 2019

Hi i've tested this version with a stack gradle in windows 10 and i am getting an error:

PS D:\dev\ws\haskell\stack-test> hie-bios check .\src\MyLib.hs
cradle
  Cradle {cradleRootDir = "D:\\dev\\ws\\haskell\\stack-test", cradleOptsProg = CradleAction: stack}
[1 of 1] Compiling Main             ( C:\TEMP\wra1104.hs, C:\TEMP\wra1104.o )
Linking C:\TEMP\wra1105.exe ...
setTargets
  (.\src\MyLib.hs , .\src\MyLib.hs)
  [*.\src\MyLib.hs]
modGraph
  [ModSummary {
      ms_hs_date = 2019-11-16 09:37:02.2444937 UTC
      ms_mod = MyLib,
      ms_textual_imps = [(Nothing, Prelude)]
      ms_srcimps = []
   }]
modGraph
  [ModLocation {ml_hs_file = Just ".\\src\\MyLib.hs", ml_hi_file = "C:\\Users\\atrey\\AppData\\Local\\haskell-ide-engine\\1c4f925fc9fb40767202021de0bd97c68f3cc0c8\\MyLib.hi", ml_obj_file = "D:\\dev\\ws\\haskell\\stack-test\\.stack-work\\odir\\MyLib.o"}]
hidir
  Just [C, :, \, U, s, e, r, s, \, a, t, r, e, y, \, A, p, p, D, a,
        t, a, \, L, o, c, a, l, \, h, a, s, k, e, l, l, -, i, d, e, -, e,
        n, g, i, n, e, \, 1, c, 4, f, 9, 2, 5, f, c, 9, f, b, 4, 0, 7, 6,
        7, 2, 0, 2, 0, 2, 1, d, e, 0, b, d, 9, 7, c, 6, 8, f, 3, c, c, 0,
        c, 8]
PS D:\dev\ws\haskell\stack-test> hie-bios.no-stdout.exe check .\src\MyLib.hs
hie-bios.no-stdout.exe: hie-bios.no-stdout.exe: can't find a package database at D:\bin\stack\x86_64-windows\ghc-8.6.5\lib\package.conf.d
PS D:\dev\ws\haskell\stack-test> hie-bios.no-stdout.exe debug .\src\MyLib.hs
Cradle failed to load
Exit Code: ExitFailure 1
Stderr: Failed to parse result of calling cabal
�[0mError parsing targets: Unknown local package: D
Note that to specify options to be passed to GHCi, use the --ghci-options flag�[0m

It seems the error is caused by windows paths...
The project has a stack.yaml and a cabal.project. Creating a hie.yaml with cradle: {stack} doesn't change the behaviour.

@jneira
Copy link
Member

jneira commented Nov 16, 2019

Some tests using directly the wrapper:

D:\dev\ws\haskell\stack-test>C:\Users\atrey\AppData\Local\hie-bios\wrapper-0.3.0.exe
wrapper-0.3.0.exe: HIE_BIOS_OUTPUT: getEnv: does not exist (no environment variable)

D:\dev\ws\haskell\stack-test>set HIE_BIOS_OUTPUT=C:\Users\atrey\AppData\Local\hie-bios\hie.bios.out

D:\dev\ws\haskell\stack-test>C:\Users\atrey\AppData\Local\hie-bios\wrapper-0.3.0.exe
ghc: no input files
Usage: For basic information, try the `--help' option.

D:\dev\ws\haskell\stack-test>C:\Users\atrey\AppData\Local\hie-bios\wrapper-0.3.0.exe src\MyLib.hs
[1 of 1] Compiling MyLib            ( src\MyLib.hs, src\MyLib.o )

D:\dev\ws\haskell\stack-test>C:\Users\atrey\AppData\Local\hie-bios\wrapper-0.3.0.exe --interactive src\MyLib.hs
"C:\\Users\\atrey\\AppData\\Local\\hie-bios\\hie.bios.out"

D:\dev\ws\haskell\stack-test>type C:\Users\atrey\AppData\Local\hie-bios\hie.bios.out
D:\dev\ws\haskell\stack-test
--interactive
src\MyLib.hs

@jneira jneira mentioned this pull request Nov 16, 2019
Now all the cradles communicate by reading the value of HIE_BIOS_OUTPUT
which is a filepath, and writing the arguments to that temporary file.
This makes all the cradles more robust about the build tools spurting
things to stdout.
@mpickering
Copy link
Collaborator Author

Thanks, I pushed up a fix if you can try that at some point.

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didnt test it yet by hand, but other than that it looks good!

src/HIE/Bios/Cradle.hs Outdated Show resolved Hide resolved
src/HIE/Bios/Cradle.hs Outdated Show resolved Hide resolved
exe/Main.hs Outdated Show resolved Hide resolved
src/HIE/Bios/Internal/Debug.hs Show resolved Hide resolved
@jinwoo
Copy link
Contributor

jinwoo commented Nov 16, 2019

@jinwoo I have changed the bios cradle in this PR so you have to output the options to a file given on the HIE_BIOS_OUTPUT env var. Is this interface ok for you?

Yeah, this is fine by me. Thanks for the heads up, @mpickering

@jneira
Copy link
Member

jneira commented Nov 16, 2019

Thanks, I pushed up a fix if you can try that at some point.

@mpickering: i had to add CradleError in the export list of Hie.Bios to compile this.
Then i've got the same error so i traced the ghc-pkg args:
pkg_ghc_args:["-package-db","D:\\dev\\ws\\haskell\\stack-test\\.stack-work\\install\\33169d5a\\pkgdb","-package-db","C:\\sr\\snapshots\\67a70798\\pkgdb","-package-db","D:\\bin\\stack\\x86_64-windows\\ghc-8.6.5\\lib\\package.conf.d\r"]

Note that the rest of paths are readed correctly and the last one has a r at the end.

@jneira
Copy link
Member

jneira commented Nov 17, 2019

With the last commit, hie-bios check works correctly (the \r is gone) but hie-bios debug still fails with the same error:

PS D:\dev\ws\haskell\stack-test> hie-bios check .\src\MyLib.hs
pkg_ghc_args["-package-db","D:\\dev\\ws\\haskell\\stack-test\\.stack-work\\install\\33169d5a\\pkgdb","-package-db","C:\\sr\\snapshots\\67a70798\\pkgdb","-package-db","D:\\bin\\stack\\x86_64-windows\\ghc-8.6.5\\lib\\package.conf.d"]
src\MyLib.hs:4:12:• Variable not in scope: putStrLnX :: [Char] -> IO ()
• Perhaps you meant ‘putStrLn’ (imported from Prelude)
PS D:\dev\ws\haskell\stack-test> hie-bios debug .\src\MyLib.hs
pkg_ghc_args["-package-db","D:\\dev\\ws\\haskell\\stack-test\\.stack-work\\install\\33169d5a\\pkgdb","-package-db","C:\\sr\\snapshots\\67a70798\\pkgdb","-package-db","D:\\bin\\stack\\x86_64-windows\\ghc-8.6.5\\lib\\package.conf.d"]
Cradle failed to load
Exit Code: ExitFailure 1
Stderr: Failed to parse result of calling cabal
�[0mError parsing targets: Unknown local package: D
Note that to specify options to be passed to GHCi, use the --ghci-options flag�[0m

Both commands are calling stackAction (where i am tracing pkg_ghc_args) and the package paths are correct for both.

@jneira
Copy link
Member

jneira commented Nov 17, 2019

i've tried d700b2b but the error persists with cabal debug 😟

@jneira
Copy link
Member

jneira commented Nov 17, 2019

I've added some traces to readProcessWithOutputFile:

PS D:\dev\ws\haskell\stack-test> hie-bios debug .\src\MyLib.hs
readProcessWithOutputFile: fp="stack", args=["repl","--no-nix-pure","--no-load","--with-ghc","C:\\Users\\atrey\\AppData\\Local\\hie-bios\\wrapper-0.3.0.exe","D:\\dev\\ws\\haskell\\stack-test"]
readProcessWithOutputFile: res="", stde=["\ESC[0mError parsing targets: Unknown local package: D","Note that to specify options to be passed to GHCi, use the --ghci-options flag\ESC[0m"], stdo=[]
readProcessWithOutputFile: fp="stack", args=["path","--ghc-package-path"]
readProcessWithOutputFile: res="", stde=[], stdo=["D:\\dev\\ws\\haskell\\stack-test\\.stack-work\\install\\33169d5a\\pkgdb;C:\\sr\\snapshots\\67a70798\\pkgdb;D:\\bin\\stack\\x86_64-windows\\ghc-8.6.5\\lib\\package.conf.d"]
Cradle failed to load
Exit Code: ExitFailure 1
Stderr: Failed to parse result of calling cabal
�[0mError parsing targets: Unknown local package: D
Note that to specify options to be passed to GHCi, use the --ghci-options flag�[0m

It seems you cant pass a path to a directory to stack repl?

PS D:\dev\ws\haskell\stack-test> stack repl  --no-load --with-ghc C:\Users\atrey\AppData\Local\hie-bios\wrapper-0.3.0.exe D:\dev\ws\haskell\stack-test
Error parsing targets: Unknown local package: D

@mpickering
Copy link
Collaborator Author

@jneira Does hie-bios check work? I think this is a bug with the debug command.

@jneira
Copy link
Member

jneira commented Nov 17, 2019

@mpickering yeah, hie-bios check works fine since 40445c7
hi-bios debug is failing in master too so it seems not be related with this pr

@mpickering mpickering merged commit 4daabb3 into master Nov 18, 2019
mpickering added a commit that referenced this pull request Nov 18, 2019
Adding basic tests for the cradle loading and GHC API logic. The tests are far from complete but they have already showed up some issues with #90 

We really need to enable CI for windows as well as that has been a very tricky platform to support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants