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

CI: Build artifacts using GHC 9.2.7, exclude Windows GHC 8.10.7 job #1962

Merged
merged 4 commits into from
Oct 16, 2023

Conversation

RyanGlScott
Copy link
Contributor

@RyanGlScott RyanGlScott commented Oct 16, 2023

Resolves #1961, in the sense that the issue is now worked around.

This will be important in a follow-up commit that allows the GHC 8.10.7 job to
fail on Windows. Doing so requires that this job not impact any other jobs in
the workflow.
Resolves #1961, in the sense that the issue is now worked around.
@kquick
Copy link
Member

kquick commented Oct 16, 2023

I would not be opposed to removing the retry logic in the CI if you wanted to do that as well.

@RyanGlScott
Copy link
Contributor Author

Indeed, I am rather skeptical of the utility that retrying cabal build offers, considering that it's one of the less flaky parts of the CI. (That is, if it fails, it's usually for sensible reasons.) I'll push a separate commit to remove it.

The logic for retrying the `cabal build` command three consecutive times in a
row upon failure was apparently added to work around macOS-related dylib
issues, but it is unclear (and unlikely) if these issues still exist. Moreover,
this logic has the distinct disadvantage of potentially masking more serious
issues, such as the issues observed in #1961.

In principle, `cabal build` should not be an especially flaky part of the CI—it
should either work 100% of the time or fail 100% of the time. Let's just remove
the `retry cabal build` logic to make it easier to notice issues with `cabal
build` in the future. We can always revisit this choice later if need be.
@RyanGlScott RyanGlScott marked this pull request as ready for review October 16, 2023 16:58
@RyanGlScott RyanGlScott requested a review from kquick October 16, 2023 16:58
Copy link
Member

@kquick kquick left a comment

Choose a reason for hiding this comment

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

Wouldn't it be convenient if these files supported variables so we only had to change this in one place...

Just a comment suggestion, otherwise this looks good.

.github/ci.sh Show resolved Hide resolved
@RyanGlScott RyanGlScott added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Oct 16, 2023
@mergify mergify bot merged commit 35396bc into master Oct 16, 2023
39 checks passed
@mergify mergify bot deleted the T1961 branch October 16, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
2 participants