Integrate Cabal-Helper

[fendor]

Since haskell/haskell-language-server#138 would render the Code in https://github.com/haskell/haskell-language-server/blob/master/src/Ide/Cradle.hs vastly obsolete, I would like to propose that this repo integrates Cabal-Helper, especially the CabalHelper Cradle.

A possible usage of Cabal-Helper in gen-hie would be to have two modes of generating a hie.yaml file:

• heuristic (parsing .cabal file, etc...)
• exact (using cabal-helper)

If you do not want to impose the dependencies on all consumers of this library, I think it would make sense to enable/disable the usage of cabal-helper via a cabal flag.

What this would bring:

• Move vast parts of Cradle.hs to this repo, making it a possible implicit cradle discovery mechanism.
• This repo becoming the only source of implicit cradle discovery.
• Add a flag to gen-hie to use an exact heuristic for creating hie.yaml files.

I think this may bring us the best of both worlds: a more error resilient implicit cradle discovery in HLS, and the more exact generation of hie.yaml with cabal-helper.

The PR haskell/haskell-language-server#138 is unaffected from this issue in general.
cc @jneira

[Avi-D-coder]

I'm certainly open to this, but I don't have time to implement this (starting a fellowship on Monday).

My one minor concern is that the current cabal-helper cradle can load private libs under stack. If we converged on using cabal-helper to generate hie.yaml configs we would lose that advantage.

[jneira]

Agree with @Avi-D-coder, if there are cases where even the hie-bios implicit cradle fails we could use c-h to get a correct cradle.
A problem with the fallback is the cradle could return a succesfull cradle but with emty or wrong ghc options and you cant say it for sure until you start the ghc session. That fallbackk should be implemened upstream (in hls).

[fendor]

I think it is more important to be comprehesnible than fixing every use-case. In this sense, nesting (or sequencing) implicit cradle discovery brings us a can of worms, e.g. when one cradle mechanism works while the other doesnt.
So, how do we tell which is used by looking at logs? How do we present the error? How might we tell users how to fix it?
Even worse, if it doesnt work in both modes, do we show the error of both discovery mechanisms (bad idea, might show contradicting and unhelpful error messages)? Only of one of the modes (will make it harder to debug these modes)? Either way, the solution is imperfect anyways.

In the end, it all leads to the one and only solution: it needs to be fixed in the tools themselves and they need to be patched to support our workflow (cabal show-build-info, obelisk, stack needs to add a similar command). Everything else is just adding a bandaid.
That is why I think it would be better to have only one cradle discovery mechanism in hls (at least).
Due to the cradle abstraction, it doesnt even matter that much since changing the behaviour is a single line change. Sequencing is probably like three line changes.

As a last work-around, it is possible to allow the user to specify explicitly in a hie.yaml file to use the cabal-helper cradle. E.g.

cradle:
  other:
    cabal-helper:
      cabal:

Granted, it would break the debug binary hie-bios, because something like hie-bios debug ... would not work anymore, but we could have a similar binary in gen-hie that is aware of the cabal-helper cradle.

[Avi-D-coder]

I agree that one mechanism is better than two, even if we inherit the limitations of cabal and stack's repls.

Hie-bios's no prefixes matched error is very easy to understand, unlike cabal-helper's silent errors, linker errors, CPP errors, and whatever else I have not yet seen.

[Avi-D-coder]

I think the use cabal-helper option should be a LSP config not in hie.yaml.

[Avi-D-coder]

I still suspect integrating cabal-helper will require more code then just implementing the last two missing features.

Common stanzas just require a extra field and a pass over components.

Conditionals have to be parsed and the expressions we care about (GHC, OS) have to be evaluated.

[fendor]

The cabal-helper cradle already exists though and I think it lends itself perfectly for the gen-hie binary.
I will prepare a PR, maybe this weekend or early next week, then we can talk about how to continue.

Conditionals have to be parsed and the expressions we care about (GHC, OS) have to be evaluated.

Requires a lot of parsing and monkey patching, imo. If gen-hie depends on Cabal, you could use the Cabal parser anyways.

[jneira]

I think it is more important to be comprehesnible than fixing every use-case. In this sense, nesting (or sequencing) implicit cradle discovery brings us a can of worms, e.g. when one cradle mechanism works while the other doesnt.

Fair enough, given there will be more generic solutions from both cabal and stack.

[Avi-D-coder]

The cabal-helper cradle stuff looks good, thanks for porting it.

I'm not interested in using the Cabal as a library to parse, I'd like to keep the dependency behind a flag, since it's so big. Extracting info from cabal files is pretty straight forward since we don't care about most things. If we need a more complete parser cabal-fmt's looks like the way to go.

[fendor]

If we use cabal-helper for generating hie.yaml, I get
this for gen-hie:

cradle:
  cabal:
    - path: "src/"
      component: "lib:implicit-hie"

    - path: "app/"
      component: "implicit-hie:exe:gen-hie"

    - path: "test/"
      component: "implicit-hie:test:implicit-hie-test"

    - path: "test/unit/"
      component: "implicit-hie:test:unit-tests"

    - path: "test/utils/"
      component: "implicit-hie:test:unit-tests"

this for haskell-ide-engine:

cradle:
  cabal:
    - path: "src/"
      component: "lib:haskell-ide-engine"

    - path: "test/dispatcher/"
      component: "haskell-ide-engine:test:dispatcher-test"

    - path: "test/functional/"
      component: "haskell-ide-engine:test:func-test"

    - path: "app/"
      component: "haskell-ide-engine:exe:hie"

    - path: "test/utils/"
      component: "haskell-ide-engine:lib:hie-test-utils"

    - path: "app/"         # <----------- this needs to be more accurate
      component: "haskell-ide-engine:exe:hie-wrapper"

    - path: "test/plugin-dispatcher/"
      component: "haskell-ide-engine:test:plugin-dispatcher-test"

    - path: "test/unit/"
      component: "haskell-ide-engine:test:unit-test"

    - path: "test/wrapper/"
      component: "haskell-ide-engine:test:wrapper-test"

    - path: "hie-plugin-api/"
      component: "lib:hie-plugin-api"

this for cabal-extras:

cradle:
  cabal:
    - path: "cabal-bundler/cli/"
      component: "cabal-bundler:exe:cabal-bundler"

    - path: "cabal-bundler/src/"
      component: "cabal-bundler:lib:cabal-bundler-internal"

    - path: "cabal-deps/cli/"
      component: "cabal-deps:exe:cabal-deps"

    - path: "cabal-deps/src/"
      component: "cabal-deps:lib:cabal-deps-internal"

    - path: "cabal-diff/cli/"
      component: "cabal-diff:exe:cabal-diff"

    - path: "cabal-diff/test/"
      component: "cabal-diff:test:cabal-diff-golden"

    - path: "cabal-diff/src/"
      component: "cabal-diff:lib:cabal-diff-internal"

    - path: "cabal-env/cli/"
      component: "cabal-env:exe:cabal-env"

    - path: "cabal-env/src/"
      component: "cabal-env:lib:cabal-env-internal"

    - path: "cabal-store-check/cli/"
      component: "cabal-store-check:exe:cabal-store-check"

    - path: "cabal-store-check/src/"
      component: "cabal-store-check:lib:cabal-store-check-internal"

    - path: "cabal-store-gc/cli/"
      component: "cabal-store-gc:exe:cabal-store-gc"

    - path: "cabal-store-gc/src/"
      component: "cabal-store-gc:lib:cabal-store-gc-internal"

    - path: "peura/src/"
      component: "lib:peura"

this for cabal

cradle:
  cabal:
    - path: "Cabal/"
      component: "lib:Cabal"

    - path: "Cabal/tests/"
      component: "Cabal:test:check-tests"

    - path: "Cabal/tests/custom-setup/"
      component: "Cabal:test:custom-setup-tests"

    - path: "Cabal/tests/"
      component: "Cabal:test:hackage-tests"

    - path: "Cabal/tests/"
      component: "Cabal:test:parser-tests"

    - path: "Cabal/tests/"
      component: "Cabal:test:rpmvercmp"

    - path: "Cabal/tests/"
      component: "Cabal:test:unit-tests"

    - path: "Cabal/Cabal-quickcheck/src/"
      component: "Cabal:test:unit-tests"

    - path: "Cabal/Cabal-quickcheck/src/"
      component: "lib:Cabal-quickcheck"

    - path: "cabal-benchmarks/bench/"
      component: "cabal-benchmarks:test:cabal-benchmarks"

    - path: "cabal-install/main/"
      component: "cabal-install:exe:cabal"

    - path: "cabal-install/solver-dsl/"
      component: "cabal-install:lib:cabal-install-solver-dsl"

    - path: "cabal-install/"
      component: "cabal-install:lib:cabal-lib-client"

    - path: "cabal-install/tests/"
      component: "cabal-install:test:integration-tests2"

    - path: "cabal-install/tests/"
      component: "cabal-install:test:memory-usage-tests"

    - path: "cabal-install/tests/"
      component: "cabal-install:test:solver-quickcheck"

    - path: "cabal-install/tests/"
      component: "cabal-install:test:unit-tests"

    - path: "cabal-testsuite/src/"
      component: "lib:cabal-testsuite"

    - path: "cabal-testsuite/main/"
      component: "cabal-testsuite:exe:cabal-tests"

    - path: "cabal-testsuite/"
      component: "cabal-testsuite:exe:setup"

    - path: "solver-benchmarks/"
      component: "lib:solver-benchmarks"

    - path: "solver-benchmarks/main/"
      component: "solver-benchmarks:exe:hackage-benchmark"

    - path: "solver-benchmarks/tests/"
      component: "solver-benchmarks:test:unit-tests"

Looks pretty alright to use already. Load time is horrible, since we are building every component in the package, but I imagine that it will be the "more correct" approach.

[Avi-D-coder]

This looks pretty good. We need to handle multiple executables and other-modules and nested packages.

The duplicate other-modules (./exe/Arguments.hs) files are not a issue since any competent that uses it must have the it's dependency (The first listing one is allays used by hls).

hls

cradle:
  cabal:
    - path: "./src"
      component: "lib:haskell-language-server"

    - path: "./exe/Main.hs"
      component: "haskell-language-server:exe:haskell-language-server"

    - path: "./exe/Arguments.hs"
      component: "haskell-language-server:exe:haskell-language-server"

    - path: "./exe/Wrapper.hs"
      component: "haskell-language-server:exe:haskell-language-server-wrapper"

    - path: "./exe/Arguments.hs"
      component: "haskell-language-server:exe:haskell-language-server-wrapper"

    - path: "./test/functional"
      component: "haskell-language-server:test:func-test"

    - path: "ghcide/src"
      component: "lib:ghcide"

    - path: "ghcide/src-ghc86"
      component: "lib:ghcide"

    - path: "ghcide/src-ghc88"
      component: "lib:ghcide"

    - path: "ghcide/src-ghc810"
      component: "lib:ghcide"

    - path: "ghcide/test/preprocessor/Main.hs"
      component: "ghcide:exe:ghcide-test-preprocessor"

    - path: "ghcide/exe/Main.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/exe/Utils.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/exe/Arguments.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/exe/Paths_ghcide.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/test/cabal"
      component: "ghcide:test:ghcide-tests"

    - path: "ghcide/test/exe"
      component: "ghcide:test:ghcide-tests"

    - path: "ghcide/test/src"
      component: "ghcide:test:ghcide-tests"

[jneira]

I wonder if we could get that level of precision but faster only reading the (cabal.project and .cabal files) configuration using cabal-parsers instead cabal-helper.
As Cabal usually is pretty backwards compatible we could statically link the last Cabal version.
And for stack you have to parse yaml...

[fendor]

@jneira I like that idea!

[Avi-D-coder]

Does cabal-parsers render common stanzas and conditionals or is that in higher in cabals stack?

[jneira]

Afaik all those features are included in the parser.