feat(ci): cache compiled artifacts during the image build - #76
Conversation
There was a problem hiding this comment.
Reviewed the Dockerfile + docker.yml changes.
Logic checks out: the builder stage runs with --platform=$BUILDPLATFORM and cross-compiles for arm64 rather than emulating, so hardcoding the sccache download to x86_64-unknown-linux-musl is correct, not a bug. Secret handling via --mount=type=secret + 2>/dev/null || true fallback is safe for the no-credentials (fork/hosted-runner) case, matches the "sccache: not in use" behavior described in the PR body. CARGO_INCREMENTAL=0 paired with sccache is the right call.
Nit: the sccache release tarball is fetched over HTTPS with no checksum/signature verification before being placed in /usr/local/bin. Low risk given it's straight from mozilla/sccache's GitHub releases, but worth a sha256sum -c if you want to harden the supply chain later.
Known blocker already called out in the description (pin points at an unmerged FerrLabs/.github#248 commit, draft until that lands) — not re-flagging since it's your own tracked TODO.
No correctness issues found.
|
Pin remonté sur |
There was a problem hiding this comment.
Follow-up on my earlier approval (commit 30b4687): the last commit moves the reusable-docker-build.yml pin off the draft PR to a commit on main, as planned. But that target commit has a genuine bug — see inline comment. The nit about checksum-verifying the sccache download is still open and unchanged, not re-flagging.
| name: Build & Push LFSX | ||
| needs: meta | ||
| uses: FerrLabs/.github/.github/workflows/reusable-docker-build.yml@2883414f68b4029d28d8700d2a51d070f7528111 # main | ||
| uses: FerrLabs/.github/.github/workflows/reusable-docker-build.yml@33cba2f024a028c848981e63112c5f82de04e05c # main |
There was a problem hiding this comment.
Blocking: this pin (33cba2f024a028c848981e63112c5f82de04e05c) points at a broken reusable-docker-build.yml. In the build job's shell step, the Actions-cache block is:
if [ "${SCCACHE_GHA:-false}" = "true" ] && [ -n "${ACTIONS_RUNTIME_TOKEN:-}" ]; then
if [ "${SCCACHE_GHA:-false}" = "true" ] && [ -n "${ACTIONS_RUNTIME_TOKEN:-}" ] && [ -n "${ACTIONS_CACHE_URL:-${ACTIONS_RESULTS_URL:-}}" ]; then
printf '%s' "$ACTIONS_RUNTIME_TOKEN" >"$RUNNER_TEMP/gha-runtime-token"
secret_args+=(--secret "id=gha-cache-url,src=$RUNNER_TEMP/gha-cache-url")
secret_args+=(--secret "id=gha-runtime-token,src=$RUNNER_TEMP/gha-runtime-token")
echo "Actions cache passed to the build"
fiTwo consecutive ifs with only one fi — the outer if is never closed, so the whole run: block is a bash syntax error (unexpected end of file). That fails the "Build & push (buildah)" step for every caller of this reusable workflow at this pin, not just when sccache-gha is set — it's a parse-time failure, not a runtime branch. Separately, even if it parsed, $RUNNER_TEMP/gha-cache-url is referenced by --secret but the script never writes that file (only gha-runtime-token is written).
This already merged to LFSX main, so the next docker.yml run (release/push) will fail at the build step. Needs a fix in FerrLabs/.github (remove the duplicate if/write the missing file) and then re-pin here, or revert this pin in the meantime.
Closes #52. Draft until FerrLabs/.github#248 merges — the pin below points at that PR's commit and has to move to the one on
mainfirst, exactly as #70 did.What I found when I actually built it
I had recommended parking this, on the grounds that the org's sccache points at
garage.ferrlabs.svc.cluster.localand a public repository builds on hosted runners that cannot resolve it. That part holds. The Actions cache of the run is the backend that is reachable, and wiring it turned out to have a constraint worth writing down:ACTIONS_CACHE_URLandACTIONS_RUNTIME_TOKENcannot be passed in. They exist only inside a step's environment and are not readable from a workflow expression, so nowith:orsecrets:block can carry them from here. The job that runs the build has to export them itself, which is why #248 changes the shared workflow rather than this repository handing values over. That is a larger blast radius than "add two lines todocker.yml", and it is the honest cost of this issue.The build without credentials still builds
Verified locally rather than assumed:
docker buildx buildwith no secrets mounted printssccache: not in useand compiles normally. That is the fork pull request case, where GitHub withholds everything — a build that failed there would have been a worse regression than the slow build this fixes.--show-statsruns after the compile, so the hit rate is in the build log rather than a matter of faith.What I cannot report yet
The issue asks for a before and after. The before is on record: 5 m 08 s for the two-architecture build of v0.17.2. The after needs a release to happen twice — once to populate the cache, once to use it — so the number goes in the issue when it exists, not in this description as a guess.