ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions #1394

This PR solves one item in #1392.

For macOS, we need to take also #1153 into account.

#1153#issue-1450259053:

maybe it makes sense to reduce the number of MacOS jobs a little bit.

I'm opening for suggestion regarding that.

The problem with this is that it removes all our tests on native ARM.

We could instead run some Linux tasks on ARM, using Bitcoin Core's ARM runner on Cirrus. But if we do this, then this makes it less much attractive to move Linux x86_64 tasks to GitHub Actions. I don't know, it's all related to each other. :/

The problem with this is that it removes all our tests on native ARM.

Want me to drop the second commit?

Converting to draft for now.

Please review #1389 first.

The problem with this is that it removes all our tests on native ARM.

Want me to drop the second commit?

Hm, to be honest, I'm not sure what I want (or what others think). But yeah, for now, I lean towards keeping the Cirrus tasks.

There are basically three independent dimensions we want to test: CPU architectures, OSes, and compilers. If you ask me, testing on architectures like ARM is crucial, so it's good to have a good number of (ideally native) ARM jobs. Testing on a particular OS like macOS can be restricted to a few configurations: It's mostly to make sure that the build works etc., because the interaction of the actual library with the OS is very limited (basically just one printf and one abort in case of error). BUT: macOS is a bit of an exception because they have Apple clang, so macOS means not only a different OS but also a different compiler...

One way forward is to keep the Cirrus CI jobs for now, and then move those to the ARM Linux runner (with the same config) in a second PR before Sep 1st. None of this should be too controversial, since it's just adding jobs, just possibly overshooting a bit. And then we could have a third PR that cleans up the overshooting, and here we can then see what exactly we want to remove.

Rebased and dropped the second commit.

Related: https://github.com/github/roadmap/issues/528.

Updated 10037078145b2b94b8fbd49c1a7c7c0aa5ac31b3 -> 59ae09ba4838634b486fdf630e0befa0544941cc (pr1394.02 -> pr1394.03, diff):

• reworked this based on feedback from https://github.com/bitcoin-core/secp256k1/pull/1398

Updated 59ae09ba4838634b486fdf630e0befa0544941cc -> 10180bf7cb50ddcdfe0f6c50a85f0b63f42f0708 (pr1394.03 -> pr1394.04, diff):

• dropped the Set environment variables step in favor of direct env: ${{ matrix.env_vars }} assignment

This uses the wrong GCC, namely Clang: https://github.com/bitcoin-core/secp256k1/actions/runs/5841706589/job/15842131976?pr=1394#step:4:43

But we have the same bug on Cirrus CI: :disappointed: https://cirrus-ci.com/task/5276055561306112?logs=test#L17

This is so easy to get wrong on macOS. I suggest we just replace the gcc symlink to point to the actual gcc...

This uses the wrong GCC, namely Clang: bitcoin-core/secp256k1/actions/runs/5841706589/job/15842131976?pr=1394#step:4:43

But we have the same bug on Cirrus CI: 😞 cirrus-ci.com/task/5276055561306112?logs=test#L17

This is so easy to get wrong on macOS. I suggest we just replace the gcc symlink to point to the actual gcc...

Fixed here.

Also see #1402.

I'm currently looking into the failure, trying to figure out the right path. :)

I'm currently looking into the failure, trying to figure out the right path. :)

Doing the same on my macOS :)

ACK 8e54a346d2fa5aeedd6ba5201fcb084c281cf6a7

That's really clean now, I think.

@ Reviewers:

I think what still needs conceptual discussion when it comes to macOS is whether

• we follow the approach I suggested above, namely "moving" some of the existing macOS jobs to self-hosted ARM Linux Cirrus, in order to keep some ARM testing,
• or whether we do something else, e.g., add just ARM QEMU jobs. It's possible to run both the ctimetests (using MSan) and the normal tests in QEMU. The only caveat is that we can run the ctimetests only for gcc, because gcc has no MSan.

Not entirely sure. Having everything on GHA., and not depending on the self-hosted runners, makes our infrastructure simpler. But on the other hand, having the cttimetest on GCC/ARM is probably a good idea. We could at least move as much as possible to GHA, and just have a single Cirrus job that covers that particular cttimetest.

Anyway, all of this should probably happen after this PR here.

cttimetest on GCC/ARM

If this is just a single task running on a single CPU for 5 minutes, it may fit in the "free" Cirrus budget, or someone may be willing to sponsor the Cirrus Credits needed for the task, until it can be moved elsewhere.

cttimetest on GCC/ARM

If this is just a single task running on a single CPU for 5 minutes, it may fit in the "free" Cirrus budget, [...] until it can be moved elsewhere.

Indeed, it should be. It's basically just the compilation. The actual test runs in one second (!) on my machine...

I think my comment was primarily about simplicity and not about the money (one CI provider is simpler than two), but I agree, it's a good observation. So yep, I vote for keeping this single job on Cirrus for now, and we can re-evaluate when/if GitHub will have macOS M1 (and perhaps at that time valgrind-macos will support it.

edit: I still think all of this belongs to a separate PR, we should first get the additional GHA jobs merged here.

ACK 8e54a346d2fa5aeedd6ba5201fcb084c281cf6a7

I vote for keeping this single job on Cirrus for now @jonasnick pointed out to me that we don't run the cttimetests on ARM currently (not before and not after this PR). I lost track of that apparently when I wrote that above... (Sorry, too many platforms and special cases...) To clarify, this means that removing the existing macOS tasks on Cirrus would be a loss only in the sense that run fewer normal tests on ARM. What we currently do for ARM is to run some tests in QEMU. My suggestion is to run more tasks in QEMU to mitigate this loss. We currently run them on Cirrus, but GHA should work equally.

Independently of this, it still makes sense to add a job that runs the cttimetests on ARM, and the only feasible way to do this right now is using Cirrus CI. I'll pick up the work #970 to implement this.

So I guess the Cirrus macos task can be removed, and a comment be added to switch to M1, once it is there? Similar to how it was done in Bitcoin Core:

runs-on: macos-13  # Use M1 once available https://github.com/github/roadmap/issues/528

So I guess the Cirrus macos task can be removed, and a comment be added to switch to M1, once it is there?

Yes, indeed. See #1404.