-
Notifications
You must be signed in to change notification settings - Fork 718
Rewrite release CI #11072
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
base: master
Are you sure you want to change the base?
Rewrite release CI #11072
Conversation
Perhaps it would be good to first establish what the goals with this PR is as it seems to be doing a few different things. I think as a first step, migrating to github CI should match the configurations and artifacts produced by the existing release CI. Afterwards, once that is achieved then further platforms and packaging tweaks can be explored in a more controlled manner. It seems risky to both migrate to a different CI platform, modify the supported platforms and also modify packaging options all in one commit. (Not that I am expressing a judgement about the value of also achieving those things as individual goals) |
The gitlab configuration contains EOLed distros and lacks many of the artifacts that the current PR provides. Matching gitlab would be mostly a regression. My impression is that the current release CI is not properly maintained, which is what this PR is trying to solve. |
Which EOL distros are you referring to exactly? It would be good to remove those certainly. Adding new platforms for release configurations seems to be an additive task which can be achieved in follow-up discussions if cabal maintainers decide they wish to produce binaries for new platforms. What additional artifacts are produced? How are those different from the existing release artifacts? Just so we are clear here Julian, I agree with the premise of this patch. Perhaps if you consider the other changes as necessary/desirable as well, another way forward would be to produce a detailed list of what is the same/different to before, so each one can be considered individually during a review. |
There are some updates I should do to this PR as well, but here is the inventory:
|
Our strategy going forward is as follows:
We end up with the following build environments:
|
This proposal seems to be a different approach to building artifacts than the cabal project has used in the past, could you open an issue to discuss this proposal with the other maintainers? It seems to be a different topic to this MR. It would be good to build less binaries overall, perhaps this approach could be made easier by adding some features of options to |
Indeed, your strategy seems very attractive and quite radical. Is it applicable to a different major project, such as GHC? or ghcup itself or something else major? I'd feel more comfortable if I could contant existing users of such a strategy and even more comfortable, if we could piggy-back on GHC, as we do currently, using CI very similar to what GHC uses and taking advantage of the professionalism of @chreekat and good will of the Haskell Foundation in the management of the release CI. Did GHC consider this strategy? |
@Mikolaj I find your response incredibly disrespectful as a contributor, because you're insinuating "we don't trust your expertise on this matter". I'm being paid by IOG to do this type of work and we will do it anyway, with or without upstream. But as we've been asked by @bgamari and others during ZuriHac whether we intend to contribute back upstream, here is the answer: yes we do. And here is our proof. In turn, this is a great opportunity for the cabal team to demonstrate that they're willing to collaborate with other teams outside of their core "cabal". Also as a side note: the Haskell Foundation itself apparently trusts my expertise on this matter as they are funding my proposal (and I am the person managing the Haskell Foundation GitHub runners). That doesn't mean our goals align and I'm willing to make adjustments to this PR, but I can't work with you, unless the commentary remains professional and technical. |
Edited: I don't feel I can productively contribute to reviewing this PR, so let me thank @mpickering for discussing its merits up to this point and let ma ask @mpickering and other cabal maintainers and developers to carry on. |
Right, I separately chatted with Julian and Mikolaj. It does seem like there are some unresolved tensions from past interactions. Thank you for your gracious reflection Mikolaj that you would find engaging with this PR difficult. With @hasufell I discussed the ways that we could move this PR forward, and that we should agree a shared plan of work. Julian is quite keen to make the CI line-up more with what's required for ghcup, which is understandable since ghcup is the primary way which cabal binaries are distributed. Cabal maintainers are keen to not have a system which relies on one person (Julian) to maintain, and wish to understand the new system carefully so they can use it without external help in future.
Does that match your impression from our conversation @hasufell ? |
@mpickering that sounds resonable. Tomorrow I will start:
Well, I think that the bus factor will be much better with the move to GitHub, since most cabal contributors already have exposure to GitHub CI and it doesn't require intimate knowledge of runners (except for FreeBSD, but I'm confident that @geekosaur also has the required expertise to handle it). |
From
I would too like to ask for an issue with goals clearly stated for sure, and possibly trade-offs and resources considerations. |
@ffaf1 I can do that, but please note that the motivation and merits have been discussed for years and I would like to assume by now Cabal developers are well aware. There even have been synchronous calls about this topic and multiple Cabal developers/contributors have expressed positive interest in this approach. So yes, I can open an issue with motivation and explanation, but I won't be participating in argumentative discussion, nor will I try to prove why GitHub is a better decision for release CI in the context of Cabal. This is up to you to decide. This is work that we're using over at stable-haskell and I'm happy to upstream it and invest additional effort into making it upstreamable. |
Thanks for making the ticket Julian, that's just a normal part of contributing to cabal. Having a place to discuss the issue which isn't on the PR is useful for the longevity of recording these discussions. I'll get back to looking at this next week when I'm back at work if you have made the changes we discussed. |
Only thing left in this PR is executing on the "only 2 bindists for linux" plan in a separate commit. |
4914cd0
to
20415dd
Compare
@mpickering thanks for your thorough review |
05154a8
to
94df65d
Compare
main = setupAndCabalTest $ do | ||
main = do | ||
-- TODO: this might be a GHC bug that needs fixing | ||
skipIfAlpine "bug #11041" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I feel like this should be in a separate PR. I can cut it and get it in if you'd prefer not to be juggling another one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is removed, then the PR CI fails, so I disagree that this needs to be in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean I need to make another PR for this, then wait until that's reviewed + merged and then continue with this PR... then I'm afraid I don't have the time for that. As I indicated above: I don't work on this in my free time and the time allocation for dealing with this PR is running out. It's been 2 weeks now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still thinking it deserves its own PR and possibly issue for documentation reasons. Would you have a problem with my cherry-picking that commit into a separate PR without removing it here, so the two won't conflict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This is not uncommon with GHC PRs that depend on each other, BTW: use cherry-picking and let the first one to be merged win without conflicting with the other. I consider this a shortcoming of GitHub and GitLab and probably other similar UIs.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still thinking it deserves its own PR and possibly issue for documentation reasons. Would you have a problem with my cherry-picking that commit into a separate PR without removing it here, so the two won't conflict?
Sure
@@ -7,7 +7,26 @@ index-state: hackage.haskell.org 2025-06-27T20:43:14Z | |||
|
|||
-- never include this or its TH dependency in a release! | |||
package cabal-install | |||
flags: -git-rev | |||
flags: -git-rev -lukko |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Didn't we previously nuke the
lukko
stuff, or at least mark it as default off? - You should probably update the comment to indicate that it only applies to
-git-rev
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not nuked, the flag is just off by default. I want to enforce that release is always built without it, due to bugs on 32bit. If someone messes with the cabal file flags, then the release may break. This makes sure that can't happen. The releases in general should never depend on default flags. All flags needed should be explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to create a short README in the scripts/release folder with a sketch of a workflow with these new scripts? In particular, what order one is expected to call them in? This readme could be referenced from the release wiki page.
As @mpickering has indicated, this goes into the wiki: https://github.com/haskell/cabal/wiki/Making-a-release#c6-publishing-the-artifacts I will update the wiki after this PR is merged, not before. The entry point is |
I'll have to disagree with Matt on this one. That page is already bloated like hell and hard to navigate. We should strive to make it lighter, and this re-write is a great opportunity to do that. But we can agree to disagree.
I find a bunch of Because of the disagreements above, I don't feel confident to approve this PR, but I removed my "change-requested" block, so whenever Brandon is happy, it'll be easy to move on. Thanks for all the effort you put in it! Remarkable work and hopefully the one that will make our release process smoother. |
@ulysses4ever sure... I want to avoid going back and forth with minor changes, especially when there's no clear consensus among cabal devs. I think the cabal team can figure that out after it's merged. I'm happy to clear up specific questions though. |
some of my concerns were addressed and some were pushed for "future work" around the Wiki
This is a rewrite and an extension of #9437
The main motivation is to:
The major difference to the previous PR is that we run the "integration tests" of cabal-install (because that is the main part that diverges from the validate pipeline in terms of configuration and build flags).