Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

Overhaul ghc api initialization error handling #568

Closed
wants to merge 5 commits into from

Conversation

pepeiborra
Copy link
Collaborator

There are really a lot of cases to handle as seen below. Thanks
@jneira for help discovering them all.

Non Nix

The table below shows a couple of combinations of cradles and ghcide versions in a
non-Nix environment. All the version mismatches are now handled as follows:

  • "Cannot satisfy package" - arises due to -package-id flags referencing
    dependencies bundled in another ghc version
  • "version-check" - detected by ghc-check using either the version of the "ghc"
    package or the abi of the base package
  • "linker error" - arises due to missing symbols in the "ghc-prim" package when
    loading an incompatible version
cradle/ghcide 8.6 8.8 8.10
Cabal 8.6 success cannot satisfy package cannot satisfy package
Cabal 8.8 cannot satisfy package success cannot satisfy package
Cabal 8.10 cannot satisfy package cannot satisfy package success
Stack 8.6 success linker error version-check
Stack 8.8 version-check success version-check
Stack 8.10 version-check version-check success

Nix

Because Nix redefines the libdir to point at the run-time ghc installation, all
the invalid combinations in the table above are detected by the "installation
check" performed by ghc-check.

@jneira
Copy link
Member

jneira commented May 17, 2020

Mmm, maybe it is obvious but i suppose you dont want to rely in build tools even at runtime calling stack exec ghc -- --numeric-version and/or cabal execute ghc -- --numeric-version

@fendor
Copy link
Collaborator

fendor commented May 17, 2020

BTW, from the work on HIE, we know that cabal exec -- ghc --numeric-version does not work until the project has at least been configured. stack exec ... works as expected, though.

@pepeiborra
Copy link
Collaborator Author

Asking build tools sounds like a fine idea to me in the context of hie-bios, which is the abstraction that ghcide relies on. An API to interrogate the ghc version for the project, which ghcide and other tools could rely on for early checks, would certainly be a neat way to catch most of incompatibilities. Even if it only works for certain cradles.

But loading packages and package databases is at the heart of the problem, so I suspect we still want to beef up the error handling in ghcide itself.

Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks! One minor comment.

exe/Rules.hs Outdated Show resolved Hide resolved
pepeiborra and others added 3 commits May 18, 2020 10:22
There are really a lot of cases to handle as seen below. Thanks
@jneira for help discovering them all.

Non Nix
=========

The table below shows a couple of combinations of cradles and ghcide versions in a
non-Nix environment. All the version mismatches are now handled as follows:

- "Cannot satisfy package" - arises due to `-package-id` flags referencing
  dependencies bundled in another ghc version
- "version-check" - detected by ghc-check using either the version of the "ghc"
  package or the abi of the base package
- "linker error" - arises due to missing symbols in the "ghc-prim" package when
  loading an incompatible version

cradle/ghcide | 8.6 | 8.8 | 8.10
--------------|-----|----|---
Cabal 8.6   | success | cannot satisfy package | cannot satisfy package
Cabal 8.8   | cannot satisfy package | success | cannot satisfy package
Cabal 8.10  | cannot satisfy package | cannot satisfy package | success
Stack 8.6   | success | linker error | version-check
Stack 8.8   | version-check | success | version-check
Stack 8.10  | version-check | version-check | success

Nix
=========

Because Nix redefines the libdir to point at the run-time ghc installation, all
the invalid combinations in the table above are detected by the "installation
check" performed by ghc-check.
Co-authored-by: Moritz Kiefer <moritz.kiefer@purelyfunctional.org>
Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failure looks legitimate:

Could not find (DsError,(0,15),"parse error") in List [Diagnostic {_range = Range {_start = Position {_line = 0, _character = 0}, _end = Position {_line = 100000, _character = 0}}, _severity = Just DsError, _code = Nothing, _source = Just "compiler", _message = "ghcide compiled by GHC  8.6.5 failed to load packages: thread killed . Please ensure that ghci is compiled with the same GHC installation as the project.\nCallStack (from HasCallStack):\n  error, called at src/Development/IDE/GHC/Util.hs:187:17 in ghcide-0.1.0-6AQWwJWe8FISph2Jfl2Ds:Development.IDE.GHC.Util", _tags = Nothing, _relatedInformation = Nothing}]

@pepeiborra
Copy link
Collaborator Author

The test failure looks legitimate:

Could not find (DsError,(0,15),"parse error") in List [Diagnostic {_range = Range {_start = Position {_line = 0, _character = 0}, _end = Position {_line = 100000, _character = 0}}, _severity = Just DsError, _code = Nothing, _source = Just "compiler", _message = "ghcide compiled by GHC  8.6.5 failed to load packages: thread killed . Please ensure that ghci is compiled with the same GHC installation as the project.\nCallStack (from HasCallStack):\n  error, called at src/Development/IDE/GHC/Util.hs:187:17 in ghcide-0.1.0-6AQWwJWe8FISph2Jfl2Ds:Development.IDE.GHC.Util", _tags = Nothing, _relatedInformation = Nothing}]

Yes, this is due to gtry catching async exceptions. I have a fix almost ready but I think we want to merge the multi component PR first anyway

Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

@cocreature
Copy link
Collaborator

Needs rebasing on master.

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Jun 3, 2020

This needs a rewrite - I'll open a new PR when ready

@pepeiborra pepeiborra closed this Jun 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants