Move session loading logic into ghcide library

[lukel97]

This way haskell-language-server can reuse it without having to maintain two versions of this session setup code, which was getting quite tricky to maintain. Needs #693 and #696 merged first

[jneira]

Some caveats of using a private lib:

• It causes not deterministic build plan errors that forces you to delete the store partially or totally
• Stack cradle cant handle them in a reliable way
• Being a private lib, hls will not be able to use it (or am i missing something?)

Still this could be a preliminary step before convert it in a package, but the transition should be done asap, imo.

[lukel97]

@jneira just before I read your comment I found out that I forgot to add the visibility: public field to the library! I've integrated it with haskell-language-server now but you're right, it was a bit of a pain and there are definitely some drawbacks. For one, I'm not sure if this works with ghc <8.8. Is there any reason why we can't just move this into ghcide itself?

[lukel97]

@jneira ok right off the bat haskell-language-server fails to build with stack build --stack-yaml=stack-8.6.5.yaml, stack 2.3.1 because it can't parse this bit of the cabal file:

  build-depends:
    , ghcide:ghcide-session-loader

[jneira]

Mmm i thought public libraries was not fully supported in cabal-install but the final pr has been recently merged, so it would need cabal-3.4 (and hoping not to hit more bugs). We will have to wait a while to have stack support.

[jneira]

@bubba what do you think about creating directly a new subpackage (ghcide-session-loader? ghcide-bios?) with the same contents as the public library?
Maybe it will no need separate the exe from the main lib in ghcide, as the subpackage would depend on the ghcide library and the ghicde executable will depend on its library and the subpackage.
It supposes more maintaining work but imo it will be more reliable.

[lukel97]

@jneira I think a sub-package is probably overkill for just one module. With sub packages you need to be careful whenever you're making releases on hackage etc. so that you don't upload them in the wrong order. I always mess that up with haskell-lsp and haskell-lsp-types. I've moved it into just the ghcide library for now, with the one caveat being that loadSession just errors if the -fghc-lib flag is set

[jneira]

I think ghcide maintainers preferred to separate hie-bios from the ghcide main lib, to not leak it to daml.
Although maybe they could confirm if it is so.

[lukel97]

@jneira ah if thats the case then, then we might not have much choice

[jneira]

@bubba Other alternative would be move the ghc session logic to hls and remove the exe part from ghcide entirely, letting it be a pure lib 😄

[pepeiborra]

@bubba @jneira Can you enumerate all the options and their tradeoffs? Things to consider:

• testability
• minimal dependencies in the ghcide Library stanza
• compatibility with tools and IDEs

Hopefully making it easy for the code owners (@digitalasset @cocreature) to pick one.

[cocreature]

Other alternative would be move the ghc session logic to hls and remove the exe part from ghcide entirely, letting it be a pure lib smile

That breaks all tests we currently have.

I think ghcide maintainers preferred to separate hie-bios from the ghcide main lib, to not leak it to daml.
Although maybe they could confirm if it is so.

Yes, we need to be able to build ghcide (the library) without a dependency on any Haskell build tool stuff which includes hie-bios.

What could work is to just hide the session stuff and the dependency on hie-bios behind a cabal flag.

[lukel97]

Approach	Pros	Cons
Move loadSession into lib:ghcide	Simple	ghcide needs to be independent of haskell stuff so no hie-bios: No dice
Move loadSession into sublibrary	All in the one package	Really dubious stack support
Move loadSession into new package	Works with all stack/cabal versions	More release churn. Now have a circular dependency on exe:ghcide -> lib:ghcide-session-loader -> lib:ghcide, so exe:ghcide might also need split out into another package
Hide behind a cabal flag	All in the one package	No good way of specifying cabal flags for dependencies directly in .cabal files. Will need to use cabal.project in haskell-language-server

[cocreature]

cabal flags have a default. Apart from DAML nobody cares about being able to disable this so the sane option is to include it by default. We could even just reuse the ghc-lib flag here. Then apart from DAML nobody has to specify the flag and it’s not really an issue that you cannot specify flags for your build-depends in your cabal file.

[lukel97]

@cocreature I was thinking that as well, if the default was to include hie-bios/the session loader then haskell-language-server might still be able to get uploaded to hackage eventually. I'm happy to try out this option

[jneira]

Now have a circular dependency on exe:ghcide -> lib:ghcide-session-loader -> lib:ghcide, so exe:ghcide might also need split out into another package

I think there would not be such circularity cause one package depends on the lib of the another one, no on its executables (they are private for dependency management purpose).
The advantage would be it works out of the box with all stack/cabal versions and the separation is cleaner (maybe too much), only one small independent component will be more reliable, in general. Also it would make impossible to introduce dependencies from other ghcide lib modules to the session loader.

That said, agree a cabal flag with the default value would be the best option. Not sure about how can it affect the ghcide executable, it will be no buildable with the session loader disabled (so maybe simply mark it as buildable: false for the chosen flag would suffice).

[lukel97]

I think there would not be such circularity cause one package depends on the lib of the another one, no on its executables (they are private for dependency management purpose).

I didn't realise stack/cabal could handle that!

@cocreature @jneira for now I've just reused the ghc-lib flag which I was already using as a check. The ghcide executable has buildable: False when this is turned on anyway so there shouldn't be any difference

[lukel97]

Argh this module fails to build only on windows when building with stack and ghc-8.10. Window + cabal + ghc-8.10 seems fine. Not exactly sure what's causing it

[lukel97]

Looks like the template-haskell bit used to generate the GhcVersionChecker is the culprit. Maybe that stuff does need moved back out into a separate module

[cocreature]

Great work @bubba, thank you very much!