Skip to content

CI: don't allow-newer for GHC 9.10 #10194

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

Merged
merged 4 commits into from
Jul 13, 2024
Merged

CI: don't allow-newer for GHC 9.10 #10194

merged 4 commits into from
Jul 13, 2024

Conversation

ulysses4ever
Copy link
Collaborator

@ulysses4ever ulysses4ever commented Jul 11, 2024

When updating to GHC 9.10 (#9914), we had to put a bunch of allow-newer's and pull some patches from head.hackage. This shouldn't be needed after the ecosystem has caught up. This PR shows that the only issue remaining is rere (ulysses4ever/rere#28).

We didn't apply the bandaids (allow-newer's and head.hackage source repo) unconditionally and instead used impl(ghc >= 9.10.0). Now, we don't remove this stuff, we bump the conditional from 9.10.0 to 9.12.0 anticipating the same procedure for the future (yet unplanned) release of GHC 9.12.

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@ulysses4ever ulysses4ever marked this pull request as draft July 11, 2024 14:06
@ulysses4ever
Copy link
Collaborator Author

Oh rere... ulysses4ever/rere#28

@ulysses4ever
Copy link
Collaborator Author

A bunch of our internal packages didn't get version bumps as they should have as a part of #9914. I really want to create an "update the project for a new GHC" guide...

@ulysses4ever ulysses4ever force-pushed the no-allow-newer-for-9.10 branch from 032c948 to cfc34b3 Compare July 11, 2024 14:48
@ulysses4ever
Copy link
Collaborator Author

Looks fine now, ready for reviews...

@ulysses4ever ulysses4ever added the squash+merge me Tell Mergify Bot to squash-merge label Jul 11, 2024
@andreabedini
Copy link
Collaborator

andreabedini commented Jul 12, 2024

Thanks @ulysses4ever.
At risk of telling something obvious, I want to highlight that, for a source distribution, it is meaningless to pass CI in presence of allow-newer, constraints or source-repository-package.
Nobody has that information because it is not part of the source package metadata, so passing CI does not guarantee that anybody else will be able to do the same.

We learnt this lesson the hard way :-)

https://github.com/IntersectMBO/cardano-node/blob/269dbda47ae538e355e837539d3e0e9833642fc7/cabal.project#L64-L66

@ulysses4ever
Copy link
Collaborator Author

@andreabedini thank you for chiming in. I guess I didn't document this shim enough. The only purpose of it is to catch Cabal issues created by a latest GHC release. If we don't have it, we won't know that we have such issues until all our dependencies update for the latest GHC, which is too late / not ideal (e.g. rere is still not updated for GHC 9.10). This probably doesn't make the life of downstream users easier, but it allows us to patch Cabal for the latest GHC earlier rather than later. Although if the downstream users are really eager to build with the latest GHC, figuring out the constraints/source repositories is easier (they could ask us, if they really eager, and we'll point them to this project file) than patching Cabal if it's not ready. Is it clear enough? Should I add more explanation to the top of the project file that this PR updates?

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jul 13, 2024
@mergify mergify bot merged commit cb34d2a into master Jul 13, 2024
53 checks passed
@mergify mergify bot deleted the no-allow-newer-for-9.10 branch July 14, 2024 00:00
@geekosaur
Copy link
Collaborator

@mergify backport 3.12

Copy link
Contributor

mergify bot commented Jul 14, 2024

backport 3.12

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jul 14, 2024
* CI: don't allow-newer for GHC 9.10

* allow-newer base for rere

* buildinfo-reference-generator: bump base bound

* cabal-testsuite: bump base bound

(cherry picked from commit cb34d2a)
@andreabedini
Copy link
Collaborator

@ulysses4ever I hope I didn't come across too grumpy. I was aware of the (noble!) intentions. Nevertheless I am trying to resist the temptation to couple things more and more. If we put the entire Haskell ecosystem in a single monorepo we won't have any of the problems you mention, but also it's not the solution we want (IMHO I admit). Perhaps GHC should not cause so much churn.

I would suggest that cabal (at least cabal-install), only tests against released version of GHC and Hackage packages. My 2c :-)

@ulysses4ever
Copy link
Collaborator Author

@ulysses4ever I hope I didn't come across too grumpy

Not at all! Please, keep sharing your thoughts. This is very helpful.

I would suggest that cabal (at least cabal-install), only tests against released version of GHC and Hackage packages.

I have so many questions regarding this proposal, I don't know where to start :-)

The idea indeed is to test against a released version eventually. But adding support for it takes weeks (for me, anyway). So, the alpha business is there to shorten the lag between the release and our support for it. It seems to me that your proposal suggests that you don't support the idea of alpha releases at all. Is that right?

On a more general note, please, try to propose concrete changes to the current process. I was updating for new GHCs for some time and it takes a lot of time, and these shims (alow-newer and head.hackage) save my personal time. I am happy to pass this task on to anyone who volunteers doing it, and they may implement whatever process they see better. I'm also happy to accept concrete suggestions about my process. E.g. don't allow to merge anything with allow-newer. But we'd need to discuss the consequences of such concrete restrictions. E.g. we'd not have GHC 9.10 today if we disallow allow-newer (rere restricts 9.10).

Suggesting "don't test against alpha's" is not concrete enough. As a more concrete option, we can agree that we don't merge CI changes that pull in alpha. That would be fine with me. In that case, I'd open a PR with the alpha, and then change it to the released version when one appears. This will add pressure on the PR author because they'll need to monitor the release date, but that's not too bad. But the issue of allow-newer and head.hackage remains. Do you want to disallow them altogether? Even in conditionals for latest GHCs? In that case, we're back to square one with us not knowing whether cabal builds with the latest GHC modulo version bounds. That's pretty useful knowledge to have IMO.

@andreabedini
Copy link
Collaborator

On a more general note, please, try to propose concrete changes to the current process. I was updating for new GHCs for some time and it takes a lot of time, and these shims (alow-newer and head.hackage) save my personal time.

This is absolutely fair. Let me just make that the time you have invested in this is hugely appreciated (and not only by me). I'll take note of what you wrote and mull over it ☺️

mergify bot added a commit that referenced this pull request Jul 17, 2024
CI: don't allow-newer for GHC 9.10 (backport #10194)
erikd pushed a commit to erikd/cabal that referenced this pull request Jan 9, 2025
* CI: don't allow-newer for GHC 9.10

* allow-newer base for rere

* buildinfo-reference-generator: bump base bound

* cabal-testsuite: bump base bound
erikd pushed a commit to erikd/cabal that referenced this pull request Jan 9, 2025
* CI: don't allow-newer for GHC 9.10

* allow-newer base for rere

* buildinfo-reference-generator: bump base bound

* cabal-testsuite: bump base bound
@ulysses4ever ulysses4ever restored the no-allow-newer-for-9.10 branch February 21, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants