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

make sure cabal-install is compatible with Cabal #10264

Merged
merged 1 commit into from
Sep 1, 2024

Conversation

geekosaur
Copy link
Collaborator

@geekosaur geekosaur commented Aug 20, 2024

See #9833

If a ghc ships with a compatible Cabal, it will be preferred by the solver on cabal install cabal-install; the new cabal-install should in fact be compatible. So we test this on release branches that have been included in at least one GHC release.

Fixes: #9833
Fixes: #9835
Fixes: #9838
Fixes: #9863

CI note: there's really no way to test this, aside from reproducing #9833 for which it's a bit too late. Testing will only be possible with the 9.12 branch and future release branches. As such, manual CI would mean making an sdist of the HEAD of 9.12 branch, then trying to build that sdist against 9.10.1 (which is essentially what this does, except that it won't actually do that test until it's backported). Note that that build may well succeed, because I've been trying to be careful with backports specifically to avoid repeating the mistakes we made with 3.10.3; the point of this PR is to try to catch that kind of failure in CI before we make a release with a breaking change.

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
Copy link
Collaborator

validate.yml is already close to incomprehensible. Any chance we can do it as a separate workflow? As a bonus, you won't need the if's.

@geekosaur
Copy link
Collaborator Author

Problem is, I need the sdists from an earlier step, or else generate them myself and then make sure the tree doesn't interfere with my install from the sdist somehow.

@geekosaur
Copy link
Collaborator Author

geekosaur commented Aug 20, 2024

Also, it's bad enough that matrix.ghc has to be updated in two places because it can't be shared; it'd be worse across multiple files. (Well, it can be shared, but it'd need to be stored as JSON in a separate file. I'm not sure if that's better or worse.)

@geekosaur geekosaur force-pushed the validate-sdists branch 2 times, most recently from 2db6a88 to 056ea72 Compare August 20, 2024 06:33
@ulysses4ever
Copy link
Collaborator

Repeating some steps in a separate workflow may be a lesser evil, I think.

Storing the matrix in a json file sounds like a reasonable idea too.

@geekosaur geekosaur force-pushed the validate-sdists branch 8 times, most recently from c610092 to 6ae49f7 Compare August 21, 2024 16:02
@geekosaur
Copy link
Collaborator Author

geekosaur commented Aug 21, 2024

Can't test this further without backporting it to a release branch. Although it'd be nice if I could identify a PR targeting master; I don't think that information is available in GHA. (ETA: it is, so the sdist jobs shouldn't run except on release branches now.)

@geekosaur
Copy link
Collaborator Author

As a bonus, you won't need the if's.

Actually, I would; they just become conditions for the workflow to run instead of conditions for various steps to run.

@ulysses4ever
Copy link
Collaborator

Which conditions would you check on a standalone workflow? You can select the platform, the GHC version, and the target branch declaratively. Not that this was super important. Most of all, I'm concerned and wary about adding anything to validate.yml at this point.

@geekosaur geekosaur force-pushed the validate-sdists branch 2 times, most recently from 0423c5f to c36d094 Compare August 21, 2024 23:47
@geekosaur geekosaur force-pushed the validate-sdists branch 8 times, most recently from 6e078c0 to 3ef5b02 Compare August 22, 2024 19:44
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's amazing!

@geekosaur geekosaur force-pushed the validate-sdists branch 5 times, most recently from 6b7d532 to 2f18811 Compare August 25, 2024 21:06
@geekosaur
Copy link
Collaborator Author

geekosaur commented Aug 25, 2024

Most recent change corrected it to actually test the problematic case: it now succeeds trivially if Cabal and friends come from Hackage, and only runs the actual test if they are bootlibs. This also means --prefer-oldest is irrelevant as the bootlib will always be preferred.

This also means it shouldn't test the latest release of GHC branches, it should test the first release. (Which in turn means that there wouldn't be any duplication with matrix.ghc in validate.yml.)

@geekosaur geekosaur force-pushed the validate-sdists branch 3 times, most recently from 92a8009 to a4544b3 Compare August 28, 2024 01:49
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

.github/workflows/check-sdist.yml Show resolved Hide resolved
@geekosaur geekosaur added the merge me Tell Mergify Bot to merge label Aug 29, 2024
@mergify mergify bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Aug 29, 2024
See haskell#9833

If a ghc ships with a compatible Cabal, it will be preferred by
the solver on `cabal install cabal-install`; the new `cabal-install`
should in fact be compatible. So we test this on release branches
that have had at least one release on Hackage. (Ideally we'd check
ghc instead, but we can't do that from GHA. Even checking Hackage
is pretty painful.)
@mergify mergify bot merged commit 94e6ad4 into haskell:master Sep 1, 2024
50 checks passed
@geekosaur
Copy link
Collaborator Author

@mergify backport 3.12

Copy link
Contributor

mergify bot commented Sep 18, 2024

backport 3.12

✅ Backports have been created

@geekosaur
Copy link
Collaborator Author

@mergify backport 3.14

Copy link
Contributor

mergify bot commented Sep 18, 2024

backport 3.14

✅ Backports have been created

  • Backport to branch 3.14 not needed, change already in branch 3.14

@geekosaur
Copy link
Collaborator Author

Note: it's fine to do this post-release, since it will only do anything once a ghc is shipped with Cabal 3.14.

@geekosaur geekosaur deleted the validate-sdists branch September 20, 2024 01:53
mergify bot added a commit that referenced this pull request Sep 20, 2024
make sure cabal-install is compatible with Cabal (backport #10264)
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 merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
3 participants