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)
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33302.
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.
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.
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 }}
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.
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.