Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

Matching hie GHC version to the project GHC version #439

Closed
alanz opened this issue Jan 11, 2018 · 22 comments
Closed

Matching hie GHC version to the project GHC version #439

alanz opened this issue Jan 11, 2018 · 22 comments
Milestone

Comments

@alanz
Copy link
Collaborator

alanz commented Jan 11, 2018

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
Copy link
Collaborator Author

alanz commented Jan 20, 2018

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

@Tehnix
Copy link
Collaborator

Tehnix commented Jan 23, 2018

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
Copy link
Collaborator

Tehnix commented Jan 24, 2018

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
Copy link
Collaborator Author

alanz commented Jan 24, 2018

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
Copy link
Collaborator

Tehnix commented Jan 24, 2018

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
Copy link
Collaborator Author

alanz commented Jan 24, 2018 via email

@Tehnix
Copy link
Collaborator

Tehnix commented Jan 24, 2018

I'll start working on it 👍

@Tehnix
Copy link
Collaborator

Tehnix commented Jan 24, 2018

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

@alanz
Copy link
Collaborator Author

alanz commented Jan 30, 2018

This is now supported (configuration-wise) in emacs, via emacs-lsp/lsp-haskell#9

@alanz
Copy link
Collaborator Author

alanz commented Jan 31, 2018

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
Copy link
Contributor

ktonga commented Mar 7, 2018

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
Copy link
Collaborator Author

alanz commented Mar 8, 2018 via email

@ktonga
Copy link
Contributor

ktonga commented Mar 10, 2018

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
Copy link
Collaborator Author

alanz commented Mar 10, 2018

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
Copy link
Contributor

ktonga commented Mar 11, 2018

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
Copy link
Collaborator Author

alanz commented Mar 11, 2018

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
Copy link
Collaborator

mgsloan commented Mar 11, 2018 via email

@DanielG
Copy link
Collaborator

DanielG commented Mar 11, 2018

@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
Copy link
Collaborator

DanielG commented Mar 11, 2018

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
Copy link
Contributor

ktonga commented Mar 12, 2018

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
Copy link
Contributor

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
Copy link
Collaborator Author

alanz commented Aug 17, 2018

@nponeccop , the hie-wrapper executable basically implements the script from #439 (comment), and uses the cross-platform API to launch the identified exe with the same params as the original.

@alanz alanz closed this as completed Dec 24, 2018
@alanz alanz added this to the prehistory milestone Feb 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants