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

Add component root directory #166

Merged
merged 2 commits into from
Apr 25, 2020
Merged

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Apr 13, 2020

Supersedes #157

Implements the suggestion to wait until GHC is done parsing its flag to modify certain filepaths in DynFlags if they arent already absolute paths.

Con: Ghc options now may contain relative filepaths.
Pro: fixes bugs.

@fendor fendor requested review from jneira and mpickering April 13, 2020 13:47
@fendor fendor changed the title WIP: Add component root directory Add component root directory Apr 13, 2020
@fendor fendor force-pushed the dont-mangle-ghc-options branch from 3973d66 to 8bc55af Compare April 13, 2020 14:21
src/HIE/Bios/Environment.hs Outdated Show resolved Hide resolved
@jneira
Copy link
Member

jneira commented Apr 13, 2020

Hi, i am testing this version in the hls repo and i've got no error about the missing header, but neither compiler warnings or errors:

PS D:\dev\ws\haskell\hls> hie-bios check .\ghcide\src\Development\IDE\Core\Compile.hs      
hie-bios.exe: `gcc.exe' failed in phase `C pre-processor'. (Exit code: 1)
PS D:\dev\ws\haskell\hls> hie-bios.patch check .\ghcide\src\Development\IDE\Core\Compile.hs
hie-bios.patch.exe: module ‘Development.IDE.Compat’ cannot be found locally
.. rest of ghcide modules
module ‘Development.IDE.Plugin.Completions.Types’ cannot be found locally
PS D:\dev\ws\haskell\hls> cd .\ghcide\
PS D:\dev\ws\haskell\hls\ghcide> hie-bios check .\src\Development\IDE\Core\Compile.hs
src\Development\IDE\Core\Compile.hs:116:5:Empty 'do' block
PS D:\dev\ws\haskell\hls\ghcide> hie-bios.patch check .\src\Development\IDE\Core\Compile.hs
src\Development\IDE\Core\Compile.hs:116:5:Empty 'do' block

PS D:\dev\ws\haskell\hls> hie-bios.patch debug .\ghcide\src\Development\IDE\Core\Compile.hs: https://gist.github.com/jneira/0ee4c6c29fadaaa7f92f4348205f8b90

The ghc options now has -isrc instead -ighcide\src so it can't find the modules (and it continues having -Iinclude instead -Ighcide\include)

@fendor
Copy link
Collaborator Author

fendor commented Apr 13, 2020

@jneira Thanks for the feedback!
I have yet to test it in depth! Apparently, I messed something up :)

@fendor
Copy link
Collaborator Author

fendor commented Apr 14, 2020

With ghcide based on "multi-rebase", it seemed to work fine. Do you have a branch with HIE where you fixed the compilation that I can take a look at?

@jneira
Copy link
Member

jneira commented Apr 14, 2020

Do you have a branch with HIE where you fixed the compilation that I can take a look at?

I tried hls (current master, based on multi-rebase) at first but then i observed the result of calling directly hie-bios check .\ghcide\src\Development\IDE\Core\Compile.hs is the same.

could be the difference ghcide is a subproject for hls?

@fendor
Copy link
Collaborator Author

fendor commented Apr 14, 2020

I dont know. Do you have a branch for it?
EDIT: Even if there is only a relative path for the hie-bios patch.

@jneira
Copy link
Member

jneira commented Apr 14, 2020

Mmm, i am not sure if i understand. I am in the project root dir of haskell-language-server, after checking out actual master:

PS D:\dev\ws\haskell\hls> cd ..
PS D:\dev\ws\haskell> cd hie-bios
PS D:\dev\ws\haskell\hie-bios> git status
On branch dont-mangle-ghc-options
Your branch is up to date with 'fendor/dont-mangle-ghc-options'.

nothing to commit, working tree clean
PS D:\dev\ws\haskell\hie-bios> cabal install
Wrote tarball sdist to
D:\dev\ws\haskell\hie-bios\dist-newstyle\sdist\hie-bios-0.4.0.tar.gz
Resolving dependencies...
Up to date
Copying 'hie-bios.exe'
PS D:\dev\ws\haskell\hie-bios> cd ..
PS D:\dev\ws\haskell> cd hls
PS D:\dev\ws\haskell\hls> git status
On branch master
Your branch is up to date with 'upstream/master'.

nothing to commit, working tree clean
PS D:\dev\ws\haskell\hls> hie-bios check .\ghcide\src\Development\IDE\Core\Compile.hs
hie-bios.exe: module ‘Development.IDE.Compat’ cannot be found locally
.....
module ‘Development.IDE.Plugin.Completions.Types’ cannot be found locally

@jneira
Copy link
Member

jneira commented Apr 15, 2020

@fendor the last commit makes hie-bios debug .\ghcide\...\Compile.hs works like a charm, many thanks!

I am gonna test it using as a lib in hls.

@jneira
Copy link
Member

jneira commented Apr 15, 2020

I've adapted hls to this branch and it can load ghcide modules! great work 👍

@alanz
Copy link

alanz commented Apr 16, 2020

@jneira what is the next step?

@jneira
Copy link
Member

jneira commented Apr 16, 2020

@alanz haskell/haskell-language-server#64

@fendor
Copy link
Collaborator Author

fendor commented Apr 22, 2020

@mpickering Are you fine with merging this?

@mpickering
Copy link
Collaborator

Generally good but please don't modify the interface to addCmdOpts.

@fendor fendor force-pushed the dont-mangle-ghc-options branch from d5b7fc9 to d88195a Compare April 24, 2020 19:42
@fendor fendor requested a review from mpickering April 24, 2020 19:42
src/HIE/Bios/Types.hs Outdated Show resolved Hide resolved
@@ -167,6 +167,17 @@ addCmdOpts cmdOpts df1 = do
liftIO $ hPutStrLn stderr "cannot set package flags with :seti; use :set"
-}

makePathsAbsolute :: FilePath -> DynFlags -> DynFlags
Copy link
Collaborator Author

@fendor fendor Apr 24, 2020

Choose a reason for hiding this comment

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

Should this be part of the public API? Probably, but then I think a better name could be helpful?

Add makePathsAbsolute functions to normalise GHC flags.
Wire root directory for each cradle.
Add component root to output of `flags` CLI command.
Rename and expose makeDynFlagsAbsolute.
@fendor fendor force-pushed the dont-mangle-ghc-options branch from e199358 to 89d2881 Compare April 24, 2020 20:17
@mpickering
Copy link
Collaborator

Looks good with a CHANGELOG entry as this may break ghcide.

@fendor
Copy link
Collaborator Author

fendor commented Apr 25, 2020

ChangeLog is updated!

@mpickering
Copy link
Collaborator

Thanks. Great patch. Merge as you feel fit after squashing etc.

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.

4 participants