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

Update cabal-helper to 1.1.0.0 #1758

Closed jneira closed 4 years ago

jneira commented 4 years ago

It has several bug fixes

Avi-D-coder commented 4 years ago

I'm going to make an attempt at using implicit-hie as hie-bios implicit cradle today and tomorrow. What are we using cabal-helper for? Where does it's cradle discovery work, that implicit-hie does not. I'm guessing cabal-helper handles GHC and platform conditional modules, but I have not tested that. Does cabal-helper handle v2 style multi package repos (E.g. optics)?

fendor commented 4 years ago

Cabal-helper has roughly the same purpose as hie-bios but tries to achieve that at a lower level. While hie-bios uses a process abstraction and does not attempt to interact with any oddities of the tools, cabal-helper reads plan.json, setup-config (a binary file), etc... Afaict implicit-hie writes out hie.yaml so that hie-bios can load it. In that sense, implicit-hie is not related to cabal-helper at all. In my point of view, implicit-hie can be an extension, generate hie.yaml on demand, but I dont see it as an replacement for cabal-helper, at the moment. (But be welcome to suprise me and subvert my expectations!)

Cabal-Helper can deal with v2-style multi-projects. It also has full knowledge about the build-plan for each unit, etc...

Avi-D-coder commented 4 years ago

My plan is to attempt to have hie-bios generate a hie.yaml file whenever a valid cradle is not found. It would probably write to hie-gen.yaml, so as to avoid writing over hie.yaml and enable the user to see and override the generated config with hie.yaml.

There are a number of cases where cabal-helper fails to generate a good config, if 1.1 fixes some of those great, but my guess is that it will continue to be very brittle due to it's approach. Once implict-hie can generate hie.yaml files for more situations than cabal-helper gives a valid cradle I think we should drop cabal-helper. This would also simply cradle logic since we would only need hie-bios.

Avi-D-coder commented 4 years ago

So to illustrate my point, I built the https://github.com/haskell/haskell-language-server/pull/68 with 8.8.3 and cabal-helper 1.1 with stack.

Then I tried using hls on hls without a hie.yaml. It froze at 50%, I then ran gen-hie and despite the hie.yaml file being incomplete/wrong it worked when I opened the same file.

I'm going to repeat the test with cabal, to see if cabal-helper works better. Edit: same with cabal

I'm fixing the discrepancy between the generated hie file and the correct ones before I look at ingratiation with hie-bios.

DanielG commented 4 years ago

@Avi-D-coder I had a quick look at your implicit-hie code and I don't understand why you think a homegrown cabal file parser would be more roboust than going through the proper channels (aka. lib:Cabal or Setup.hs) to the point that you're proposing to replace cabal-helper? That just seems like an endless source of churn as you're trying to catch up to what cabal is doing.

Reminds me of the horrible hacks we were doing in ghc-mod eons ago actually, what is old is new again I guess. Admittedly the runtime compilation in cabal-helper is nasty but a necessary evil for the interface we wanted to have, which goes beyond what hie-bios' repl based approach can provide.

Support for show-build-info in cabal-helper is absolutely on the TODO list since that seems to be your main complaint in other threads. I just haven't found the time yet, patches welcome and all that :)

Avi-D-coder commented 4 years ago

@DanielG It is indeed very hacky, and I believe show-build-info will make it obsolete, but it works for now. I think parsing .cabal is less fragile because the config spec is pretty stable, and my experience with cabal-helper is that it breaks quite often.

Ultimately I'm okay with horrible hacks if they work. If the tiny parser gets to complicated then I can probably switch to a real one like cabal-fmt's.

I'm curious about what cabal-helper can do that a manually created hie.yaml can't?

jneira commented 4 years ago

I agree with @fendor, generate a working (even if you have to complete or correct it) hie.yaml automatically is great but i would like to be able to use the ide with no config file at all, especially in order to make things easier for beginners. Otoh, i see the points of @Avi-D-coder: few dependencies (including Cabal:lib!) and no need to wrap hie-bios basic behaviour. But i think they can be complementary at least for now (as suggested by @fendor):

@Avi-D-coder If you dont mind, i would update the cabal-helper version anyway, even if we decide finally to replace it. i think the api has not changed significantly and the update will no take much effort Moreover i would add the hie.yaml generation only in hls, as hie is in maintenance mode

Avi-D-coder commented 4 years ago

Yeah it definitely makes sense to upgrade cabal-helper.

jneira commented 4 years ago

Well cabal works fine simply changing bounds but stack doesnt:

Error: While constructing the build plan, the following exceptions were encountered:

In the dependencies for cabal-doctest-1.0.8:
    Cabal-3.2.0.0 from stack configuration does not match >=1.10 && <3.1  (latest matching version is 3.0.2.0)
needed due to haskell-ide-engine-1.4 -> cabal-doctest-1.0.8

In the dependencies for ghc-paths-0.1.0.12:
    Cabal-3.2.0.0 from stack configuration does not match >=1.6 && <3.1  (latest matching version is 3.0.2.0)
needed due to haskell-ide-engine-1.4 -> ghc-paths-0.1.0.12

In the dependencies for haddock-api-2.22.0:
    Cabal-3.2.0.0 from stack configuration does not match ^>=2.4.0  (latest matching version is 2.4.1.0)
needed due to haskell-ide-engine-1.4 -> haddock-api-2.22.0

In the dependencies for lens-4.17.1:
    Cabal-3.2.0.0 from stack configuration does not match >=1.10 && <2.5  (latest matching version is 2.4.1.0)
needed due to haskell-ide-engine-1.4 -> lens-4.17.1

Well, lets see what can we do with cabal freeze to fix stack resolvers 😄