Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use implicit-hie #88

Closed
wants to merge 4 commits into from
Closed

Conversation

Avi-D-coder
Copy link
Collaborator

Related to haskell/hie-bios#178.
Alternative to #68
Resolves #38, until stack and cabal show-build-info.

See haskell/hie-bios#178 for bugs and progress.

@Avi-D-coder Avi-D-coder self-assigned this May 5, 2020
@fendor
Copy link
Collaborator

fendor commented May 11, 2020

Hi!
Thank you for your project!

The problem I see with implicit-hie is that it doesnt really fit the bill of replacing cabal-helper. And while I agree that cabal-helper has a lot of problems, I do not think that just generating the hie.yaml does really solve it. If it generates anything wrong, and users dont know about the hie.yaml file, you are bound to cause a lot of confusion, especially you do not aim to have a complete parser that covers any case imaginable, iirc. The first impression will be way more complex, e.g. what is this, why is it generated, should I commit this generated hie.yaml, why does it miss certain directories, etc...

So, I would like to use implicit-hie only explictly, not implicitly. To do that, I propose to use a code-lens at the top of the respective *.cabal file that writes the hie.yaml only on explicit approval of the user.
This would look roughly like this (imagining that this is a *.cabal file, not a hs source file):
example_codelens

The generated hie.yaml could be displayed in a showMessage and then the user could explicitly accept or reject the proposal. (I havent tested that this is really possible or look visually appealing)

I think making this feature opt-in makes adoption easier!

cc @jneira

@ndmitchell
Copy link
Collaborator

@fendor - I can certainly see the merits to your idea - it's nice. But I'd argue that a common problem is people who have no idea about this mechanism or hie.yaml files. For them, generating a fully commented hie.yaml always could be a real boon since now they've got explicit logging about what the tool is doing and in the comments we could even say why. That might be an alternative way to go.

@Avi-D-coder
Copy link
Collaborator Author

@fendor gen-hie generates hie.yaml files. In this PR no file is generated it just uses an improved hie-bios implicit cradle, behind the senses the logic is the same, but no file is written. If the implicit cradle is wrong, with this approach the user can just run gen-hie to see the implicit cradle config.

Cabal-helper failure mode is opaque, you can't see why it's broken. With implicit-hie you can see the config and fix it, without starting from scratch. Additionally in common use cases stack new basic-project stack build nvim test/Spec.hs cabal-helper fails in hie, where this PR succeeds.

When a new user tries to write a test with hie, the implicit cabal-helper cradle greets them with this:

[coc.nvim] Fail on initialisation for "/home/host/basic-project/test/Spec.hs". Could not obtain
 flags for: "test/Spec.hs".
2020-05-11 15:18:55.589709719 [ThreadId 4] - Run entered for HIE(hie) Version 1.2, Git revision 35f62cffb6bae6c3f86113cb0c55f52b7192689d (dirty) (3841 commits) x86_64 ghc-8.8.3
2020-05-11 15:18:55.590066799 [ThreadId 4] - Operating as a LSP server on stdio
2020-05-11 15:18:55.590229929 [ThreadId 4] - Current directory:/home/host/basic-project
2020-05-11 15:18:55.590307235 [ThreadId 4] - Operating system:linux
2020-05-11 15:18:55.590370663 [ThreadId 4] - args:["--lsp"]
2020-05-11 15:18:55.632921084 [ThreadId 4] - Cabal-Helper decided to use: ProjLocStackYaml {plStackYaml = "/home/host/basic-project/stack.yaml"}
2020-05-11 15:18:57.130011337 [ThreadId 4] - Module "/home/host/basic-project/File.hs" is loaded by Cradle: Cradle {cradleRootDir = "/home/host/basic-project", cradleOptsProg = CradleAction: Other Stack}
2020-05-11 15:18:57.130225025 [ThreadId 4] - Executing Stack GHC with args: --numeric-version
2020-05-11 15:18:57.417367194 [ThreadId 29] - Executing Stack GHC with args: --print-libdir
Using hie version: Version 1.2, Git revision 35f62cffb6bae6c3f86113cb0c55f52b7192689d (dirty) (3841 commits) x86_64 ghc-8.8.3
2020-05-11 15:18:57.735153201 [ThreadId 35] - New cradle: /home/host/basic-project/test/Spec.hs
Using hoogle db at: /home/host/.hoogle/default-haskell-5.0.17.hoo
2020-05-11 15:18:57.735971637 [ThreadId 35] - Cabal-Helper decided to use: ProjLocStackYaml {plStackYaml = "/home/host/basic-project/stack.yaml"}
2020-05-11 15:18:59.069778267 [ThreadId 35] - Module "/home/host/basic-project/test/Spec.hs" is loaded by Cradle: Cradle {cradleRootDir = "/home/host/basic-project", cradleOptsProg = CradleAction: Other Stack}
2020-05-11 15:18:59.069886873 [ThreadId 35] - Found cradle: Cradle {cradleRootDir = "/home/host/basic-project", cradleOptsProg = CradleAction: Other Stack}
2020-05-11 15:18:59.495249141 [ThreadId 62] - Fail on cradle initialisation: (ExitFailure 2)["Could not obtain flags for: \"test/Spec.hs\".","","This module was not part of any component we are aware of.","","Component: ChLibName ChMainLibName with source directory: [\"src\"]","Component: ChExeName \"basic-project-exe\" with source directory: [\"app\"]","","","To expose a module, refer to:","https://docs.haskellstack.org/en/stable/GUIDE/","If you are using `package.yaml` then you don't have to manually expose modules.","Maybe you didn't set the source directories for your project correctly."]
2020-05-11 15:18:59.49576909 [ThreadId 35] - ghcDispatcher:Got error for a request: IdeError {ideCode = OtherError, ideMessage = "Fail on initialisation for \"/home/host/basic-project/test/Spec.hs\". Could not obtain flags for: \"test/Spec.hs\".", ideInfo = Null} with mid: Nothing
2020-05-11 15:18:59.495959435 [ThreadId 35] - New cradle: /home/host/basic-project/test/Spec.hs
2020-05-11 15:18:59.496760552 [ThreadId 35] - Cabal-Helper decided to use: ProjLocStackYaml {plStackYaml = "/home/host/basic-project/stack.yaml"}
2020-05-11 15:19:00.890654384 [ThreadId 35] - Module "/home/host/basic-project/test/Spec.hs" is loaded by Cradle: Cradle {cradleRootDir = "/home/host/basic-project", cradleOptsProg = CradleAction: Other Stack}
2020-05-11 15:19:00.890755945 [ThreadId 35] - Found cradle: Cradle {cradleRootDir = "/home/host/basic-project", cradleOptsProg = CradleAction: Other Stack}
2020-05-11 15:19:01.320899254 [ThreadId 85] - Fail on cradle initialisation: (ExitFailure 2)["Could not obtain flags for: \"test/Spec.hs\".","","This module was not part of any component we are aware of.","","Component: ChLibName ChMainLibName with source directory: [\"src\"]","Component: ChExeName \"basic-project-exe\" with source directory: [\"app\"]","","","To expose a module, refer to:","https://docs.haskellstack.org/en/stable/GUIDE/","If you are using `package.yaml` then you don't have to manually expose modules.","Maybe you didn't set the source directories for your project correctly."]
2020-05-11 15:19:01.321222626 [ThreadId 35] - ghcDispatcher:Got error for a request: IdeError {ideCode = OtherError, ideMessage = "Fail on initialisation for \"/home/host/basic-project/test/Spec.hs\". Could not obtain flags for: \"test/Spec.hs\".", ideInfo = Null} with mid: Nothing


@fendor
Copy link
Collaborator

fendor commented May 11, 2020

generating a fully commented hie.yaml always could be a real boon

Sure, that is helpful in general.


gen-hie generates hie.yaml files. In this PR no file is generated it just uses an improved hie-bios implicit cradle, behind the senses the logic is the same, but no file is written

True, I overlooked that a bit. Makes the change a bit easier to accept, imo.

Cabal-helper failure mode is opaque, you can't see why it's broken.

To be fair, the implicit cradle discovery is always somewhat opaque. In HIE, we show the command that fails, so users can execute it on the command-line. I dont think that is fundatementally different to executing gen-hie (which might not be clear how to install).

Additionally in common use cases stack new basic-project stack build nvim test/Spec.hs cabal-helper fails in hie, where this PR succeeds.

That is true and a very annoying bug. But it does not mean that the tool itself is fundamentally flawed because of this. And bugs happen in every tool, although, admittedly, it shouldnt be in this trivial case.

When a new user tries to write a test with hie, the implicit cabal-helper cradle greets them with this

that is a problem of HIE and how it doesnt react to changed cradle dependencies, such as changed *.cabal files, and how we choose to match these filepaths, it is not cabal-helper's fault.


As a side note, why do you need the fork of hie-bios? You could define your own loadImplicitCradle that is used instead of the one from hie-bios. Makes this PR independent from the merge in hie-bios.

@jneira
Copy link
Member

jneira commented May 11, 2020

In this PR no file is generated it just uses an improved hie-bios implicit cradle, behind the senses the logic is the same, but no file is written. If the implicit cradle is wrong, with this approach the user can just run gen-hie to see the implicit cradle config.

Imo, to establish a implicit mechanism to configure loading a project in the wild it should potentially cover all use cases. cabal-helper can do it cause it accesses directly all possible build info for past and future versions of tools based in the Cabal library.

If it is not able to load a project it is a bug that could be fixed but no a structural limitation. I know implicit-hie aims to be a provisional solution until show-build-info and/or cabal repl file (and its counterparts in stack!) land but cabal-helper wants to integrate the first one, so we will continue using cabal-helper and the change will be hopefully transparent. But cabal-helper will not link the Cabal library at runtime (or for fewer requested info).

Same for other new features or breaking changes in cabal and stack: cabal-helper could help us as a indirection layer in the long run.

Cabal-helper failure mode is opaque, you can't see why it's broken. With implicit-hie you can see the config and fix it, without starting from scratch. Additionally in common use cases stack new basic-project stack build nvim test/Spec.hs cabal-helper fails in hie, where this PR succeeds.

Well i would argue that being implicit supposes being "opaque" in some degree. Using implicit-hie without config file would be too. It is matter of error quality and you can suggest generate a explicit hie.yaml file with gen-hie(or show it directly in the error?) to workaround cabal-helper or implicit-hie problems in the same way.

The bug with test and bench components and stack is caused by the fact that stack does not build them by default, it asks users to do a stack buils --tests --benchs. That issue is tracked in hie and in a draft pr in cabal-helper itself: DanielG/cabal-helper#92. The resolution does not seem easy but i hope it is feasible.

EDIT: The comment shares some arguments with @fendor's one but i think it could be useful

@Avi-D-coder
Copy link
Collaborator Author

Avi-D-coder commented May 11, 2020

What I mean by opaque is that cabal-helper failures for different reasons then hie-bios, the error you get from a wrong gen-hie implicit config is hie-bio's no prefix matched. I can look at the generated hie.yaml and fix the broken config. I can't fix a cabal-helper cradle.

If it is not able to load a project it is a bug that could be fixed but no a structural limitation.

The issue is that this has been broken for a long time, and is not the only issue. Your right it's not a structural limitation of cabal-helper, but not being a fundamental flaw does not make it work any better today. Cabal-helper is much more complex and I would posit leads to many of these simple issues, that are not present with hie.yaml files.

There is a reason ghcide, hls and hie all have a hie.yaml file in there repo, instead of using cabal-helper. Dead simple not complete gen-hie handles those cases where cabal-helper fails.

I know implicit-hie aims to be a provisional solution until show-build-info and/or cabal repl file (and its counterparts in stack!) land but cabal-helper wants to integrate the first one, so we will continue using cabal-helper and the change will be hopefully transparent. But cabal-helper will not link the Cabal library at runtime (or for fewer requested info).

What is cabal-helper needed for after show-build-info? Why wouldn't we just make a cradle from show-build-info directly or is that what cabal-helper will be?

@jneira
Copy link
Member

jneira commented May 11, 2020

What I mean by opaque is that cabal-helper failures for different reasons then hie-bios, the error you get from a wrong gen-hie implicit config is hie-bio's no prefix matched. I can look at the generated hie.yaml and fix the broken config. I can't fix a cabal-helper cradle.

mmm, i am little bit confused, if i understood correctly the proposal is to use an implicit-hie that will not generate any hie.yaml so there will not be a file to examine. In that context no prefix matched will not be so useful for a newcomer that never has used the hie.yaml in the first place.

Otoh you can propose the hie.yaml generated by gen-hie (with the code action f.e.) in face of cabal-helper or implicit-hie (or other cradle) errors in the same way.

What is cabal-helper needed for after show-build-info? Why wouldn't we just make a cradle from show-build-info directly or is that what cabal-helper will be?

Well, the fact is cabal-helper wants to integrate s-b-i (and we could help to do it). Otoh we will have to wait for a similar feature in stack (there is no even a open issue about afaik). We could decide then move that logic to another smaller library if we finally only are using that feature.

@Avi-D-coder
Copy link
Collaborator Author

We can include an error message that shows the yaml representation of the bios cradle. There is no equivalent transparency for cabal-helper. It either works or it doesn't.

Most of my issues are with cabal-helper's stack integration. For cabal all we need is show-build-info, gen-hie could continue to be used for stack until stack adds show-build-info.
Gen-hies stack support is already better than cabal-helper and I am continuing to improve it.

@Avi-D-coder
Copy link
Collaborator Author

Avi-D-coder commented May 12, 2020

Closing this, since it now conflicts.
Please continue discussing at #88.

@fendor fendor mentioned this pull request Jul 13, 2020
@masaeedu
Copy link

masaeedu commented Apr 9, 2021

@Avi-D-coder but this is #88 😅

@Avi-D-coder
Copy link
Collaborator Author

@masaeedu Your right this is the full context if that's what your looking for https://github.com/haskell/haskell-language-server/pulls?q=is%3Apr+Implicit+hie+cradle+is%3Aclosed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Converge on implicit cradle discovery mechanism
5 participants