Find the libdir directory of ghc at run-time #1496

fendor · 2019-12-23T17:15:04Z

Finds the libdir directory of the appropriate ghc version at run-time.
Introduces the isCabalCradle predicate to complement the isStackCradle

Documentation is missing.
getProjectGhcVersion can be expressed with execProjectGhc

While it works in manual test-cases I have the following considerations:

Cradle initialisation happens in too many places. At least the one in "NotInitialized" can be skipped, imo.
We might select the wrong ghc version in certain circumstances.
I dont know what happens if the system lib dir is Nothing

alanz · 2019-12-23T17:51:59Z

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs

+  mGhcPath <- getProjectGhcPath crdl
+  case mGhcPath of
+    Nothing -> return Nothing
+    Just ghcPath -> catch (Just <$> tryCommand (ghcPath ++ " --print-libdir")) $ \(_ :: IOException) -> return Nothing


Catching and discarding the exception is probably a bad thing.

At the very least we should log something on failure.

Is the the exception that would be thrown if GHC is not installed? Do we report it gracefully elsewhere if that is the case?

No, not yet. Not sure how to properly report it, since it is executed before the server is actually started, e.g. When we start the GHC thread

Perhaps we should re-look at this startup sequencing, and launch the various processes once the initialize message comes in, while processing it.

That way we can determine the cradle according to the passed-in project root, and we have a way to report errors. The spec says we are not to process anything before initialization is complete anyway, so it is an appropriate place to do it.

cc @bubba @mpickering

Yes, I think the LSP server should be bootstrapped before the dispatcher. And while we're at it, we should provide an easier way to log/show these messages via LSP. Presumably some sort of transformer stack hierarchy:

IO a < LspM a < IdeM a < IdeGhcM a

Would that mean to execute onStartup https://github.com/haskell/haskell-ide-engine/blob/master/src/Haskell/Ide/Engine/Server.hs#L125 in NotInitialized?

And perhaps we should rename onStartup to onInitialize, and make it clear where in the server life cycle it operates.

Oh, so that is actuallly fine as-is? Just need to refactor when the cradle is loaded?

And this change should also resolve haskell/vscode-haskell#164

Not entirely. For hie-bios multi-cradles, also #1490 needs to be resolved.

lukel97 · 2019-12-24T02:26:54Z

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs

+  mGhcPath <- getProjectGhcPath crdl
+  case mGhcPath of
+    Nothing -> return Nothing
+    Just ghcPath -> catch (Just <$> tryCommand (ghcPath ++ " --print-libdir")) $ \(_ :: IOException) -> return Nothing


Yes, I think the LSP server should be bootstrapped before the dispatcher. And while we're at it, we should provide an easier way to log/show these messages via LSP. Presumably some sort of transformer stack hierarchy:

IO a < LspM a < IdeM a < IdeGhcM a

.circleci/config.yml

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs

src/Haskell/Ide/Engine/Server.hs

test/functional/HieBiosSpec.hs

Fallback to normal ghc if stack fails

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs

src/Haskell/Ide/Engine/Scheduler.hs

src/Haskell/Ide/Engine/Version.hs

fendor · 2019-12-29T10:21:46Z

@bubba thanks for fixing the tests!
PR is ready for merge, waiting for some reviews now.

alanz · 2019-12-29T12:06:33Z

src/Haskell/Ide/Engine/Server.hs

+        flip labelThread "scheduler" =<<
+            (forkIO (
+              Scheduler.runScheduler scheduler errorHandler callbackHandler (Just lf) mcradle
+              `E.catch` \(e :: E.SomeException) ->
+              (errorm $ "Scheduler thread exited unexpectedly: " ++ show e)
+            ))
+        flip labelThread "reactor" =<<
+            (forkIO (
+              reactorFunc
+              `E.catch` \(e :: E.SomeException) ->
+              (errorm $ "Reactor thread exited unexpectedly: " ++ show e)
+            ))
+        flip labelThread "diagnostics" =<<
+            (forkIO (
+              diagnosticsQueue tr
+              `E.catch` \(e :: E.SomeException) ->
+              (errorm $ "Diagnostic thread exited unexpectedly: " ++ show e)
+            ))
+


At some future date we should go back to launching these threads with race rather than forkIO, so if one does die, we know immediately, and can see something in the log. With this setup it can still limp along.

This missed CI as it came in from haskell#1505

Adapt GhcModPluginSpec after merge of #1496

fendor requested review from alanz, mpickering and lukel97 December 23, 2019 17:15

alanz reviewed Dec 23, 2019

View reviewed changes

fendor force-pushed the dont-depend-on-ghc-at-run-time branch from 862da6b to 536edec Compare December 23, 2019 21:10

lukel97 added this to In progress in Dec 2019 Release via automation Dec 24, 2019

lukel97 approved these changes Dec 24, 2019

View reviewed changes

fendor mentioned this pull request Dec 25, 2019

HIE doesn't work after build. "haskell-lsp: Failed to parse message header" error #1499

Open

Ailrun mentioned this pull request Dec 26, 2019

HIE gives "could not execute: <some preprocessor exe>" #1500

Open

fendor mentioned this pull request Dec 26, 2019

Issue with hover and linting errors stopped working #1501

Closed

alanz reviewed Dec 26, 2019

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

fendor commented Dec 27, 2019

View reviewed changes

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs Outdated Show resolved Hide resolved

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs Show resolved Hide resolved

fendor requested review from alanz and lukel97 December 27, 2019 12:25

fendor commented Dec 27, 2019

View reviewed changes

src/Haskell/Ide/Engine/Server.hs Outdated Show resolved Hide resolved

src/Haskell/Ide/Engine/Server.hs Outdated Show resolved Hide resolved

test/functional/HieBiosSpec.hs Outdated Show resolved Hide resolved

test/functional/HieBiosSpec.hs Outdated Show resolved Hide resolved

fendor force-pushed the dont-depend-on-ghc-at-run-time branch from bb361f6 to 61f9391 Compare December 27, 2019 13:34

fendor changed the title ~~WIP: Find the libdir directory of ghc at run-time~~ Find the libdir directory of ghc at run-time Dec 27, 2019

fendor force-pushed the dont-depend-on-ghc-at-run-time branch 9 times, most recently from ad9e3a5 to fe25526 Compare December 28, 2019 18:07

fendor added 4 commits December 28, 2019 19:07

Find the libdir directory of ghc at run-time

f77fa4c

Update tests to select libdir based on cradle

fd66fa7

Express getProjectGhcVersion with execProjectGhc

b5290e0

Fallback to normal ghc if stack fails

Move version check into onInitialisation

21f894a

fendor force-pushed the dont-depend-on-ghc-at-run-time branch from fe25526 to 21f894a Compare December 28, 2019 18:08

fendor commented Dec 28, 2019

View reviewed changes

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs Outdated Show resolved Hide resolved

hie-plugin-api/Haskell/Ide/Engine/Cradle.hs Outdated Show resolved Hide resolved

src/Haskell/Ide/Engine/Scheduler.hs Show resolved Hide resolved

src/Haskell/Ide/Engine/Version.hs Outdated Show resolved Hide resolved

This was referenced Dec 28, 2019

GHC 8.8 support #1482

Merged

HIE unable to find ghc binary for stack project #1504

Closed

fendor and others added 3 commits December 28, 2019 21:50

Cleanup code, comments and old variable names

5741038

Improve error message of unparsable hie.yaml

c3ca30e

Use new lsp-test to fix hieBiosError test case

fde449b

fendor mentioned this pull request Dec 29, 2019

HIE crashes if there is no "ghc" on the path with explicit stack config #1495

Closed

alanz reviewed Dec 29, 2019

View reviewed changes

alanz approved these changes Dec 29, 2019

View reviewed changes

lukel97 approved these changes Dec 29, 2019

View reviewed changes

alanz merged commit 310450e into haskell:master Dec 29, 2019

Dec 2019 Release automation moved this from In progress to Done Dec 29, 2019

alanz added a commit to alanz/haskell-ide-engine that referenced this pull request Dec 29, 2019

Adapt GhcModPluginSpec after merge of haskell#1496

f99237b

This missed CI as it came in from haskell#1505

alanz added a commit that referenced this pull request Dec 29, 2019

Merge pull request #1507 from alanz/fix-master

072ff7f

Adapt GhcModPluginSpec after merge of #1496

fendor mentioned this pull request Dec 29, 2019

HIE crashes if there is no "ghc" on the path with explicit stack config mpickering/haskell-ide-engine#60

Closed

jneira mentioned this pull request Jan 9, 2020

Windows 10, VS Code: HIE server crashes - spaces in path not handled properly? haskell/vscode-haskell#84

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Find the libdir directory of ghc at run-time #1496

Find the libdir directory of ghc at run-time #1496

fendor commented Dec 23, 2019 •

edited

alanz Dec 23, 2019

fendor Dec 23, 2019

alanz Dec 23, 2019

lukel97 Dec 24, 2019

fendor Dec 24, 2019

alanz Dec 24, 2019

fendor Dec 24, 2019

alanz Dec 24, 2019

alanz Dec 26, 2019

fendor Dec 26, 2019

lukel97 Dec 24, 2019

fendor commented Dec 29, 2019

alanz Dec 29, 2019

Find the libdir directory of ghc at run-time #1496

Find the libdir directory of ghc at run-time #1496

Conversation

fendor commented Dec 23, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fendor commented Dec 29, 2019

Choose a reason for hiding this comment

fendor commented Dec 23, 2019 •

edited