ci: Fix image caching and apply other improvements #1756

pull hebasto wants to merge 4 commits into bitcoin-core:master from hebasto:251003-ci-docker changing 3 files +25 −43

hebasto commented at 9:57 PM on October 3, 2025: member

This PR fixes an issue where only the latest image cache was available.

For other minor improvements, see the individual commit messages.
hebasto force-pushed on Oct 4, 2025
real-or-random added the label ci on Oct 13, 2025
real-or-random added the label bug on Oct 13, 2025
real-or-random commented at 1:26 PM on October 13, 2025: contributor

Relevant docs: https://docs.docker.com/build/cache/backends/gha/
in .github/actions/run-in-docker-action/action.yml:25 in fb9856a140 outdated
```
  19 |        id: main_builder
  20 |        continue-on-error: true
  21 |        with:
  22 |          context: .
  23 |          file: ${{ inputs.dockerfile }}
  24 | -        tags: ${{ inputs.tag }}
```
real-or-random commented at 1:27 PM on October 13, 2025:

Even after looking at the docs, I'm not sure what the tags parameter is actually supposed to do. Did you figure this out?

hebasto commented at 1:48 PM on October 13, 2025:

It's meant to be used when exporting a built image to a registry.
in .github/actions/run-in-docker-action/action.yml:24 in fb9856a140 outdated
```
  22 |          context: .
  23 |          file: ${{ inputs.dockerfile }}
  24 | -        tags: ${{ inputs.tag }}
  25 |          load: true
  26 | -        cache-from: type=gha
  27 | +        cache-from: type=gha,scope=${{ runner.arch }}
```
real-or-random commented at 1:29 PM on October 13, 2025:

Would it be a good idea to add {{input.tag}} here (and maybe rename tag to scope)?

hebasto commented at 1:50 PM on October 13, 2025:

I don't think so. Tags are not used for caching.

real-or-random commented at 1:54 PM on October 13, 2025:

I think we're talking past each other. (But sorry, my message was very unclear.) My suggestion is to add an input parameter to the run-in-docker-action that is used as a prefix to scope, i.e., the caller sets the scope.

In our case, the caller could then set scope=ci-image-${{ runner.arch }} or something like this.

hebasto commented at 2:09 PM on October 13, 2025:

I see. Reworked.
in .github/actions/run-in-docker-action/action.yml:7 in fb9856a140 outdated
```
   2 | @@ -3,10 +3,8 @@ description: 'Run a command in a Docker container, while passing explicitly set
   3 |  inputs:
   4 |    dockerfile:
   5 |      description: 'A Dockerfile that defines an image'
   6 | -    required: true
   7 | -  tag:
   8 | -    description: 'A tag of an image'
   9 | -    required: true
  10 | +    required: false
  11 | +    default: ./ci/linux-debian.Dockerfile
```
real-or-random commented at 1:31 PM on October 13, 2025:

I'm not sure if this is a reasonable default. I mean, yes, this is what we always use. But currently, this action is not specific to libsecp256k1 at all, and I think it would be nice to keep it that way.

hebasto commented at 1:52 PM on October 13, 2025:

It seems unlikely that this action would be reused elsewhere.

hebasto commented at 2:57 PM on October 13, 2025:

The controversial commit has been dropped.

real-or-random commented at 8:01 AM on October 14, 2025:

It seems unlikely that this action would be reused elsewhere.

True, but I think it's still a good idea to drop the default. It simply decreases coupling.
in ci/linux-debian.Dockerfile:3 in fb9856a140
```
   0 | @@ -1,5 +1,7 @@
   1 |  FROM debian:stable-slim
   2 |  
   3 | +ENV DEBIAN_FRONTEND=noninteractive
```
real-or-random commented at 1:33 PM on October 13, 2025:

I think this is not a good idea because the Dockerfile can also be used interactively on a developer's machine. See https://serverfault.com/a/797318

hebasto commented at 2:01 PM on October 13, 2025:

Thanks! Reworked.
hebasto force-pushed on Oct 13, 2025

ci: Add `scope` parameter to `cache-{to,from}` options

This change fixes an issue where only the latest image cache was
available.

122014edb3

hebasto force-pushed on Oct 13, 2025
ci: Drop `tags` input for `docker/build-push-action`
The `tags` input is unused for caching.
b2a95a420f

ci: Bump `docker/build-push-action` version

See https://github.com/docker/build-push-action/releases.

70ae177ca0

ci: Set `DEBIAN_FRONTEND=noninteractive`

This suppresses `debconf: unable to initialize frontend: ...` warnings.

f163c35897

hebasto force-pushed on Oct 13, 2025
hebasto commented at 2:57 PM on October 13, 2025: member

@real-or-random

Thank you for the review! Your feedback has been addressed.
real-or-random approved
real-or-random commented at 7:59 AM on October 14, 2025: contributor

utACK f163c35897db9a4adc2a5b37926942817a4fdeb7
real-or-random merged this on Oct 14, 2025
real-or-random closed this on Oct 14, 2025
hebasto deleted the branch on Oct 14, 2025

Contributors

hebasto

real-or-random

Labels

bug ci