Distributable binaries#165
Conversation
I hate that I have to do this
Practically, CabalHelper is only used in HLS-wrapper. In my opinion, we may remove it if it is not really used. Also, there is the implementation of https://github.com/haskell/haskell-language-server/blob/master/src/Ide/Cradle.hs#L158 which obtained the libdir in Haskell IDE Engine. That should work, right? |
409ca9c to
773ec86
Compare
43fb062 to
53c3676
Compare
|
The stuff that needs to go into ghcide has PRs open here, but has been cherry-picked into the ghcide submodule for now haskell/ghcide#697 haskell/ghcide#696 |
| # url = https://github.com/wz1000/ghcide.git | ||
| url = https://github.com/fendor/ghcide.git | ||
| # url = https://github.com/fendor/ghcide.git | ||
| url = https://github.com/bubba/ghcide.git |
There was a problem hiding this comment.
Is this the correct upstream repo?
There was a problem hiding this comment.
Until those other PRs and @wz1000s PRs get merged into ghcide, I think so. This checkout is just @fendor's previous submodule + my patches cherry-picked on top
| @@ -0,0 +1,72 @@ | |||
| # Releases and distributable binaries | |||
|
|
|||
| Starting with 0.X.0.0 haskell-language-server provides pre-built binaries on | |||
There was a problem hiding this comment.
I imagine 0.3.0.0, if we merge soon.
use split sections
The cabal-helper cradle was only used by the wrapper for detecting the project GHC version in the absence of an explicit hie.yaml file, whilst ghcide itself used the hie-bios implicit cradle logic. This brings the two in sync so the wrapper should behave more predictably now.
…age-server into github-action-builds
|
I hope we will not miss cabal-helper in the future (although for missing something you need remembering it 😝 ) |
| @@ -1,862 +0,0 @@ | |||
| {-# LANGUAGE ScopedTypeVariables #-} | |||
There was a problem hiding this comment.
Here rests my biggest contribution to HIE.
RIP
There was a problem hiding this comment.
The most beautifully documented module ⚰️
| -- | Find the cradle that the given File belongs to. | ||
| -- | ||
| -- First looks for a "hie.yaml" file in the directory of the file | ||
| -- or one of its parents. If this file is found, the cradle | ||
| -- is read from the config. If this config does not comply to the "hie.yaml" | ||
| -- specification, an error is raised. | ||
| -- | ||
| -- If no "hie.yaml" can be found, the implicit config is used. | ||
| -- The implicit config uses different heuristics to determine the type | ||
| -- of the project that may or may not be accurate. | ||
| findLocalCradle :: FilePath -> IO (Cradle Void) | ||
| findLocalCradle fp = do | ||
| cradleConf <- findCradle fp | ||
| crdl <- case cradleConf of | ||
| Just yaml -> do | ||
| hPutStrLn stderr $ "Found \"" ++ yaml ++ "\" for \"" ++ fp ++ "\"" | ||
| loadCradle yaml | ||
| Nothing -> loadImplicitCradle fp | ||
| hPutStrLn stderr $ "Module \"" ++ fp ++ "\" is loaded by Cradle: " ++ show crdl | ||
| return crdl |
There was a problem hiding this comment.
Dont we need something similar for exe/Main.hs? So, should be put into the library, also for writing tests against it.
There was a problem hiding this comment.
The corresponding line in Ghcide's exe/Main.hs (src/Development/Ide/Session.hs now) is
cradle <- maybe (loadImplicitCradle $ addTrailingPathSeparator dir) loadCradle hieYamlbut it looks like the hieYaml is cached in some memo thingy
cradleLoc <- liftIO $ memoIO $ \v -> do
res <- findCradle v
-- Sometimes we get C:, sometimes we get c:, and sometimes we get a relative path
-- try and normalise that
-- e.g. see https://github.com/digital-asset/ghcide/issues/126
res' <- traverse makeAbsolute res
return $ normalise <$> res'I would have liked to have shared that bit but I'm not 100% sure how to abstract it
There was a problem hiding this comment.
Is that still used? I can imagine that we do not need to cache this.
There was a problem hiding this comment.
@fendor After looking at it a bit closer I think the memoisation part does make sense, since I presume that shake action can get called/executed (not sure what the terminology is) multiple times on an individual file. And since findCradle does a lot of directory traversing we probably don't want to do that every single time
fendor
left a comment
There was a problem hiding this comment.
One small question, other than that, just awesome!
| import Data.Void | ||
| import HIE.Bios | ||
| import Ide.Cradle (findLocalCradle) | ||
| import Ide.Logger (logm) |
There was a problem hiding this comment.
Why is the logger gone here?
There was a problem hiding this comment.
The logger never worked in the first place from what I could tell, since the wrapper didn't call the haskell-lsp setup function! So all those logm/debugm calls seemed to just go down the drain.
| @@ -0,0 +1,72 @@ | |||
| # Releases and distributable binaries | |||
|
@bubba sorry, due to windows build issues i had no time to test it again. 😟 |
Ignore the commits for now, they should be squashed when merged!
This PR contains a lot of stuff, a lot of it coming from upstream components so it might not be visible in the diff:
stackisn't on the path, it will report an error then and there, rather than having hls/ghcide confusingly fail later on. The vscode extension uses this new error message as well to provide a pop up message linking the user to a website to install the missing tool