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

Use implicit-hie when no explicit hie.yaml #782

Merged
merged 4 commits into from
Sep 13, 2020
Merged

Use implicit-hie when no explicit hie.yaml #782

merged 4 commits into from
Sep 13, 2020

Conversation

jneira
Copy link
Member

@jneira jneira commented Sep 10, 2020

  • The test suite hangs in my windows 10 😟
  • I've tried several projects without hie.yaml and seems to work fine

//cc @Avi-D-coder

@jneira jneira requested review from pepeiborra and fendor September 10, 2020 21:47
@pepeiborra
Copy link
Collaborator

pepeiborra commented Sep 11, 2020

See my comment in haskell/haskell-language-server#386 (comment)
What does loadImplicitHieCradle do for projects that have no Cabal/Stack descriptors?

The test suite hangs because lsp-test has a timeout of 30? seconds, thus failing tests become super slow.

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

EDIT: I have just read the comment, and I think this is correct, gen-hie works for cabal and stack projects, only.

Comment on lines -204 to -207
-- This is needed to prevent a GHC crash when building
-- Development.IDE.Session with stack on 8.10.1 on Windows
if (impl(ghc > 8.9) && os(windows))
ghc-options: -fexternal-interpreter
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why has this been removed?

Copy link
Member Author

@jneira jneira Sep 11, 2020

Choose a reason for hiding this comment

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

Sorry for nor mentioning it, my local windows build was failing due to a segfault in the external interpreter (the thing that line try to fix in ci!)
The ci job has failied for other reason, let's see if it continue failing due to segfault without -fexternal-interpreter after fixin the actual error.
//cc @bubba

@fendor
Copy link
Collaborator

fendor commented Sep 11, 2020

We maybe need more code-paths to support stand-alone files.

@jneira
Copy link
Member Author

jneira commented Sep 11, 2020

We maybe need more code-paths to support stand-alone files.

jumm i assumed incorrectly gen-hie would keep the fallback to the direct cradle with no build system present. I'll take a look.

@jneira
Copy link
Member Author

jneira commented Sep 11, 2020

My original plan was use loadImplicitHieCradle and fallback to actual hie-bios loadImplicitCradle in case the first one fails with an error. I did something similar between the implicit and cabal-helper one here: https://github.com/jneira/haskell-language-server/blob/9969b75785ba8d6e5538c177f297008d4038e992/src/Ide/Cradle.hs#L62-L92
But i think we decided in other pr to keep only one path, maybe it is time to reconsider it.

@fendor
Copy link
Collaborator

fendor commented Sep 11, 2020

Can we turn loadImplicitHieCradle into loadImplicitHieCradle :: FilePath -> IO (Maybe (Cradle a))? E.g. only return a cradle if it is stack or cabal? If it is Nothing, we can default to hie-bios implicit cradle.

EDIT: My motivation is, that this is essentially a single code-path, since the implicit cradle won't select cabal or stack since it is always covered by implicit-hie.

@jneira
Copy link
Member Author

jneira commented Sep 11, 2020

Well, i am not sure if hie-implicit-cradle (or ghcide?) does not handle standlone files (although gen-hie does not for sure):

D:\ws\haskell\standalone-hs>ghcide
ghcide version: 0.3.0 (GHC: 8.8) (PATH: D:\bin\ghcide.exe)
Ghcide setup tester in D:\ws\haskell\standalone-hs.
Report bugs at https://github.com/digital-asset/ghcide/issues

Step 1/4: Finding files to test in D:\ws\haskell\standalone-hs
Found 1 files

Step 2/4: Looking for hie.yaml files that control setup
Found 1 cradle
  ()

Step 3/4: Initializing the IDE

Step 4/4: Type checking the files
[INFO] Consulting the cradle for "D:\\ws\\haskell\\standalone-hs\\Main.hs"
Output from setting up the cradle Cradle {cradleRootDir = "D:\\ws\\haskell\\standalone-hs", cradleOptsProg = CradleAction: Default}
[INFO] Using interface files cache dir: C:\Users\user\AppData\Local\ghcide\main-da39a3ee5e6b4b0d3255bfef95601890afd80709
[INFO] Making new HscEnv[main]

Completed (1 file worked, 0 files failed)

D:\ws\haskell\standalone-hs>ghcide-impl-hie
ghcide version: 0.3.0 (GHC: 8.8) (PATH: D:\bin\ghcide-impl-hie.exe)
Ghcide setup tester in D:\ws\haskell\standalone-hs.
Report bugs at https://github.com/digital-asset/ghcide/issues

Step 1/4: Finding files to test in D:\ws\haskell\standalone-hs
Found 1 files

Step 2/4: Looking for hie.yaml files that control setup
Found 1 cradle
  ()

Step 3/4: Initializing the IDE

Step 4/4: Type checking the files
[INFO] Consulting the cradle for "D:\\ws\\haskell\\standalone-hs\\Main.hs"
Output from setting up the cradle Cradle {cradleRootDir = "D:\\ws\\haskell\\standalone-hs", cradleOptsProg = CradleAction: Default}
[INFO] Using interface files cache dir: C:\Users\user\AppData\Local\ghcide\main-da39a3ee5e6b4b0d3255bfef95601890afd80709
[INFO] Making new HscEnv[main]

Completed (1 file worked, 0 files failed)

ghcide is HEAD and ghcide-impl-hie is this pr
I will test it with vscode too but maybe @Avi-D-coder could confirm it
I've opened the hs file with this pr version and it works

@pepeiborra
Copy link
Collaborator

I think this is great and I'm happy to merge it, but I have a couple of requests that can either be done in this PR of in follow-ups:

  1. Inform the user that there is no cradle #788
  2. Write the implicit cradle to disk
  3. Give the user a choice if there are both stack and cabal project descriptors
  4. Delete all the cradle files from ghcide and hls - no longer needed

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM, the rest can be adressed in follow-up PRs

@pepeiborra pepeiborra merged commit f79e930 into haskell:master Sep 13, 2020
@jneira
Copy link
Member Author

jneira commented Sep 13, 2020

I think this is great and I'm happy to merge it, but I have a couple of requests that can either be done in this PR of in follow-ups:

1. Inform the user that there is no cradle #788

2. Write the implicit cradle to disk

3. Give the user a choice if there are both stack and cabal project descriptors

4. Delete all the cradle files from ghcide and hls - no longer needed

Agree with all of them, i would add a fifth one: state in the (at least) the hls readme the cases when the implicit cradle will fail: common stanzas and conditionals that introduce new modules+path definitions. I think (hope?) they are not so frequent but still.

@jneira jneira deleted the implicit-hie branch October 6, 2020 05:36
pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Use implicit-hie when no explicit hie.yaml

* Use implicit-hie-cradle master in all build config files

* Set correct hie-bios version for ghc-8.10.1

* Fix windows ci build
pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Use implicit-hie when no explicit hie.yaml

* Use implicit-hie-cradle master in all build config files

* Set correct hie-bios version for ghc-8.10.1

* Fix windows ci build
pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Use implicit-hie when no explicit hie.yaml

* Use implicit-hie-cradle master in all build config files

* Set correct hie-bios version for ghc-8.10.1

* Fix windows ci build
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.

3 participants