haskell-ide-engine icon indicating copy to clipboard operation
haskell-ide-engine copied to clipboard

Extract Cabal-Helper specific code into its own library

Open fendor opened this issue 4 years ago • 18 comments

Talking with @jneira on irc, we realized this makes it easier to share code between the projects hie, hls and, potentially, ghcide! Code would be entirely taken from Cradle.hs in hie-plugin-api and the entire logic of discovering cradles with cabal-helper would be moved into its own library. Opinions?

fendor avatar Feb 03 '20 23:02 fendor

I've just discovered that Haskell.IDe.Engine.Cradle only imports Haskell.Ide.Engine.Logger from hie own code! Almost it is asking to live in its own home. 😄

jneira avatar Feb 04 '20 05:02 jneira

In case we decided to create the library, what about implicit-hie-bios (or hie-bios-implicit or something alike), to not bind it to cabal-helper. Maybe some day both build tools stack and cabal gives us enough info to use them directly? or we could add support for other tools implicit cradle?

jneira avatar Feb 04 '20 08:02 jneira

I really like the abstraction boundary that hie-bios figures out what the flags for the session should be. If it became a plugin API that I could then configure I'd be much less happy, as it's something I'm super grateful to not have to care about. Whether it incorporates or includes cabal-helper like functionality under the hood is a detail, so if this is transparent, I'm fine with it.

ndmitchell avatar Feb 04 '20 08:02 ndmitchell

I would not have considered it a plugin API, but I think I see what you mean. The intention would have been to extract code from Haskell.IDe.Engine.Cradle since only roughly six functions are used elsewhere in haskell-ide-engine, none of them specific to cabal-helper. I think one can argue that this micro-package would be nothing else but moved code for now. Do you dislike the idea to have an independent package for cabal-helper to hie-bios cradle conversion? Does that already count as a plugin API?

fendor avatar Feb 04 '20 09:02 fendor

Is the cabal-helper to hie-bios cradle something ghcide has to mix in? I don't like that idea. Or something that hie-bios always links in? If so, I don't see the advantage of separating the package.

ndmitchell avatar Feb 04 '20 10:02 ndmitchell

No, currently only hie uses it and hls copied the code to https://github.com/haskell/haskell-language-server/blob/master/src/Ide/Cradle.hs It is an alternative implementation to using the implicitCradle function. It is optional and would not affect ghcide at all, unless ghcide decides to use cabal-helper as well.

fendor avatar Feb 04 '20 10:02 fendor

I think hls and ghcide should converge on this as much as possible.

ndmitchell avatar Feb 04 '20 10:02 ndmitchell

Absolutely agreed. I think, somewhere it was mentioned that you agreed in bristol to use cabal-helper? Should there be an attempt to use cabal-helper in ghcide? EDIT: https://github.com/haskell/haskell-ide-engine/issues/1416#issuecomment-578409687

... that hie-bios has good implicit-project support, probably making use of cabal-helper.

fendor avatar Feb 04 '20 10:02 fendor

@ndmitchell just in case it would be solvable, what are your concerns about link statically cabal-helper in ghcide? is it that it depends on the Cabal library or another problematic lib? or the runtime compilation it triggers?

If hls uses cabal helper directly it will need to pass the cradle generated by c-h to ghcide (the component that actually is using it directly). Not sure if ithat is already possible in ghcide or it should be changed to accept "external" cradles.

Obviously if we add support in ghcide both would have the implicit cradle available. Imo it would be the desirable option over the former one.

Other options i can think of:

  • discover another, more lightweight way to generate a reliable implicit cradle
  • make hie-bios use cabal-helper at runtime (i.e. calling c-h as a executable), creating another cradle type (implicit?). Not sure if it is possible/viable

jneira avatar Feb 04 '20 11:02 jneira

I've heard cabal-helper takes a lot of work to support. At the moment, if there's an issue with the GHC flags, I go to hie-bios because the contract is hie-bios provides the flags to start a GHC session. I like the abstract interface "give me the flags" and the support model "ping Matthew et al". It's as much lines of communication and separation of responsibilities as it is code. If you guys want to shove cabal-helper behind the same interface and support model, provided there is no GPL impact, I'm happy (and moreover, consider it none of my business).

ndmitchell avatar Feb 04 '20 12:02 ndmitchell

That is exactly how we integrated cabal-helper. It is hidden within a Cradle and you just query it, like any other cradle. Granted, the mechanism is not as light-weight as the implicit cradle discovery in hie-bios, but we have no extra logic in hie to support cabal-helper. That's why usage of cabal-helper is completely isolated in this one module.

fendor avatar Feb 04 '20 12:02 fendor

FYI https://github.com/haskell/haskell-language-server/blob/master/src/Ide/Cradle.hs

alanz avatar Feb 04 '20 18:02 alanz

Which should end up wherever we agree

alanz avatar Feb 04 '20 18:02 alanz

Well i think the consesus had been using the cabal-helper cradle in haskell-language-server, cause cradle setup is in the main exe module owned by h-l-s: https://github.com/haskell/haskell-language-server/issues/38#issuecomment-582789371 @fendor could we close this or you have some concerns about?

EDIT: well, a concern is we have to port manually each change in the cabal-helper cradle between hie and hls

jneira avatar Feb 27 '20 06:02 jneira

I still think outsourcing it into different package does no harm and makes it easier to support both hls and hie. As I read it, there was no argument against it?

fendor avatar Feb 27 '20 08:02 fendor

@fendor no other than the usual overhead of maintain a separate lib: github repo, maybe upload to hackage/stackage, etc and the fact hie will be deprecated at some point. Otoh it could be used in another projects. If you are fine setting up the new package i am too :wink:

jneira avatar Feb 27 '20 08:02 jneira

Other option could be integrate the cabal-helper cradle in cabal-helper itself, if @DanielG agree it is a good idea add hie-bios as dependency.

jneira avatar Feb 27 '20 10:02 jneira

I think, for now I would prefer it as a separate library. I feel like it does not really belong into cabal-helper. However, lets wait for @DanielG opinion :)

fendor avatar Feb 27 '20 11:02 fendor