ci: Add "macOS 12 native arm64" task by hebasto · Pull Request #25160 · bitcoin/bitcoin

hebasto · 2022-05-17T19:06:55Z

The release binaries for Apple Silicon macOS (arm64-apple-darwin) have been available since v23.0.

It seems reasonable to add a task for such a platform to CI. Also it would prevent regressions like #24958.

A native macOS task does not aware of Linux container settings, and it does not use the `depends_built_cache`.

maflcko · 2022-05-18T06:13:35Z

Would this actually prevent #24958? Wouldn't it silently "fail" green?

hebasto · 2022-05-18T06:21:35Z

Would this actually prevent #24958? Wouldn't it silently "fail" green?

bitcoin/ci/test/00_setup_env_mac_native_arm64.sh

Line 11 in f2e07bd

    
           export BITCOIN_CONFIG="--with-gui --with-miniupnpc --with-natpmp --enable-reduce-exports"

It will fail due to

bitcoin/configure.ac

Lines 1751 to 1757 in f7a1e67

    
           dnl Enable NAT-PMP support. 
        
           AC_MSG_CHECKING([whether to build with support for NAT-PMP]) 
        
           if test "$have_natpmp" = "no"; then 
        
             if test "$use_natpmp" = "yes"; then 
        
                AC_MSG_ERROR([NAT-PMP requested but cannot be built. Use --without-natpmp]) 
        
             fi 
        
             AC_MSG_RESULT([no])

Zero-1729 · 2022-05-18T06:28:57Z

Concept ACK

maflcko · 2022-05-18T06:51:28Z

Not sure if it makes sense to run this check on every commit. There is some probability of breaking that is acceptable and unavoidable. I think we should focus on checks that are likely to catch errors often enough to be a net-positive considering the maintenance overhead and CPU overhead of the added CI task.

So if #24958 is the only known bug this prevents, I tend toward NACK, even though the code looks good and correct.

I expect that there are ppl running M1 locally, but if not, this could also be implemented as a nightly task to run outside this repo.

promag

Concept ACK

promag · 2022-05-18T17:45:15Z

ci/test/04_install.sh

  sudo -H pip3 install --upgrade pip
  # shellcheck disable=SC2086
-  IN_GETOPT_BIN="/usr/local/opt/gnu-getopt/bin/getopt" ${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES
+  IN_GETOPT_BIN="$(brew --prefix gnu-getopt)/bin/getopt" ${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES


Nice one 🤓

.cirrus.yml

hebasto · 2022-05-20T08:29:10Z

CPU overhead of the added CI task

I don't think the M1 CPU time is shared with any other task.

maflcko · 2022-05-20T08:34:18Z

According to https://cirrus-ci.org/faq/#are-there-any-limits there is a limit of "12.0 CPUs macOS VM (1 VM)", so it will be shared with the other macos instance?

hebasto · 2022-05-20T08:41:50Z

According to https://cirrus-ci.org/faq/#are-there-any-limits there is a limit of "12.0 CPUs macOS VM (1 VM)", so it will be shared with the other macos instance?

It looks like docs are a bit inconsistent after introducing Apple Silicon support.

They are correct about x86_64:

But M1 limits differ:

DrahtBot · 2022-05-23T22:16:47Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

maflcko · 2022-05-24T06:34:13Z

Ok, looks like it won't affect the limits for now. Though, I am still wondering how many bugs this will catch that are not found by other tasks.

hebasto · 2022-05-24T07:01:49Z

Updated f2e07bd -> 84cfbc7 (pr25160.02 -> pr25160.03, diff):

addressed @promag's comment

I am still wondering how many bugs this will catch that are not found by other tasks.

I hope every bug on the macOS arm64 platform, which has a steadily growing number of users, will be caught before release.

Zero-1729

crACK 84cfbc7

maflcko · 2022-06-21T15:12:03Z

I hope every bug on the macOS arm64 platform, which has a steadily growing number of users, will be caught before release.

It only caught one bug for now, which doesn't seem enough to justify the maintenance burden? Note that in this repo, *bsd, s390x, various linux distros, valgrind, ... aren't tested either.

maflcko · 2022-06-22T05:53:15Z

I guess the 1st+3rd commit could still make sense. Then I could add it to my btc_nightly CI run, if you want.

hebasto · 2022-06-22T08:44:20Z

I guess the 1st+3rd commit could still make sense. Then I could add it to my btc_nightly CI run, if you want.

Done in #25444.

hebasto · 2022-06-22T08:50:43Z

Btw, it seems Cirrus Labs stopped updating macOS images for x86_64. At least, image: monterey-xcode-13.4 or image: monterey-xcode-13.4.1 fail, although, they are available for arm64.

My question remained unanswered.

0bb7a1f ci: Improve naming related to "macOS 12 native x86_64" task (Hennadii Stepanov) 8e017f3 ci, refactor: Add `MACOS_NATIVE_TASK_TEMPLATE` (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#25160 as [suggested](bitcoin/bitcoin#25160 (comment)). ACKs for top commit: MarcoFalke: ACK 0bb7a1f 🚘 Tree-SHA512: d50fe8a51a3364e76d1a5394f718e30bd2994ccdaa4bf73c017c5d81bff00539dcff1cd3879c8b4b6b442b7248b0aa6491489a27c6dd7ec1f3e976aa2a03c730

maflcko · 2022-06-22T12:06:53Z

if amd64 is unsupported, we should probably remove it and just switch to arm64?

0bb7a1f ci: Improve naming related to "macOS 12 native x86_64" task (Hennadii Stepanov) 8e017f3 ci, refactor: Add `MACOS_NATIVE_TASK_TEMPLATE` (Hennadii Stepanov) Pull request description: Split from bitcoin#25160 as [suggested](bitcoin#25160 (comment)). ACKs for top commit: MarcoFalke: ACK 0bb7a1f 🚘 Tree-SHA512: d50fe8a51a3364e76d1a5394f718e30bd2994ccdaa4bf73c017c5d81bff00539dcff1cd3879c8b4b6b442b7248b0aa6491489a27c6dd7ec1f3e976aa2a03c730

…ive" task da16893 ci: Use `macos-ventura-xcode:14.1` image for "macOS native" task (Hennadii Stepanov) 7028365 ci: Make `getopt` path architecture agnostic (Hennadii Stepanov) Pull request description: The "macOS native" CI task always uses the recent OS image. This PR updates it up to the recent macOS release. Cirrus Labs [stopped](#25160 (comment)) updating macOS images for `x86_64`, therefore, an `arm64` image been used. Also `make test-security-check` has been dropped as it ["isn't even expected to pass"](#26386 (comment)) on `arm64` in CI. ACKs for top commit: Sjors: utACK da16893 Tree-SHA512: 36785d33b7f11b3cdbc53bcfbf97d88bf821fad248c825982dd9f8e3413809a4ef11190eaf950e60fdf479b62ff66920c35d9ea42d534723f015742eec7e19b6

ci, refactor: Add MACOS_NATIVE_TASK_TEMPLATE

fa9f45f

A native macOS task does not aware of Linux container settings, and it does not use the `depends_built_cache`.

hebasto force-pushed the 220517-ci branch 3 times, most recently from f0b6f07 to 8c17f08 Compare May 17, 2022 19:22

DrahtBot added the Tests label May 17, 2022

hebasto force-pushed the 220517-ci branch from 8c17f08 to f2e07bd Compare May 17, 2022 19:42

promag reviewed May 18, 2022

View reviewed changes

DrahtBot mentioned this pull request May 23, 2022

build: Make --with-gui=qt6 configure option available on macOS #25191

Closed

hebasto added 2 commits May 24, 2022 08:54

ci: Add "macOS 12 native Apple Silicon" task

168e7f8

ci: Improve naming related to "macOS 12 native x86_64" task

84cfbc7

hebasto force-pushed the 220517-ci branch from f2e07bd to 84cfbc7 Compare May 24, 2022 06:54

hebasto changed the title ~~ci: Add "macOS 12 native Apple Silicon" task~~ ci: Add "macOS 12 native arm64" task May 24, 2022

Zero-1729 reviewed Jun 20, 2022

View reviewed changes

hebasto closed this Jun 21, 2022

hebasto mentioned this pull request Jun 22, 2022

ci: macOS task imrovements #25444

Merged

hebasto mentioned this pull request Oct 25, 2022

ci: Use macos-ventura-xcode:14.1 image for "macOS native" task #26388

Merged

hebasto deleted the 220517-ci branch December 3, 2022 18:19

bitcoin locked and limited conversation to collaborators Dec 3, 2023

Conversation

hebasto commented May 17, 2022

Uh oh!

maflcko commented May 18, 2022

Uh oh!

hebasto commented May 18, 2022

Uh oh!

Zero-1729 commented May 18, 2022

Uh oh!

maflcko commented May 18, 2022

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag May 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hebasto commented May 20, 2022

Uh oh!

maflcko commented May 20, 2022

Uh oh!

hebasto commented May 20, 2022

Uh oh!

DrahtBot commented May 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

maflcko commented May 24, 2022

Uh oh!

hebasto commented May 24, 2022

Uh oh!

Zero-1729 left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jun 21, 2022

Uh oh!

maflcko commented Jun 22, 2022

Uh oh!

hebasto commented Jun 22, 2022

Uh oh!

hebasto commented Jun 22, 2022

Uh oh!

maflcko commented Jun 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DrahtBot commented May 23, 2022 •

edited

Loading