haskell / haskell-ide-engine

The engine for haskell ide-integration. Not an IDE
BSD 3-Clause "New" or "Revised" License
2.38k stars 210 forks source link

Matching hie GHC version to the project GHC version #439

Closed alanz closed 5 years ago

alanz commented 6 years ago

Come up with a process to launch the right hie, to match the compiler. And this has to work for stack and cabal, so stack install is not an option, necessarily.

I think there are two alternative approaches

  1. Make a concierge process that does the initial handshake until the initialize message with the project root is given, then detect the GHC version and spawn the appropriate hie version. The trick will be to replace the one process with the other, or to delegate stdio to the spawned hie. With the current state of initialization.

  2. Expose a helper that the client can invoke when determining the project root that also returns the hie instance to use.

The second option may be simpler, and can fall back to just using a setting in the client. These things tend to be pretty stable.

alanz commented 6 years ago

Also, report an error if we discover that a project is using a mismatched GHC, via an LSP error message.

Tehnix commented 6 years ago

As a temp fix, currently I'm using a wrapper script around HIE to make it start the right hie version,

#!/usr/bin/env bash
DEBUG=1
indent=""
function debug {
  if [[ $DEBUG == 1 ]]; then
    echo "$indent$@" >> /tmp/hie-wrapper.log
  fi
}

curDir=`pwd`
debug "Launching HIE for project located at $curDir"
indent="  "

GHCBIN='ghc'
# If a .stack-work exists, assume we are using stack.
if [ -d ".stack-work" ]; then
  debug "Using stack GHC version"
  GHCBIN='stack ghc --'
else
  debug "Using plain GHC version"
fi
versionNumber=`$GHCBIN --version`
debug $versionNumber

HIEBIN='hie'
BACKUP_HIEBIN='hie'
# Match the version number with a HIE version, and provide a fallback without
# the patch number.
if [[ $versionNumber = *"8.0.1"* ]]; then
  debug "Project is using GHC 8.0.1"
  HIEBIN='hie-8.0.1'
  BACKUP_HIEBIN='hie-8.0'
elif [[ $versionNumber = *"8.0.2"* ]]; then
  debug "Project is using GHC 8.0.2"
  HIEBIN='hie-8.0.2'
  BACKUP_HIEBIN='hie-8.0'
elif [[ $versionNumber = *"8.0"* ]]; then
  debug "Project is using GHC 8.0.*"
  HIEBIN='hie-8.0'
elif [[ $versionNumber = *"8.2.1"* ]]; then
  debug "Project is using GHC 8.2.1"
  HIEBIN='hie-8.2.1'
  BACKUP_HIEBIN='hie-8.2'
elif [[ $versionNumber = *"8.2.2"* ]]; then
  debug "Project is using GHC 8.2.2"
  HIEBIN='hie-8.2.2'
  BACKUP_HIEBIN='hie-8.2'
elif [[ $versionNumber = *"8.2"* ]]; then
  debug "Project is using GHC 8.2.*"
  HIEBIN='hie-8.2'
else
  debug "WARNING: GHC version does not match any of the checked ones."
fi

if [ -x "$(command -v $HIEBIN)" ]; then
  debug "$HIEBIN was found on path"
elif [ -x "$(command -v $BACKUP_HIEBIN)" ]; then
  debug "Backup $BACKUP_HIEBIN was found on path"
  HIEBIN=$BACKUP_HIEBIN
else
  debug "Falling back to plain hie"
  HIEBIN='hie'
fi

$HIEBIN $@

And to built the different binaries from source, I use (with some redundancies),

stack --stack-yaml=stack-8.0.2.yaml install \
  && cp ~/.local/bin/hie ~/.local/bin/hie-8.0.2 \
  && cp ~/.local/bin/hie ~/.local/bin/hie-8.0 \
  && stack --stack-yaml=stack-8.2.1.yaml install \
  && cp ~/.local/bin/hie ~/.local/bin/hie-8.2.1 \
  && cp ~/.local/bin/hie-8.2.1 ~/.local/bin/hie-8.2 \
  && stack --stack-yaml=stack.yaml install \
  && cp ~/.local/bin/hie ~/.local/bin/hie-8.2.2 \
  && cp ~/.local/bin/hie-8.2.2 ~/.local/bin/hie-8.2

I don't know if this could be useful for others? I currently have just put it on my path, and then altered the VSCode plugin to call hie-wrapper instead of plain hie.

^ the temporary VSCode edits specifically are, in ~/.vscode/extensions/alanz.vscode-hie-server-0.0.7/hie-vscode.sh (hie => hie-wrapper),

export HIE_SERVER_PATH=`which hie-wrapper`

...

# Run directly
hie-wrapper --lsp $@
...

Finally, one can see what's going on with tail -f /tmp/hie-wrapper.log, or change DEBUG to 0, if they don't want these messages.

Tehnix commented 6 years ago

While this would preferably be in Haskell, I think this provides a fine transition until we get that part integrated fully. Furthermore, if we get any form of binaries built for HIE, we can easily extend the script to also download these binaries, if they are not found for the specific GHC version. I'd be more than happy to do that part (if it's something we want to continue with) :)

alanz commented 6 years ago

I think this is a great step, and basically what I had in mind for the initial approach anyway.

And the only future change would be to rewrite the wrapper in haskell, which should help with it being more cross-platform friendly.

And possibly extending cabal-helper to spit out the GHC version for us.

Tehnix commented 6 years ago

I'm a little unsure on the next step. It can easily be added to the VSCode hie-vscode.sh and I can change the Atom integration to launch through a script instead of HIE directly, but stuff like Neovim would be left for manual setup of the script.

Should I just go ahead with the two first, and then perhaps make a README entry under Neovim on how to add the wrapper script? It would mean keeping a minimum of two places in sync (Atom, VSCode), but I guess we can live with this for now.

alanz commented 6 years ago

I think we should add the wrapper to the vscode repo, and the build script to hie (perhaps as a Makefile?), but make the use of the wrapper in vscode subject to a config setting, which does not use it by default.

We can flip that default later.

On 24 January 2018 at 11:50, Christian Kjær notifications@github.com wrote:

I'm a little unsure on the next step. It can easily be added to the VSCode hie-vscode.sh and I can change the Atom integration to launch through a script instead of HIE directly, but stuff like Neovim would be left for manual setup of the script.

Should I just go ahead with the two first, and then perhaps make a README entry under Neovim on how to add the wrapper script? It would mean keeping a minimum of two places in sync (Atom, VSCode), but I guess we can live with this for now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/haskell/haskell-ide-engine/issues/439#issuecomment-360077367, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZAB34MJNP0tEBxNZ2GRBr6mbVGXyBpks5tNvzdgaJpZM4RaZO_ .

Tehnix commented 6 years ago

I'll start working on it 👍

Tehnix commented 6 years ago

Should all be done, I'll make the README update after the remaining PRs are merged.

alanz commented 6 years ago

This is now supported (configuration-wise) in emacs, via https://github.com/emacs-lsp/lsp-haskell/pull/9

alanz commented 6 years ago

The best way forward may be some kind of concierge process that receives the LSP initialize message from the client, determines the project GHC version, and then calls something like exec to replace itself with the right one.

See http://hackage.haskell.org/package/unix-2.7.2.2/docs/System-Posix-Process.html#v:executeFile and https://stackoverflow.com/questions/4204915/please-explain-the-exec-function-and-its-family

ktonga commented 6 years ago

Is anyone working on this? I could give it a shot (with some help probably)

PS: Is there anything I can read to understand why this is needed? Is it a ghc-mod limitation?

alanz commented 6 years ago

Hie is like ghci in that it actually loads the project, to get info. For that to work, the GHC version built into hie must be able to load the compiled products.

So it must match.

I would imagine we could get it from cabal-helper or ghc-mod.

On 08 Mar 2018 01:54, "Gastón Tonietti" notifications@github.com wrote:

Is anyone working on this? I could give it a shot (with some help probably)

PS: Is there anything I can read to understand why this is needed? Is it a ghc-mod limitation?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/haskell/haskell-ide-engine/issues/439#issuecomment-371328041, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZAByWhOEHD9bNUVAEHeSIla2XA0I9Bks5tcHNNgaJpZM4RaZO_ .

ktonga commented 6 years ago

Just to make it visible here, as per this comment it seems the new approach is a bit different from the one described using a concierge process.

Are we sure about this new handshaky launching? I don't think any LSP client implementation has such facility to invoke multiple times a command for launching the server, so in most cases user will end up scripting it.

But i guess it's easy enough using a one-liner like: GHCVER=$(hie --ghc-ver); hie-$GHCVER

alanz commented 6 years ago

I think we are feeling our way on this, and perhaps some approaches will work better with some clients than others.

I think the concierge option will be the most bullet-proof eventually, but it does need a fair bit of config.

Also, eventually the right version of hie can be determined by the project dependencies if using nix or stack.

At the moment, "doing the simplest thing" has resulted in it being set in the client.

Next step is to make it easy for the client to check if it has the right version.

And after that full concierge.

A bit of a brain dump, sorry.

For me first prize is still the concierge process.

ktonga commented 6 years ago

While implementing this on #493 I found that using cabal-helper for finding the GHC version has a few pitfalls I would like to enumerate and discuss if you will.

  1. cabal-helper requires ghc to be on the $PATH, but that not always is the case, for instance if you use stack o cabal configure with -w option.

  2. To use compilerVersion you're asked for the project's root dir (the one containing project.cabal), that one is easy. But you also have to provide project's build dir (the one containing setup-config) and that one is not so easy.

For projects using cabal (old) build it's trivial, it's just ./dist, but in the case of projects using stack you might have several, for instance:

./.stack-work/dist/x86_64-linux-nopie/Cabal-2.0.1.0/setup-config
./.stack-work/dist/x86_64-linux-nopie/Cabal-1.24.2.0/setup-config

We could just pick one arbitrarily but dunno how correct would that be.

Also, I think we should consider supporting cabal new-build for any new functionality and in that case it gets even worse, we have to know the ghc version in use to find the ghc version in use :)

./dist-newstyle/build/x86_64-linux/ghc-8.2.2/ghc-mod-5.9.0.0/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/HaRe-0.8.4.1/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/ghc-mod-core-5.9.0.0/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/haskell-ide-engine-0.1.0.0/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/haskell-ide-engine-0.1.0.0/c/hie/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/haskell-ide-engine-0.1.0.0/c/haskell-ide-test/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/haskell-ide-engine-0.1.0.0/c/haskell-ide-func-test/setup-config
./dist-newstyle/build/x86_64-linux/ghc-8.2.2/hie-plugin-api-0.1.0.0/setup-config

Over the time a project could accumulate multiple build dirs for different ghc version and we would not know which one to use, because we are basically trying to find that out.

An alternative to avoid all the directory choosing drama would be to use the same approach used in hie-wrapper script, either stack ghc -- --version for stack projects or just ghc --version for cabal ones.

What you reckon?

alanz commented 6 years ago

I think we should plumb this through ghc-mod, which takes care of looking after those details. cabal-helper, is basically a part of ghc-mod, and is not really intended to be used standalone.

So it would involve adding a compilerVersion command to ghc-mod, which simply calls cabal-helper with command "compiler-version".

cc @DanielG

mgsloan commented 6 years ago

I have not dug into the details related to his here. However, it may be helpful that "stack install haskell-ide-engine --copy-compiler-tool" will place it in a special bin directory which will be on the PATH when stack is used with that ghc version.

On Mar 11, 2018 12:26 AM, "Alan Zimmerman" notifications@github.com wrote:

I think we should plumb this through ghc-mod, which takes care of looking after those details. cabal-helper, is basically a part of ghc-mod, and is not really intended to be used standalone.

So it would involve adding a compilerVersion command to ghc-mod, which simply calls cabal-helper with command "compiler-version".

cc @DanielG https://github.com/danielg

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haskell/haskell-ide-engine/issues/439#issuecomment-372097983, or mute the thread https://github.com/notifications/unsubscribe-auth/AABYKtQ71rPPPoD9fPSZ06jJdKzdqIWUks5tdN-0gaJpZM4RaZO_ .

DanielG commented 6 years ago

@ktonga using cabal-helper directly is just not going to do what you want as you've found. You have to do this at a level where the build system in use (i.e. cabal-install, stack or plain GHC) is known. cabal-helper doesn't know anything about build systems, all it does is give you access to some stuff both cabal-install and stack dump to disk.

I think the right place to do this is definetly ghc-mod. All you really need to do is compare cProjectVersion from GHC's Config module with the output of ghc --numeric-version after the project type (stack vs cabal etc.) has been determined.

The way I'd do it is extending ghc-mod's Cradle type with something like cradleGhcVersion which would be initialized as part of findCradle' and then check it against cProjectVersion right in findCradle and error out if it doesn't check out.

The trickiest part is probably deciding what kind of error to throw the way we do it is kind of weird and inconsistent but I think in this case adding another case to GhcModError would make the most sense since downstream stuff like h-i-e will want to know if this specific problem happens and possibly handle it.

DanielG commented 6 years ago

Come to think of it I didn't consider cabal old-build properly. In that case using cabal-helper to determine the GHC version instead of calling ghc --numeric-version is pretty much a must if we want this to work reliably in situations where the user calls cabal configure -w.

I think in that case we can just defer erroring out straight away in findCradle and do it whenever runCHQuery is called, or something like that. If you can't figure it out I can have a closer look.

ktonga commented 6 years ago

Great, thank you a lot @DanielG for taking the time and explaining all that to me. I'll try to wrap my head around everything you've just said and open a PR on ghc-mod to add this new functionality.

nponeccop commented 6 years ago

then calls something like exec to replace itself with the right one.

it's not possible on Windows

Is this issue still open? What remains to do?

alanz commented 6 years ago

@nponeccop , the hie-wrapper executable basically implements the script from https://github.com/haskell/haskell-ide-engine/issues/439#issuecomment-359801662, and uses the cross-platform API to launch the identified exe with the same params as the original.