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

Repair autobuilds #253

Merged
merged 8 commits into from
Jan 24, 2024
Merged

Repair autobuilds #253

merged 8 commits into from
Jan 24, 2024

Conversation

schmonz
Copy link
Member

@schmonz schmonz commented Jan 13, 2024

vmactions has new versions that work differently under the hood: instead of relying on VirtualBox on a particular old macOS, they run qemu under ubuntu-latest. These seem as quick as they used to be (including Solaris, which is back to needing 10 minutes or so), hopefully reliable again, and almost certainly less likely to break over time.

With FreeBSD the only one not on Actions (CirrusCI) and the only one still not working, I converted it to Actions as well.

I think I had been seeing each of our jobs running twice on every push to a branch that's part of a PR: once on push, and once on pull_request. If I misinterpreted what I saw, then the on: stanzas (and the if: github.event_name == 'push' || github.event.pull_request.merged == true)) are unnecessarily complicated and we can put them back as they were. Also, I haven't tested that the jobs really do run when a PR is merged! When we merge this one, we have to watch carefully.

Now that feedback is fast again, I've expanded scope, adding NetBSD (x86_64) and Alpine (many architectures). Alpine's s390x build shows a bug in one of our tests!

Now that there are many more variants, I've changed how jobs are named: "defaults" (e.g., gcc, 64-bit ABI, nroff) are left out, only "non-defaults" (e.g., clang, 32-bit ABI, mandoc or true) are shown. I'm hoping this makes it easier to differentiate when something's broken.

Some of the variants are probably redundant. For example, the nroff and no-roff variants are probably covered well enough with one BSD, one GNU, and one Solaris make (and then the platforms with mandoc in place of nroff can specify no more than that). Likewise, -DDEPRECATED_FUNCTIONS_REMOVED probably needn't be a variant in every single build.

FreeBSD does a whole make package run-through, with and without DESTDIR. FreeBSD also has possibly the most thorough -W settings, to make sure we don't backslide on classes of warnings we've fixed. Maybe we want to extend one or both of these to all platform builders?

Whenever it was that the VM builders suddenly got unreliable, I felt frustrated and a bit powerless. Of course it would have been smart for me to check whether there were new @vmactions ready (or being worked on), but I didn't think of that until very recently. So I'm wishing for an automated way to be notified when @vmactions updates come out and/or the available OS version list changes, or the CodeQL action, or the ubuntu or macos version lists.

There's a lot of duplication across the .yml files. I don't think this PR should wait until we figure out how to reduce the duplication, but I'm hoping it'll get better as we learn more about Actions.

Solaris might be a little faster with the cache action reusing our hand-built libcheck artifact, but will probably keep being slow no matter what.

Once this is ready to merge, I'll make sure the master branch protection rules are updated to require all the variants we decided to keep.

My next assignment should probably be #225!

@DerDakon
Copy link
Member

FreeBSD builds had a 3x2 matrix, i.e. all 3 versions used both nroff and true. Did you change that intentionally?

@schmonz
Copy link
Member Author

schmonz commented Jan 14, 2024

FreeBSD builds had a 3x2 matrix, i.e. all 3 versions used both nroff and true. Did you change that intentionally?

Sorta -- I figured we had enough coverage of those combinations in the OpenBSD builds. But given how quick these builds seem to be, happy to put it back.

Copy link

@josuah josuah left a comment

Choose a reason for hiding this comment

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

I was wondering where all these vmactions were coming from...
https://github.com/vmactions/vbox/blob/main/vbox.sh

It is all qemu under the hood! Big joke on Github to let users spin VM to have whichever system they actually want!

I suppose Github will come with a home-grown solution to avoid users spending so many minutes at some point... But this is Github problem to solve, and it makes sense to just jump in the train until then!

Thank you once again for this continuous effort for maintainance. On my side, I am under water with firmware development, which I do exclusively these days.

Copy link

@josuah josuah left a comment

Choose a reason for hiding this comment

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

That is a lot fewer declarations this way.
We will quickly see if this still works as intended upon the next runs.

Thank you for this refresh-up!

.github/workflows/native-ubuntu.yml Outdated Show resolved Hide resolved
.github/workflows/native-macos.yml Outdated Show resolved Hide resolved
.github/workflows/vm-alpine.yml Outdated Show resolved Hide resolved
.github/workflows/vm-freebsd.yml Outdated Show resolved Hide resolved
.github/workflows/vm-netbsd.yml Outdated Show resolved Hide resolved
@schmonz schmonz force-pushed the repair-autobuilds branch 2 times, most recently from 7451024 to 62a4c52 Compare January 18, 2024 11:56
@DerDakon
Copy link
Member

I would like to see that the build indeed runs at merge as intended, otherwise LGTM.

@schmonz
Copy link
Member Author

schmonz commented Jan 22, 2024

I would like to see that the build indeed runs at merge as intended

https://github.com/notqmail/notqmail/actions/runs/7614561039 was triggered on closed. I guess we haven't tested that closing-without-merging a PR does not trigger the workflow, but probably not, and I'm comfortable with that risk. ;-)

@schmonz schmonz requested a review from DerDakon January 22, 2024 17:00
@schmonz schmonz merged commit 70762e1 into master Jan 24, 2024
39 of 40 checks passed
@schmonz schmonz deleted the repair-autobuilds branch January 24, 2024 22:23
@schmonz
Copy link
Member Author

schmonz commented Jan 24, 2024

Merged, and updated the master branch protection rules to correspond to these builds (plus the Analyze check).

@schmonz schmonz linked an issue Jan 25, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

need reliable multi-platform builds
4 participants