Add an optional matrix field allowing opt-out of configuring cirrus GHA cache when not using cirrus runners.
This is not needed for the cirruslabs/[save|restore]-cache actions, as they automatically fallback based on runner type.
Addresses #31965 (comment)
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33302.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | m3dwards |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
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.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
506 | @@ -506,7 +507,7 @@ jobs: 507 | - name: Configure Docker 508 | uses: ./.github/actions/configure-docker 509 | with: 510 | - use-cirrus: ${{ needs.runners.outputs.use-cirrus-runners }} 511 | + use-cirrus: ${{ matrix.use-cirrus != null && matrix.use-cirrus || needs.runners.outputs.use-cirrus-runners }}
?
any || true, will just be true, no?
Also, the error looks the same, so this has no effect?
506 | @@ -506,7 +507,7 @@ jobs: 507 | - name: Configure Docker 508 | uses: ./.github/actions/configure-docker 509 | with: 510 | - use-cirrus: ${{ needs.runners.outputs.use-cirrus-runners }} 511 | + use-cirrus: ${{ matrix.use-cirrus != null && matrix.use-cirrus || matrix.use-cirrus == null && needs.runners.outputs.use-cirrus-runners }}
OK I think this push got it: https://github.com/bitcoin/bitcoin/actions/runs/17473683984/job/49628131536?pr=33302#step:5:339
also, looks like a cache outage in another task?
426 | @@ -427,6 +427,7 @@ jobs: 427 | matrix: 428 | include: 429 | - name: '32 bit ARM, unit tests, no functional tests' 430 | + use-cirrus: 'false' # As we use GH runners here, manually opt-out of trying to use a cirrus-hosted docker cache.
This works but I do feel a little icky about using a truthy string value of 'false'. I realise this gets passed to bash which doesn't have types but it is used in gha expression which does have concept of truthy. It took me a second to understand how false || true resulted in false and then I clocked it was a string.
Do you think it's too invasive to perhaps change the input from a boolean "use-cirrus" to a string "cache-provider" with values of "cirrus" and "gha"?
Done in ff18b6bbaf3.
LMK what you think of the new approach. I agree it's clearer.
Run on my fork at: https://github.com/willcl-ark/bitcoin/actions/runs/17626151823
Avoid relying on future truthy evaluations of string 'false'.
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Add an optional matrix field allowing opt-out of configuring cirrus
GHA cache when not using cirrus runners.
This is not needed for the cirruslabs/[save|restore]-cache actions, as
they automatically fallback based on runner type.
ACK 00c253d494176b31dc4aaba24dc7e61aecb20be2