ci: Run "macOS native x86_64" job on GitHub Actions #28187

hebasto · 2023-07-30T19:15:09Z

Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.

If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.

IMPORTANT NOTE. We currently ship macOS release binaries for both architectures: x86_64 and arm64. If this PR gets merged, only x86_64 architecture will be tested on CI, which implies some drawbacks.

However, it has never been the case that our CI tested both architectures simultaneously. And we hope that GitHub Actions will soon host macOS arm64 runners.

Historically, we moved from x86_64 to arm64 in #26388 less than a year ago.

Security concerns:

GITHUB_TOKEN permissions (from the build log in my personal repo):

2023-07-27T07:30:17.8313534Z ##[group]GITHUB_TOKEN Permissions
2023-07-27T07:30:17.8314113Z Contents: read
2023-07-27T07:30:17.8314608Z Metadata: read
2023-07-27T07:30:17.8314957Z Packages: read
2023-07-27T07:30:17.8315233Z ##[endgroup]

Comparison of resources:

Resource	Current, Cirrus CI	Suggested, GitHub Actions
CPU	4	4 **
RAM, GB	8	14

** NOTE: However, docs are mentioning:

3-core CPU (x86_64)

DrahtBot · 2023-07-30T19:15:12Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	MarcoFalke, jarolrod, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28173 (ci: Run Windows native task on GitHub Actions by hebasto)
#21652 (ci: Switch more tasks to self-hosted by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake · 2023-07-30T19:23:59Z

This is replacing the arm64 job with an x86_64 job?

hebasto · 2023-07-30T19:26:03Z

This is replacing the arm64 job with an x86_64 job?

Yes. No arm64 macOS images are available in GitHub Actions for now.

fanquake · 2023-07-30T19:31:01Z

The PR description should probably mention that, as it's a regression in terms of testing. i.e no-longer testing on Apples primarily supported hardware.

hebasto · 2023-07-30T19:36:20Z

The PR description should probably mention that, as it's a regression in terms of testing. i.e no-longer testing on Apples primarily supported hardware.

Done.

maflcko · 2023-07-31T07:18:43Z

Can you link to 10 passing tests in your fork? Otherwise we may run into intermittent issues.

maflcko

lgtm, Should squash?

.cirrus.yml

.github/workflows/ci.yml

fanquake · 2023-07-31T09:05:02Z

~0 Is there any reason we can't use the public beta of the arm64 runners? Have we tested that at-least?

maflcko

~0 Is there any reason we can't use the public beta of the arm64 runners? Have we tested that at-least?

Can you add a link to documentation on how to set that up? With details on the runners, etc. In any case, I don't think it matters, unless you can point to a bug that was found with arm64, but not with amd64. In practice, I expect all bugs found by CI to be found by both macOS arches.

.cirrus.yml

fanquake · 2023-07-31T10:13:37Z

Can you add a link to documentation on how to set that up?

Trying to find that at the moment. Maybe it doesn't exist.

unless you can point to a bug that was found with arm64, but not with amd64.

I think it's at least worth bringing up, as the PR as presented, didn't mention that it drops (direct) testing of one of our release platforms entirely, and presented this change as-if just swapping underlying CI provider.

In terms of differences, there are code paths that are currently exercised in our CI, that no-longer would be after this change, i.e use of macOS ARM intrinsics, so we may be less-likely to detect any breakage if related code is changed, i.e can't sanity check in the CI that detection is working. Maybe that doesn't matter, I do agree that most bugs should be found in either case.

maflcko · 2023-07-31T10:22:28Z

yeah, it should be a trivial two line diff to switch to arm, once and if GitHub supports it. Seems fine to use intel for now, if the alternative is no macOS CI at all.

hebasto · 2023-08-01T08:21:44Z

Can you link to 10 passing tests in your fork? Otherwise we may run into intermittent issues.

All other comments are to be addressed shortly.

maflcko · 2023-08-04T08:36:10Z

All other comments are to be addressed shortly.

Are you still working on this? Apart from a squash and addressing the 4 review comments this is rfm, no?

hebasto · 2023-08-04T08:38:16Z

All other comments are to be addressed shortly.

Are you still working on this?

I am. My apologies for a delay.

hebasto · 2023-08-04T13:36:30Z

Updated e9b7218 -> cc26f0d (pr28187.03 -> pr28187.04):

hebasto · 2023-08-04T13:53:58Z

From IRC:

14:22 <fanquake> Yea the PR description in that PR needs updating to actually explain what’s its doing (dropping support for a release platform from CI), rather than making it look like just swapping infra

The PR description has been updated.

maflcko

lgtm ACK cc26f0d 🖐

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK cc26f0d8c8539094db8d95da9fa46ea1247a0d8c 🖐
hvJ/DOYbi+VSxM5yYqoUvgK/U86A7rpQTGv2wNi1lLHZOushQXVRrU9nznuLgMptqRzWzBwp4zMzw3g0Vz/1Aw==

.github/workflows/ci.yml

maflcko · 2023-08-04T14:50:31Z

re-ACK fe34609 🐺

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK fe34609476b11cdd907c298f7300d424bf556e98  🐺
5foeQdRIfx+XD3h5NuqyHHxTffqWN12HFWwYwYPPXiL9DHdTWEyvDuMH8wTC9SuQbCRhrqLCBC4F3+kGDwKKAw==

maflcko · 2023-08-15T10:34:04Z

.github/workflows/ci.yml

+
+env:
+  DANGER_RUN_CI_ON_HOST: 1
+  TEST_RUNNER_TIMEOUT_FACTOR: 40


Forgot CI_FAILFAST_TEST_LEAVE_DANGLING ? Otherwise, it would be good to see a functional test failure log

Edit: Done in #28278

Also, CCACHE_NOHASHDIR? Or maybe that'd be better moved to ./ci/test/00_setup_env.sh

Edit: Not yet done.

maflcko · 2023-08-15T17:16:50Z

.github/workflows/ci.yml

+  TEST_RUNNER_TIMEOUT_FACTOR: 40
+
+jobs:
+  macos-native-x86_64:


Maybe add a comment linking to github/roadmap#528 to say that this should be used, when available?

Done in #28278

achow101 · 2023-08-15T21:02:55Z

ACK 9658d0d

maflcko · 2023-08-15T17:22:40Z

.github/workflows/ci.yml

+    env:
+      MAKEJOBS: '-j4'
+      CI_USE_APT_INSTALL: 'no'
+      PACKAGE_MANAGER_INSTALL: 'echo'  # Nothing to do


nit: I think this can be removed?

Fixed in #28278

maflcko · 2023-08-15T21:40:35Z

.github/workflows/ci.yml

+
+    env:
+      MAKEJOBS: '-j4'
+      CI_USE_APT_INSTALL: 'no'


unrelated: Maybe remove this and replace it with a check on CI_OS_NAME?

Fixed in #28278

maflcko · 2023-08-15T21:41:14Z

.github/workflows/ci.yml

+    # No need to run on the read-only mirror, unless it is a PR.
+    if: github.repository != 'bitcoin-core/gui' || github.event_name == 'pull_request'
+
+    timeout-minutes: 120


Should also abort on a force push, to avoid wasting CPU, no?

GHActions have no such a feature by default. It should be a separated workflow that processes pull_request.synchronize event and terminates other workflows. However, I'm not sure about the exact implementation for now.

So if someone (honestly) pushes to a pull request a few times in a few minutes, it will block CI progress on all other pulls, assuming a few more tasks are run on GHA and the "free" limit is reached?

Should also abort on a force push, to avoid wasting CPU, no?

Done in #28282.

maflcko · 2023-08-16T06:47:03Z

.github/workflows/ci.yml

+    timeout-minutes: 120
+
+    env:
+      MAKEJOBS: '-j4'


Any reason to reduce this to 4 when it previously was 10?

Fixed in #28278

maflcko · 2023-08-16T06:59:22Z

.github/workflows/ci.yml

+        with:
+          path: ${{ env.CCACHE_DIR }}
+          key: ${{ github.job }}-ccache-cache-${{ github.run_id }}
+          restore-keys: ${{ github.job }}-ccache-cache


Looks like each pull request will rewrite the ccache and thus the hit rate will be ~0% overall?

Would be better to get the cache from the own pull only, or master.

Yea, looks like caching doesn't work at all here?

Yea, looks like caching doesn't work at all here?

ci: ccache does not work for macOS cross-compiling builds #21552

build: Fix ccache behavior when cross-compiling for darwin hosts #24620

If it doesn't work, why was any of this added to the CI template?

I think it works when there are no pull requests. It should be possible to fix this in a follow-up.

If it doesn't work, why was any of this added to the CI template?

~~Ccache regression, introduced in #19683, should be fixed. It is not related to the CI template that is correct.~~

How is this related. This is the "macOS native", not "macOS-cross" task.

How is this related. This is the "macOS native", not "macOS-cross" task.

My bad. Sorry for the noise.

Yea, looks like caching doesn't work at all here?

It works.

I think it works when there are no pull requests. It should be possible to fix this in a follow-up.

Yes, pull requests should not create their own caches. Going to address this issue shortly.

maflcko · 2023-08-17T09:56:16Z

Looks like it is now even harder to re-run tasks, if there is an intermittent network issue (for example if GH or GHA is down, which happens ~daily).

At least, all I can do is download the log, whereas on Cirrus, I can re-run my own pull.

fanquake · 2023-08-17T09:57:53Z

Looks like it is now even harder to re-run tasks,

So there is no way for developers to re-run there own PRs at all, with GitHub Actions? Other than I assume force pushing?

hebasto · 2023-08-17T10:04:18Z

Looks like it is now even harder to re-run tasks,

So there is no way for developers to re-run there own PRs at all, with GitHub Actions? Other than I assume force pushing?

Docs are not super clear about that: https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs.

maflcko · 2023-08-17T10:07:14Z

That page says "People with write permissions to a repository can re-run workflows in the repository." Maybe someone else without write access can confirm whether they can re-run on Cirrus?

fanquake · 2023-08-17T10:13:46Z

That page says "People with write permissions to a repository can re-run workflows in the repository."

Hopefully not. If this really is the case, that's another downside to GH Actions, and going to be a worse developer experience (we can't give write access to all contributors just to re-run CI). Would have been good if these things had been explained in the PR description, or discussion.

can confirm whether they can re-run on Cirrus?

Contributors can definitely re-run Cirrus jobs. Looks like they cannot re-run Github Actions workflows.

dergoegge · 2023-08-17T10:24:19Z

Maybe someone else without write access can confirm whether they can re-run on Cirrus?

I can't re-run GH actions tasks but am able to re-run cirrus jobs.

hebasto · 2023-08-17T10:37:50Z

Maybe someone else without write access can confirm whether they can re-run on Cirrus?

I can't re-run GH actions tasks but am able to re-run cirrus jobs.

Does closing a PR and following re-opening it work?

dergoegge · 2023-08-17T10:40:26Z

Does closing a PR and following re-opening it work?

No, that just cancels all jobs and the gha one just continues running

maflcko · 2023-08-17T12:56:55Z

Also, it looks like CI tasks currently need approval from a maintainer to run at all from non-members. See https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

…rocesses `github.ref` at a time 0080b56 ci: Ensure that only a single workflow processes `github.ref` at a time (Hennadii Stepanov) Pull request description: This PR ensures that only a single workflow processes any push or pull request at a time. A new push will be queued (including the master branch). For a new pull request update, the previous in-progress one will be cancelled. Address bitcoin/bitcoin#28187 (comment). ACKs for top commit: dergoegge: utACK 0080b56 Tree-SHA512: d03f1678c93c817c1c3be5e75171bfe85d494f99a9aab7113c9438e0c820e950edd5c10199cccd54868e6dc50217bb3aa5601b23bc88ad4927c001031e917513

hebasto · 2023-08-17T13:35:27Z

Also, it looks like CI tasks currently need approval from a maintainer to run at all from non-members. See docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

It is a configurable setting.

achow101 · 2023-08-17T14:00:12Z

Also, it looks like CI tasks currently need approval from a maintainer to run at all from non-members. See docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

It is a configurable setting.

It's currently set to "Require approval for first-time contributors". Could change that to "Require approval for first-time contributors who are new to GitHub". There's no "off" option.

… `github.ref` at a time 0080b56 ci: Ensure that only a single workflow processes `github.ref` at a time (Hennadii Stepanov) Pull request description: This PR ensures that only a single workflow processes any push or pull request at a time. A new push will be queued (including the master branch). For a new pull request update, the previous in-progress one will be cancelled. Address bitcoin#28187 (comment). ACKs for top commit: dergoegge: utACK 0080b56 Tree-SHA512: d03f1678c93c817c1c3be5e75171bfe85d494f99a9aab7113c9438e0c820e950edd5c10199cccd54868e6dc50217bb3aa5601b23bc88ad4927c001031e917513

… in GitHub Actions 241d6ca ci: Disable cache save for pull requests in GitHub Actions (Hennadii Stepanov) Pull request description: This PR disable cache save for pull requests in GitHub Actions. Otherwise, multiple pull requests fill GitHub Actions cache quota shortly. See a discussion [here](bitcoin/bitcoin#28187 (comment)). --- **NOTE** for the maintainers with "owner" permissions. This PR needs the `actions/cache/restore@*` and `actions/cache/save@*` acrions to be explicitly allowed in the repository's Actions permissions. ACKs for top commit: MarcoFalke: lgtm ACK 241d6ca Tree-SHA512: a7786c7ec99bfa6991bf6ae08fd7ed3546e8c5d083a1b2bae7638f6f31e77fdf2cf4fc69d85834faf87f98db1f4a82026ce1dc5fc1bc6650e8bf1c09bf7e90f5

…ub Actions 241d6ca ci: Disable cache save for pull requests in GitHub Actions (Hennadii Stepanov) Pull request description: This PR disable cache save for pull requests in GitHub Actions. Otherwise, multiple pull requests fill GitHub Actions cache quota shortly. See a discussion [here](bitcoin#28187 (comment)). --- **NOTE** for the maintainers with "owner" permissions. This PR needs the `actions/cache/restore@*` and `actions/cache/save@*` acrions to be explicitly allowed in the repository's Actions permissions. ACKs for top commit: MarcoFalke: lgtm ACK 241d6ca Tree-SHA512: a7786c7ec99bfa6991bf6ae08fd7ed3546e8c5d083a1b2bae7638f6f31e77fdf2cf4fc69d85834faf87f98db1f4a82026ce1dc5fc1bc6650e8bf1c09bf7e90f5

DrahtBot added the Tests label Jul 30, 2023

hebasto mentioned this pull request Jul 30, 2023

ci: Future of macOS and Windows MSVC CI tasks #28098

Closed

hebasto force-pushed the 230730-macos branch from 0e71de4 to 8767c61 Compare July 30, 2023 19:20

DrahtBot added the CI failed label Jul 30, 2023

DrahtBot removed the CI failed label Jul 30, 2023

hebasto force-pushed the 230730-macos branch from 8767c61 to e9b7218 Compare July 30, 2023 21:12

DrahtBot mentioned this pull request Jul 31, 2023

ci: Run Windows native task on GitHub Actions #28173

Merged

maflcko approved these changes Jul 31, 2023

View reviewed changes

.cirrus.yml Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

.github/workflows/ci.yml Outdated Show resolved Hide resolved

.github/workflows/ci.yml Outdated Show resolved Hide resolved

maflcko approved these changes Jul 31, 2023

View reviewed changes

.cirrus.yml Show resolved Hide resolved

DrahtBot mentioned this pull request Jul 31, 2023

ci: Switch remaining Linux tasks to self-hosted #21652

Merged

hebasto force-pushed the 230730-macos branch from e9b7218 to cc26f0d Compare August 4, 2023 13:18

maflcko approved these changes Aug 4, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

hebasto force-pushed the 230730-macos branch from cc26f0d to fe34609 Compare August 4, 2023 14:48

maflcko added this to the 26.0 milestone Aug 4, 2023

DrahtBot mentioned this pull request Aug 4, 2023

ci: Move tidy to persistent worker #28214

Merged

maflcko reviewed Aug 15, 2023

View reviewed changes

DrahtBot removed the request for review from achow101 August 15, 2023 21:03

achow101 merged commit b97b050 into bitcoin:master Aug 15, 2023

maflcko reviewed Aug 15, 2023

View reviewed changes

maflcko reviewed Aug 16, 2023

View reviewed changes

hebasto deleted the 230730-macos branch August 16, 2023 07:44

hebasto mentioned this pull request Aug 16, 2023

build: Fix ccache behavior when cross-compiling for darwin hosts #24620

Closed

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 17, 2023

Merge bitcoin#28187: ci: Run "macOS native x86_64" job on GitHub Actions

f081dc9

hebasto mentioned this pull request Aug 17, 2023

ci: Refactor: Remove CI_USE_APT_INSTALL #28278

Merged

hebasto mentioned this pull request Aug 17, 2023

ci: Ensure that only a single workflow processes github.ref at a time #28282

Merged

hebasto mentioned this pull request Aug 18, 2023

ci: Disable cache save for pull requests in GitHub Actions #28292

Merged

bitcoin locked and limited conversation to collaborators Aug 16, 2024

ci: Run "macOS native x86_64" job on GitHub Actions #28187

ci: Run "macOS native x86_64" job on GitHub Actions #28187

Uh oh!

Conversation

hebasto commented Jul 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

fanquake commented Jul 30, 2023

Uh oh!

hebasto commented Jul 30, 2023

Uh oh!

fanquake commented Jul 30, 2023

Uh oh!

hebasto commented Jul 30, 2023

Uh oh!

maflcko commented Jul 31, 2023

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fanquake commented Jul 31, 2023

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fanquake commented Jul 31, 2023

Uh oh!

maflcko commented Jul 31, 2023

Uh oh!

hebasto commented Aug 1, 2023

Uh oh!

maflcko commented Aug 4, 2023

Uh oh!

hebasto commented Aug 4, 2023

Uh oh!

hebasto commented Aug 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Aug 4, 2023

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maflcko commented Aug 4, 2023

Uh oh!

maflcko Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achow101 commented Aug 15, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hebasto commented Jul 30, 2023 •

edited

Loading

DrahtBot commented Jul 30, 2023 •

edited

Loading

hebasto commented Aug 4, 2023 •

edited

Loading

maflcko Aug 15, 2023 •

edited

Loading

maflcko Aug 15, 2023 •

edited

Loading

hebasto Aug 16, 2023 •

edited

Loading

fanquake commented Aug 17, 2023 •

edited

Loading