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

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:230730-macos changing 3 files +62 −20
  1. hebasto commented at 7:15 pm on July 30, 2023: member

    From #28098:

    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):

    02023-07-27T07:30:17.8313534Z ##[group]GITHUB_TOKEN Permissions
12023-07-27T07:30:17.8314113Z Contents: read
22023-07-27T07:30:17.8314608Z Metadata: read
32023-07-27T07:30:17.8314957Z Packages: read
42023-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)

  2. DrahtBot commented at 7:15 pm on July 30, 2023: contributor

    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.

  3. DrahtBot added the label Tests on Jul 30, 2023
  4. hebasto force-pushed on Jul 30, 2023
  5. DrahtBot added the label CI failed on Jul 30, 2023
  6. fanquake commented at 7:23 pm on July 30, 2023: member
    This is replacing the arm64 job with an x86_64 job?
  7. hebasto commented at 7:26 pm on July 30, 2023: member

    This is replacing the arm64 job with an x86_64 job?

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

  8. fanquake commented at 7:31 pm on July 30, 2023: member
    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.
  9. hebasto commented at 7:36 pm on July 30, 2023: member

    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.

  10. DrahtBot removed the label CI failed on Jul 30, 2023
  11. hebasto force-pushed on Jul 30, 2023
  12. maflcko commented at 7:18 am on July 31, 2023: member
    Can you link to 10 passing tests in your fork? Otherwise we may run into intermittent issues.
  13. in .cirrus.yml:353 in e9b7218520 outdated 
    337-  << : *MAIN_TEMPLATE
338-  env:
339-    << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
340-    CI_USE_APT_INSTALL: "no"
341-    PACKAGE_MANAGER_INSTALL: "echo"  # Nothing to do
342-    FILE_ENV: "./ci/test/00_setup_env_mac_native_arm64.sh"
    maflcko commented at 8:34 am on July 31, 2023:
    Can delete the arm one now?
    maflcko commented at 9:14 am on July 31, 2023:
    Or just remove the arch from the filename? The included export HOST should already take care of it and avoid a filename ping-pong.
    hebasto commented at 9:23 am on August 4, 2023:

    I hope that at some point, GitHub Actions will host arm64 macOS runners, enabling us to test both architectures.

    Maybe leave both files for now?

    maflcko commented at 9:35 am on August 4, 2023:
    Does apple still ship Intel silicon for new macOS machines? Regardless, I don’t really see a benefit in running the same task twice when there is no reason to believe it will find more bugs or is even useful in the long term.
    hebasto commented at 1:37 pm on August 4, 2023:
    Updated.
  14. in .github/workflows/ci.yml:1 in e9b7218520
    maflcko commented at 8:36 am on July 31, 2023:

    I think you forgot to skip on the monotree fork?

    0  skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == ""  # No need to run on the read-only mirror, unless it is a PR. https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution
    hebasto commented at 1:37 pm on August 4, 2023:
    Thanks! Updated.
  15. in .github/workflows/ci.yml:40 in e9b7218520 outdated 
    35+        run: brew install automake boost pkg-config miniupnpc libnatpmp zeromq qt@5 qrencode ccache gnu-getopt
36+
37+      - name: Ccache cache
38+        uses: actions/cache@v3
39+        with:
40+          path: /Users/runner/work/bitcoin/bitcoin/ci/scratch/.ccache
    maflcko commented at 8:38 am on July 31, 2023:
    Seems fragile. It may be better to set a hard-coded path, like before.
    hebasto commented at 1:37 pm on August 4, 2023:
    Reworked.
  16. in .github/workflows/ci.yml:42 in e9b7218520 outdated 
    37+      - name: Ccache cache
38+        uses: actions/cache@v3
39+        with:
40+          path: /Users/runner/work/bitcoin/bitcoin/ci/scratch/.ccache
41+          key: ${{ runner.os }}-${{ runner.arch }}-ccache-cache-${{ github.run_id }}
42+          restore-keys: ${{ runner.os }}-${{ runner.arch }}-ccache-cache
    maflcko commented at 8:40 am on July 31, 2023:
    any reason to not just use the task name, like before?
    hebasto commented at 8:58 am on August 1, 2023:

    any reason to not just use the task name, like before?

    GHA’s job (an analogue of Cirrus’s task) context has no “name”-like property.

    maflcko commented at 9:03 am on August 1, 2023:

    Ok. You are setting one in the line above: name: macOS 13 native, x86_64 [no depends, sqlite only, gui]

    But if that is not possible to use, that is fine. Though, it would be good to explain why both key and restore-key are set and why github.run_id is used?

    hebasto commented at 9:30 am on August 1, 2023:

    Ok. You are setting one in the line above: name: macOS 13 native, x86_64 [no depends, sqlite only, gui]

    But if that is not possible to use, that is fine.

    Well, it is still possible to get github.job property that has value macos-native from https://github.com/bitcoin/bitcoin/blob/e9b72185205812df51082b42fb8c25cbf53cbc03/.github/workflows/ci.yml#L14-L15

    Though, it would be good to explain why both key and restore-key are set and why github.run_id is used?

    This use case is described in actions/cache docs. TL;DR: it forces to update the cache regardless of its successful restoring.

    maflcko commented at 9:37 am on August 1, 2023:
    Mind linking to the doc in this line then? Otherwise all reviewers and future readers will have to wade through the docs themselves?
    maflcko commented at 9:42 am on August 1, 2023:

    Well, it is still possible to get github.job property that has value macos-native from

    "macos-native" as key seems fine (for ccache), no? (Any way is fine, though)

    hebasto commented at 1:38 pm on August 4, 2023:
    Updated.
  17. maflcko approved
  18. maflcko commented at 8:40 am on July 31, 2023: member
    lgtm, Should squash?
  19. fanquake commented at 9:05 am on July 31, 2023: member
    ~0 Is there any reason we can’t use the public beta of the arm64 runners? Have we tested that at-least?
  20. maflcko approved
  21. maflcko commented at 9:16 am on July 31, 2023: member

    ~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.

  22. fanquake commented at 10:13 am on July 31, 2023: member

    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.

  23. maflcko commented at 10:22 am on July 31, 2023: member
    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.
  25. maflcko commented at 8:36 am on August 4, 2023: member

    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?

  26. hebasto commented at 8:38 am on August 4, 2023: member

    All other comments are to be addressed shortly.

    Are you still working on this?

    I am. My apologies for a delay.

  27. hebasto force-pushed on Aug 4, 2023
  28. hebasto commented at 1:36 pm on August 4, 2023: member

    Updated e9b72185205812df51082b42fb8c25cbf53cbc03 -> cc26f0d8c8539094db8d95da9fa46ea1247a0d8c (pr28187.03 -> pr28187.04):

  29. hebasto commented at 1:53 pm on August 4, 2023: member

    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.

  30. in .github/workflows/ci.yml:38 in cc26f0d8c8 outdated 
    33+
34+      - name: Clang version
35+        run: clang --version
36+
37+      - name: Install Homebrew packages
38+        run: brew install automake boost pkg-config miniupnpc libnatpmp zeromq qt@5 qrencode ccache gnu-getopt
    maflcko commented at 1:55 pm on August 4, 2023:
    Any reason to reorder, remove libevent and libtool?
    hebasto commented at 2:49 pm on August 4, 2023:
    Restored.
  31. in .github/workflows/ci.yml:25 in cc26f0d8c8 outdated 
    14+jobs:
15+  macos-native-x86_64:
16+    name: macOS 13 native, x86_64 [no depends, sqlite only, gui]
17+    # Use latest image, but hardcode version to avoid silent upgrades (and breaks).
18+    # See: https://github.com/actions/runner-images#available-images.
19+    runs-on: macos-13
    maflcko commented at 1:56 pm on August 4, 2023:
    nit: forgot to set the timeout? What is the default?
    hebasto commented at 2:43 pm on August 4, 2023:

    Usage limits:

    Each job in a workflow can run for up to 6 hours of execution time.

    maflcko commented at 2:45 pm on August 4, 2023:
    May be better to reduce it to two hours to catch timeout issues earlier? But no strong opinion, just a nit.
    hebasto commented at 2:49 pm on August 4, 2023:
    Done.
  32. maflcko approved
  33. maflcko commented at 2:00 pm on August 4, 2023: member

    lgtm ACK cc26f0d8c8539094db8d95da9fa46ea1247a0d8c 🖐

    Signature:

    0untrusted 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}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: lgtm ACK cc26f0d8c8539094db8d95da9fa46ea1247a0d8c 🖐
3hvJ/DOYbi+VSxM5yYqoUvgK/U86A7rpQTGv2wNi1lLHZOushQXVRrU9nznuLgMptqRzWzBwp4zMzw3g0Vz/1Aw==
  34. hebasto force-pushed on Aug 4, 2023
  35. maflcko commented at 2:50 pm on August 4, 2023: member

    re-ACK fe34609476b11cdd907c298f7300d424bf556e98 🐺

    Signature:

    0untrusted 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}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: re-ACK fe34609476b11cdd907c298f7300d424bf556e98  🐺
35foeQdRIfx+XD3h5NuqyHHxTffqWN12HFWwYwYPPXiL9DHdTWEyvDuMH8wTC9SuQbCRhrqLCBC4F3+kGDwKKAw==
  36. maflcko added this to the milestone 26.0 on Aug 4, 2023
  37. maflcko commented at 8:41 am on August 8, 2023: member
    Anything left to do here, for a required CI-only pull request with review?
  38. in .github/workflows/ci.yml:8 in fe34609476 outdated 
    0@@ -0,0 +1,53 @@
1+# Copyright (c) 2023 The Bitcoin Core developers
2+# Distributed under the MIT software license, see the accompanying
3+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+name: bitcoin-core-ci
6+run-name: Bitcoin Core CI
7+# See: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onpushpull_requestpull_request_targetpathspaths-ignore.
8+on: [pull_request, push]
    jarolrod commented at 3:03 am on August 9, 2023:
    I’m catching up on the syntax for Github Actions, but wouldn’t this run this CI task twice when a Pull request is opened? That seems to be the case when opening this against my fork: https://github.com/jarolrod/bitcoin/pull/2
    maflcko commented at 8:13 am on August 9, 2023:
    Why would you open a pull request against your own fork? This is never done in this repo, so I don’t see a reason why this should be optimized?
    hebasto commented at 9:50 am on August 9, 2023:

    That seems to be the case when opening this against my fork: jarolrod#2

    That’s correct. But is this really the case we want to care about?

  39. maflcko requested review from fanquake on Aug 9, 2023
  40. maflcko requested review from achow101 on Aug 9, 2023
  41. ci: Run "macOS native x86_64" job on GitHub Actions 
    Also, the "macOS native arm64" task has been removed from Cirrus CI.
    9658d0dc17
  42. in ci/test/00_setup_env_mac_native.sh:3 in fe34609476 outdated 
    0@@ -1,18 +1,18 @@
1 #!/usr/bin/env bash
2 #
3-# Copyright (c) 2019-2022 The Bitcoin Core developers
4+# Copyright (c) 2023 The Bitcoin Core developers
    maflcko commented at 8:17 am on August 9, 2023:

    Obviously doesn’t matter for this file, but if you retouch, could make this

    0# Copyright (c) 2019-present The Bitcoin Core developers

    to avoid having to touch it ever again

  43. hebasto force-pushed on Aug 9, 2023
  44. hebasto commented at 10:03 am on August 9, 2023: member

    Updated fe34609476b11cdd907c298f7300d424bf556e98 -> 9658d0dc17c270138523c41a982425e276b24271 (pr28187.05 -> pr28187.06, diff):

  45. maflcko commented at 11:34 am on August 9, 2023: member

    re-ACK 9658d0dc17c270138523c41a982425e276b24271 🏂

    Signature:

    0untrusted 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}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: re-ACK 9658d0dc17c270138523c41a982425e276b24271 🏂
3JyEXQpG79qkYwPubRvV8GUf+XEeNrSnGIhjkewntd85ldpm4VDzhF/VML5xHaxJRvECt20EBNKw475OQscktBg==
  46. hebasto commented at 10:34 am on August 10, 2023: member

    Anything left to do here, for a required CI-only pull request with review?

    I believe this PR is ready to merge unless @fanquake has more comment regarding its description.

    However, it requires adjusting “Actions permissions” and “Workflow permissions” in the repo in a similar way as it was done in https://github.com/bitcoin-core/secp256k1/pull/1389. @achow101 Would you mind taking care of that, please?

  47. in .cirrus.yml:339 in 9658d0dc17 
    333@@ -334,20 +334,3 @@ task:
334   env:
335     MACOS_SDK: "Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers"
336     << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
337-
338-task:
339-  name: 'macOS 13 native arm64 [gui, sqlite only] [no depends]'
    jarolrod commented at 8:14 pm on August 10, 2023:

    nit: if there is any opinion on the pr/commit title not being sufficient, you could make it something like the following (and add a body):

    0ci: Run "macOS native" job on Github Actions, revert back to x86_64
1
2Updates the macOS native ci job to run on Github Actions which will
3allow us to stay on a free plan for the job. Additionally, temporarily
4revert back to running on a x86_64 architecture until Github Actions
5has a arm64 (Apple Silicon) runner; which is preferable to no macOS native
6ci job.
  48. jarolrod commented at 8:17 pm on August 10, 2023: member

    ACK 9658d0dc17c270138523c41a982425e276b24271

    As long as permissions are looked over and activation of Github Workflows and Actions is safe here: https://docs.github.com/en/rest/actions/permissions?apiVersion=2022-11-28

  49. maflcko commented at 4:12 pm on August 14, 2023: member
    @achow101 @fanquake Anything left to be done here? The “macOS 13 native arm64” Cirrus task will stop working in two weeks, so it would be good to do something about it. If you disagree with this change, it would be good to leave a comment.
  50. in .github/workflows/ci.yml:18 in 9658d0dc17 
    13+    tags-ignore:
14+      - '**'
15+
16+env:
17+  DANGER_RUN_CI_ON_HOST: 1
18+  TEST_RUNNER_TIMEOUT_FACTOR: 40
    maflcko commented at 10:34 am on August 15, 2023:

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

    Edit: Done in https://github.com/bitcoin/bitcoin/pull/28278

    maflcko commented at 1:34 pm on August 15, 2023:

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

    Edit: Not yet done.

  51. in .github/workflows/ci.yml:21 in 9658d0dc17 
    16+env:
17+  DANGER_RUN_CI_ON_HOST: 1
18+  TEST_RUNNER_TIMEOUT_FACTOR: 40
19+
20+jobs:
21+  macos-native-x86_64:
    maflcko commented at 5:16 pm on August 15, 2023:
    Maybe add a comment linking to https://github.com/github/roadmap/issues/528 to say that this should be used, when available?
    maflcko commented at 11:56 am on August 17, 2023:
    Done in https://github.com/bitcoin/bitcoin/pull/28278
  52. in .github/workflows/ci.yml:35 in 9658d0dc17 
    30+    timeout-minutes: 120
31+
32+    env:
33+      MAKEJOBS: '-j4'
34+      CI_USE_APT_INSTALL: 'no'
35+      PACKAGE_MANAGER_INSTALL: 'echo'  # Nothing to do
    maflcko commented at 5:22 pm on August 15, 2023:
    nit: I think this can be removed?
    maflcko commented at 12:43 pm on August 16, 2023:
    Fixed in https://github.com/bitcoin/bitcoin/pull/28278
  53. achow101 commented at 9:02 pm on August 15, 2023: member
    ACK 9658d0dc17c270138523c41a982425e276b24271
  54. DrahtBot removed review request from achow101 on Aug 15, 2023
  55. achow101 merged this on Aug 15, 2023
  56. achow101 closed this on Aug 15, 2023
  57. in .github/workflows/ci.yml:34 in 9658d0dc17 
    29+
30+    timeout-minutes: 120
31+
32+    env:
33+      MAKEJOBS: '-j4'
34+      CI_USE_APT_INSTALL: 'no'
    maflcko commented at 9:40 pm on August 15, 2023:
    unrelated: Maybe remove this and replace it with a check on CI_OS_NAME?
    maflcko commented at 12:43 pm on August 16, 2023:
    Fixed in https://github.com/bitcoin/bitcoin/pull/28278
  58. in .github/workflows/ci.yml:30 in 9658d0dc17 
    25+    runs-on: macos-13
26+
27+    # No need to run on the read-only mirror, unless it is a PR.
28+    if: github.repository != 'bitcoin-core/gui' || github.event_name == 'pull_request'
29+
30+    timeout-minutes: 120
    maflcko commented at 9:41 pm on August 15, 2023:
    Should also abort on a force push, to avoid wasting CPU, no?
    hebasto commented at 9:32 am on August 17, 2023:
    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.
    maflcko commented at 9:37 am on August 17, 2023:
    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?
    hebasto commented at 10:24 am on August 17, 2023:

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

    Done in #28282.

  59. in .github/workflows/ci.yml:33 in 9658d0dc17 
    28+    if: github.repository != 'bitcoin-core/gui' || github.event_name == 'pull_request'
29+
30+    timeout-minutes: 120
31+
32+    env:
33+      MAKEJOBS: '-j4'
    maflcko commented at 6:47 am on August 16, 2023:
    Any reason to reduce this to 4 when it previously was 10?
    maflcko commented at 12:42 pm on August 16, 2023:
    Fixed in https://github.com/bitcoin/bitcoin/pull/28278
  60. in .github/workflows/ci.yml:56 in 9658d0dc17 
    51+      - name: Ccache cache
52+        uses: actions/cache@v3
53+        with:
54+          path: ${{ env.CCACHE_DIR }}
55+          key: ${{ github.job }}-ccache-cache-${{ github.run_id }}
56+          restore-keys: ${{ github.job }}-ccache-cache
    maflcko commented at 6:59 am on August 16, 2023:

    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.

    fanquake commented at 8:39 am on August 16, 2023:
    Yea, looks like caching doesn’t work at all here?
    hebasto commented at 8:41 am on August 16, 2023:

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

    fanquake commented at 8:42 am on August 16, 2023:
    If it doesn’t work, why was any of this added to the CI template?
    maflcko commented at 8:44 am on August 16, 2023:
    I think it works when there are no pull requests. It should be possible to fix this in a follow-up.
    hebasto commented at 8:45 am on August 16, 2023:

    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.

    maflcko commented at 9:01 am on August 16, 2023:

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

    hebasto commented at 9:02 am on August 16, 2023:

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

    My bad. Sorry for the noise.

    hebasto commented at 9:53 am on August 17, 2023:

    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.

  61. hebasto deleted the branch on Aug 16, 2023
  62. sidhujag referenced this in commit f081dc9f14 on Aug 17, 2023
  63. maflcko commented at 9:56 am on August 17, 2023: member

    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.

    Screen Shot 2023-08-17 at 11 54 48

  64. fanquake commented at 9:57 am on August 17, 2023: member

    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?

  65. hebasto commented at 10:04 am on August 17, 2023: member

    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.

  66. maflcko commented at 10:07 am on August 17, 2023: member
    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?
  67. fanquake commented at 10:13 am on August 17, 2023: member

    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.

  68. dergoegge commented at 10:24 am on August 17, 2023: member

    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.

  69. hebasto commented at 10:37 am on August 17, 2023: member

    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?

  70. dergoegge commented at 10:40 am on August 17, 2023: member

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

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

  71. maflcko commented at 12:56 pm on August 17, 2023: member
    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
  72. fanquake referenced this in commit de197c19e4 on Aug 17, 2023
  73. hebasto commented at 1:35 pm on August 17, 2023: member

    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.

  74. achow101 commented at 2:00 pm on August 17, 2023: member

    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.

  75. sidhujag referenced this in commit fd298243ca on Aug 17, 2023
  76. fanquake referenced this in commit ded6873340 on Aug 21, 2023
  77. Frank-GER referenced this in commit 18bbd47ffc on Sep 8, 2023
  78. bitcoin locked this on Aug 16, 2024


hebasto DrahtBot fanquake maflcko jarolrod achow101 dergoegge


fanquake

Labels
Tests

Milestone
26.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-03 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me