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

Drop hackage-index from cabal.project #1496

Closed
jneira opened this issue Mar 4, 2021 · 6 comments
Closed

Drop hackage-index from cabal.project #1496

jneira opened this issue Mar 4, 2021 · 6 comments
Labels
CI Continuous integration status: in discussion Not actionable, because discussion is still ongoing or there's no decision yet

Comments

@jneira
Copy link
Member

jneira commented Mar 4, 2021

  • It takes some time to maintain and cause build errors if you forget to update it (and it happens to me quite frequently 😅 )
  • We have to update it frequently to get fixes and features from upstream packages and we are breaking the cache each time
  • We live in a sweet frozen moment in the past but our hackage users dont and we are missing broken builds in hackage due to bad bounds (like the recent bump of haddock-library which broke ghcide-0.7.5 and master but we did not detect neither)
@jneira jneira added CI Continuous integration status: in discussion Not actionable, because discussion is still ongoing or there's no decision yet labels Mar 4, 2021
@jneira
Copy link
Member Author

jneira commented Mar 5, 2021

Ping @pepeiborra @wz1000 @Ailrun @alanz (and the rest o maintainers, of course)

@michaelpj
Copy link
Collaborator

Including the index-state is really helpful for people contributing to the project to know that they can work on it without having to worry about bounds issues. It's a real step forward in that way, I'd be sad to see it go.

we are breaking the cache each time

Removing it just means you'll break the cache non-deterministically or when you do cabal update.

we are missing broken builds in hackage due to bad bounds

I think you will still miss bounds issues because it will depend on the state of the hackage index on your computer.

i.e. At the moment to be sure the bounds are okay at time T you must:

  • cabal update
  • Update index-state
  • Try a build

Without the index-state you still need to do step 1! And if you remember to go "oh, I should check whether this works with Hackage-as-of-today... it's not so hard to bump the index-state too.

Importantly, the computer-dependence also switches the burden of who finds bounds issues. At the moment, you find bounds issues when you bump the index-state, i.e. when you're doing maintenance and you want to check that things work with newer versions. If we don't have the index state, random contributors will find bounds issues when they happen to try and contribute to HLS with too new a Hackage index. It super sucks to have to fix bounds issues before you can make a contribution (and it makes the project seem broken), and I don't think that's the division of labour that we want.

@jneira
Copy link
Member Author

jneira commented Mar 5, 2021

Thanks for your insights.

Without the index-state you still need to do step 1! And if you remember to go "oh, I should check whether this works with Hackage-as-of-today... it's not so hard to bump the index-state too.

It is not hard but i miss it once and once again 😓, my obtuseness is the main reason for sure (although i know i am not the unique one!) but the fact is it is not a extended practice for other projects and you have to do it only for hls, helps to miss it. You can't miss cabal update for any cabal workflow you follow and cabal warns you about if it is outdated.

It super sucks to have to fix bounds issues before you can make a contribution (and it makes the project seem broken), and I don't think that's the division of labour that we want.

Fair enough, moreover given you had to open issues about bad bounds 🙂

we are missing broken builds in hackage due to bad bounds

I think you will still miss bounds issues because it will depend on the state of the hackage index on your computer.

well maybe we could fire another ci job (maybe not required to merge, or scheduled) with a build with last hackage index (removing or disabling via flag the index-state) and do a cabal update before.

@michaelpj
Copy link
Collaborator

michaelpj commented Mar 5, 2021

Fair enough, moreover given you had to open issues about bad bounds

Ugh, actually, now that you point it out, that was because of trying to build it from Hackage. So pinning the index state does mean that people pulling from Hackage might get issues. Which is also not good. So that puts the problems on a different set of users 😔

Maybe updating the index state and checking the bounds should be a step in the release process? It's hard to stop things breaking when Hackage changes, but at least we can check when we upload.

@hasufell
Copy link
Member

hasufell commented Mar 8, 2021

index state was introduced here haskell/haskell-ide-engine#1561

One could argue the index state should live in cabal.project.freeze (cabal new-freeze btw now also puts it there). I believe the PR provides the motivation of why this was done, which is basically what @michaelpj said. Removing it is a step backwards.

So I'd argue the project should use a freeze file generated by cabal. Bumping the index state then is simply:

  1. rm cabal.project.freeze
  2. cabal update
  3. cabal freeze

Another way would be to use stack2cabal. It seems to work for well for this repo, but keep in mind that it uses its own yaml parser, since stack doesn't expose its internals.


wrt hackage: afais haskell-language-server.cabal already uses the ^>= operator, which should pin the direct dependencies in an API compatible manner. If it doesn't, then someone else messed up (transitive dependencies etc).

@jneira
Copy link
Member Author

jneira commented Mar 8, 2021

Ok, so i think we should keep it and try to catch or prevent errors due to dependencies updates in other ways: setting bounds (and maintining them!), with ci jobs, etc

@jneira jneira closed this as completed Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration status: in discussion Not actionable, because discussion is still ongoing or there's no decision yet
Projects
None yet
Development

No branches or pull requests

3 participants