ci: Run "Windows (VS 2022)" job on GitHub Actions #1389

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:230728-actions changing 1 files +39 −0
  1. hebasto commented at 2:02 PM on July 28, 2023: member

    This PR solves one item in #1392.

    In response to upcoming limiting free usage of Cirrus CI, suggesting to move (partially?) CI tasks/jobs from Cirrus CI to GitHub Actions (GHA).

    Here is example from my personal repo: https://github.com/hebasto/secp256k1/actions/runs/5806269046.

    For security concerns, see:

    I'm suggesting the repository "Actions permissions" as follows:

    image

    image

    See build logs in my personal repo: https://github.com/hebasto/secp256k1/actions/runs/5692587475.

  2. maflcko commented at 3:04 PM on July 28, 2023: contributor

    Same nit here: Could delete the Windows task on Cirrus in this pull, or is there a reason why it should be kept after this is merged?

  3. hebasto commented at 3:12 PM on July 28, 2023: member

    Could delete the Windows task on Cirrus in this pull...

    Deleted.

  4. hebasto force-pushed on Aug 3, 2023
  5. hebasto commented at 1:23 PM on August 3, 2023: member

    Rebased on top of the merged #1290.

  6. real-or-random added the label ci on Aug 8, 2023
  7. in .github/workflows/ci.yml:27 in 72b8d875b2 outdated 
      22 | +
  23 | +      - name: Generate buildsystem
  24 | +        run: cmake -E env CFLAGS="/WX" cmake -B build -A x64 -DSECP256K1_ENABLE_MODULE_RECOVERY=ON -DSECP256K1_BUILD_EXAMPLES=ON -DBUILD_SHARED_LIBS=${{ matrix.build_shared_libs }}
  25 | +
  26 | +      - name: Build
  27 | +        run: cmake --build build --config RelWithDebInfo -- /p:UseMultiToolTask=true /p:CL_MPcount=3
    real-or-random commented at 5:11 PM on August 8, 2023:

    optional nit: The 3 could be computed as n + 1, where n is read from the system automatically, same below.

    hebasto commented at 7:59 AM on August 9, 2023:

    Done.

    In the MSBuild option list, it has been replaced with the high level -maxCpuCount switch.

  8. real-or-random commented at 5:17 PM on August 8, 2023: contributor

    Approach ACK

    Is there a better way to test PRs that change workflows (other than deploying them in your fork?) Or is the problem simply that we haven't enabled GitHub Actions at all? AFAIU, on [pull_request] should run the workflow from the merge commit, see for example https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/ . Is this correct?

  9. hebasto commented at 5:34 PM on August 8, 2023: member

    Is there a better way to test PRs that change workflows (other than deploying them in your fork?) Or is the problem simply that we haven't enabled GitHub Actions at all?

    Enabled GitHub Actions in this repo should help.

    AFAIU, on [pull_request] should run the workflow from the merge commit, see for example github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows . Is this correct?

    According to the docs, for on.pull_request:

    a workflow only runs when a pull_request event's activity type is opened, synchronize, or reopened.

  10. hebasto force-pushed on Aug 9, 2023
  11. hebasto commented at 7:51 AM on August 9, 2023: member

    Updated 72b8d875b21bfbd50dd95ec625d95dddcea37614 -> 5a3d45baa00057d6e27f2119ccdeea80108373f1 (pr1389.03 -> pr1389.04, diff):

  12. maflcko approved
  13. maflcko commented at 7:58 AM on August 9, 2023: contributor

    lgtm ACK

  14. real-or-random closed this on Aug 9, 2023
  15. real-or-random reopened this on Aug 9, 2023
  16. in .github/workflows/ci.yml:12 in 5a3d45baa0 outdated 
       7 | +    tags-ignore:
   8 | +      - '**'
   9 | +
  10 | +env:
  11 | +  SECP256K1_BENCH_ITERS: 2
  12 | +  SECP256K1_TEST_ITERS: 16
    real-or-random commented at 10:55 AM on August 9, 2023:

    Any reason not to keep the default of 64 for TEST_ITERS? We typically set lower values only in special environments such as QEMU.

    hebasto commented at 11:20 AM on August 9, 2023:

    I cannot recall what exactly caused my decision. Going to address your comment and make a new push after addressing #1389 (comment).

    hebasto commented at 12:13 PM on August 9, 2023:

    Fixed.

  17. real-or-random commented at 10:57 AM on August 9, 2023: contributor

    GitHub Actions should be enabled now. I tried to close and re-open to trigger the workflow, but it didn't work. But yeah, happy to merge this (with the comment addressed) even if it doesn't run on this PR. We can always adjust.

  18. hebasto commented at 11:05 AM on August 9, 2023: member

    @real-or-random

    GitHub Actions should be enabled now.

    Just to double check, are "Actions permissions" and "Workflow permissions" set to safe values?

  19. hebasto commented at 11:14 AM on August 9, 2023: member

    I tried to close and re-open to trigger the workflow, but it didn't work.

    It should though...

    Let me try once more (perhaps, it depends on whether an actor is an author).

  20. achow101 commented at 11:25 AM on August 9, 2023: member

    The suggested permissions were applied.

  21. ci: Run "Windows (VS 2022)" job on GitHub Actions a2f7ccdecc
  22. hebasto force-pushed on Aug 9, 2023
  23. hebasto commented at 12:12 PM on August 9, 2023: member

    Hmm... I'm struggling to figure out the reason for no activity in https://github.com/bitcoin-core/secp256k1/actions.

  24. achow101 commented at 1:05 PM on August 9, 2023: member

    I think github actions only runs if the repo contains a workflow, so prs adding new workflows aren't going to be run until the repo itself has something.

  25. real-or-random approved
  26. real-or-random commented at 1:33 PM on August 9, 2023: contributor

    utACK 7d05f78fab1c930abdc19da64cc5d576d3523cb7

    Let's try merging it...

  27. real-or-random commented at 1:38 PM on August 9, 2023: contributor

    Or actually... @hebasto Can you remove the second commit for now, if we're unsure whether this works at all.

  28. hebasto force-pushed on Aug 9, 2023
  29. hebasto commented at 1:39 PM on August 9, 2023: member

    Or actually... @hebasto Can you remove the second commit for now, if we're unsure whether this works at all.

    Removed.

  30. real-or-random approved
  31. real-or-random commented at 1:43 PM on August 9, 2023: contributor

    utACK a2f7ccdecc4721d972f36d6aacc5f0c85ce0557d

  32. real-or-random merged this on Aug 9, 2023
  33. real-or-random closed this on Aug 9, 2023
  34. hebasto commented at 1:44 PM on August 9, 2023: member

    image

  35. hebasto deleted the branch on Aug 9, 2023
  36. real-or-random referenced this in commit 8d2960c8e2 on Aug 9, 2023
  37. sipa referenced this in commit c0da4f60e2 on Sep 4, 2023
  38. Retropex referenced this in commit 5f62ed90f6 on Oct 4, 2023
  39. real-or-random referenced this in commit d575ef9aca on Oct 12, 2023
  40. janus referenced this in commit 1097330147 on Apr 1, 2024
  41. hebasto referenced this in commit b6de625950 on May 11, 2024
  42. delta1 referenced this in commit 6089844b3c on Apr 2, 2025
  43. div72 referenced this in commit af627d47c3 on Apr 12, 2025
  44. str4d referenced this in commit b9e749419a on Jun 4, 2025
Contributors
hebastomaflckoreal-or-randomachow101
Labels
ci
Linked (view graph)
#1392 ci: Future of CI after Cirrus pricing change #1397 ci: Remove "Windows (VS 2022)" task from Cirrus CI
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC