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

Delete global cradle files (hie-cabal.yaml, hie-stack.yaml) #741

Closed
pepeiborra opened this issue Sep 13, 2020 · 21 comments · Fixed by #1230
Closed

Delete global cradle files (hie-cabal.yaml, hie-stack.yaml) #741

pepeiborra opened this issue Sep 13, 2020 · 21 comments · Fixed by #1230

Comments

@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 haskell/ghcide#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

Originally posted by @pepeiborra in haskell/ghcide#782 (comment)

@fendor
Copy link
Collaborator

fendor commented Sep 13, 2020

If we delete them right away, won't users with older versions (HIE/HLS/ghcide) be unable to work on ghcide/hls anymore?

@pepeiborra
Copy link
Collaborator Author

If they are hacking in ghcide HEAD they should be using ghcide HEAD. But there's no rush

@jneira
Copy link
Member

jneira commented Nov 16, 2020

Original issue haskell/ghcide#796

@pepeiborra pepeiborra transferred this issue from haskell/ghcide Dec 30, 2020
@jneira
Copy link
Member

jneira commented Dec 30, 2020

maybe the files could be kept as an example of a complex project config?

@pepeiborra
Copy link
Collaborator Author

Can't gen-hie provide such an example?

@jneira
Copy link
Member

jneira commented Dec 30, 2020

Well, sort of, but the output is not exactly equal (it generates a path for each module in the exe component f.e.).
You have to install and execute the tool, of course, and you can check the file simply navigating the github repo.

@pepeiborra
Copy link
Collaborator Author

A file called hie.foo.yaml in our repository is a contract - we are telling other devs that this file can be used to load this repo in hls/ghcide. Even more than that, since we are the maintainers, we are telling them that this is the way to load this repo. And now we have made our life harder: we have to make sure to keep this file working and up to date through every change, add tests for it to make sure that it doesn't accidentally break when we upgrade hie-bios or change the layout of our repo, etc...

All that for what? I personally don't use these files anymore since gen-hie became good enough. I don't use stack either, so I am not even able to regenerate the stack hie.yaml file.

To summarise, by keeping them we are endorsing the wrong approach and making our lives harder in more than one way. So no, I don't think keeping them around is a good idea.

@ndmitchell
Copy link
Collaborator

Having dedicated examples is fine, even very desirable, but I think HLS should be using best practices, to encourage people to follow. I think we should now recommend not having a hie.yaml in most cases, since its easiest and works well. I've started to delete my hie.yaml files - they force a particular choice (Cabal vs Stack) which different devs make differently, and gen-hie seems to do better than my manual files in most cases.

@jneira
Copy link
Member

jneira commented Dec 31, 2020

Fair enough, i suppose implicit-hie is good enough to push explicit hie.yaml to special corner cases

@jneira
Copy link
Member

jneira commented Jan 19, 2021

So we should remove them, however, i would like to hear the opinion of the later "maintainers" of those files. We will delete the files if nobody is strongly against.
//cc @Ailrun @beberman @tittoassini @konn

@Ailrun
Copy link
Member

Ailrun commented Jan 19, 2021

So the dev flow will be use gen-hie to generate hie.yaml by dev self?

@jneira
Copy link
Member

jneira commented Jan 19, 2021

@Ailrun I think the idea is to use the implicit cradle and only generate the explicit config if you hit some corner case where it does not work
I assume that the implicit cradle works for hls.

@konn
Copy link
Collaborator

konn commented Jan 19, 2021

I think deleting global cradle makes sense, but deleting all cradles is not a good idea.
We need some hie.yaml to make func-test work as expected; for example, for functional test for Splice plugin, importing another module in the testdata required dedicated hie.yaml IIRC.

@Ailrun
Copy link
Member

Ailrun commented Jan 19, 2021

OK then, sounds good for me. (Though I haven't tried implicit cradle for HLS...)

@jneira
Copy link
Member

jneira commented Jan 19, 2021

We need some hie.yaml to make func-test work as expected; for example, for functional test for Splice plugin, importing another module in the testdata required dedicated hie.yaml IIRC.

Yeah iiuc this is about the global ones, gonna change the description of the issue to make it clear

@jneira jneira changed the title Delete all the cradle files Delete global cradle files (hie-cabal.yaml, hie-stack.yaml) Jan 19, 2021
@pepeiborra
Copy link
Collaborator Author

One situation where it's unclear whether the implicit cradle works at all is when the hls wrapper is being used to detect the version of ghc before downloading binaries. I think this scenario requires further testing with the implicit cradle

@berberman
Copy link
Collaborator

I just tried loading HLS using implicit cradle, and it works smoothly. (though it seems that some components are recompiled repeatedly many times, which would be another issue not related to cradle)

@Ailrun
Copy link
Member

Ailrun commented Jan 21, 2021

It looks like "default" plugins do not work with implicit cradle.
@berberman, Have you tried that too? If you tried, how did you make it work?

@Ailrun
Copy link
Member

Ailrun commented Jan 21, 2021

OK, I have to admit I have agreed to delete global cradles too hastily. It looks like all plugin packages use Cabal cradle, as there is no stack.yaml next to their .cabal files.

@konn
Copy link
Collaborator

konn commented Jan 21, 2021

I have used hie.yaml.stack and global stack.yaml alone just worked; however, it turns out that deleting hie.yaml forces us to use Cabal as a backend, which forces rebuild to me...

@Ailrun
Copy link
Member

Ailrun commented Jan 21, 2021

I think using gen-hie is a way... As @pepeiborra mentioned, it is hard to manage global hie, so instead of managing global hie, let's recommend contributors to manage their own. Maybe we should put that somewhere in the contribution document.

BTW, this still does not solve the problem of "default" plugins, so we maybe need to move them into a package or request implicit-hie to handle such a case.

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

Successfully merging a pull request may close this issue.

7 participants