Skip to content

perf(ci): give docker engine build sccache + reuse prebuilt binary - #5628

Open
abcxff wants to merge 2 commits into
mainfrom
stack/perf-ci-give-docker-engine-build-sccache-reuse-prebuilt-binary-lsxylpyy
Open

perf(ci): give docker engine build sccache + reuse prebuilt binary#5628
abcxff wants to merge 2 commits into
mainfrom
stack/perf-ci-give-docker-engine-build-sccache-reuse-prebuilt-binary-lsxylpyy

Conversation

@abcxff

@abcxff abcxff commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@abcxff

abcxff commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Stack for rivet-dev/actors

Get stack: forklift get 5628
Push local edits: forklift submit
Merge when ready: forklift merge 5628

change lsxylpyy

@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review

Small, low-risk CI change that adds sccache (Depot WebDAV backend) to the engine Docker build. Overall this closely mirrors the existing pattern already used in docker/build/*.Dockerfile and docker/builder-base/*.Dockerfile, which makes it easy to verify.

What looks good

  • Secret handling: DEPOT_TOKEN is passed via --mount=type=secret (build secret), not ARG/ENV, so it never lands in image history/layers. Matches the secure pattern already used elsewhere in this repo.
  • Fail-safe fallback: if the token is missing or the WebDAV health check (sccache --start-server && sccache --show-stats) fails, the script unsets RUSTC_WRAPPER and falls back to a normal (uncached) cargo build rather than hard-failing the build. Good for fork PRs / cases where secrets.DEPOT_TOKEN isn't populated.
  • Version pinning: SCCACHE_VERSION=v0.8.2 matches the version already pinned in docker/builder-base/{linux-gnu,linux-musl,osxcross,windows-mingw}.Dockerfile, so this doesn't introduce a second version to track.
  • Arch handling: unlike the other Dockerfiles (which hardcode x86_64 because those cross-compile from x86_64 runners), this one branches on $TARGETARCH to fetch the matching sccache binary. That's correct here because docker-images in publish.yaml runs engine-full/engine-slim natively on per-arch runners (depot-ubuntu-24.04-arm-8 / depot-ubuntu-24.04-8), so this is the right behavior for this Dockerfile specifically.
  • Dead commented-out line (# cargo install --locked tokio-console) was cleaned up in the same diff, fine as incidental cleanup.

Notes / minor points

  • Title vs. diff scope: the PR title says "give docker engine build sccache + reuse prebuilt binary", but the diff only implements the sccache portion. There's no logic here that reuses a prebuilt rivet-engine binary (e.g. downloading an artifact from the build/build-binaries job instead of recompiling in docker-images). Worth confirming whether that part is intentionally deferred to a follow-up PR or if the title/description should be trimmed to match what's actually shipped here.
  • New download target: this is the first place in the repo that fetches sccache-${SCCACHE_VERSION}-aarch64-unknown-linux-musl.tar.gz (existing Dockerfiles only ever fetch the x86_64 asset since they cross-compile). Worth confirming the arm64 leg of the docker-images job actually succeeds in CI, since a wrong asset name would only surface at build time on that specific runner/arch.
  • Since the fallback path is silent-but-logged (stdout echo), a persistently-failing cache backend wouldn't fail the build, only silently lose the speedup. Not a blocker since this mirrors existing behavior in the sibling Dockerfiles, just flagging for awareness.

Security

No concerns. Secret is build-secret mounted (not baked into layers), and the trust boundary here (CI-only, using secrets.DEPOT_TOKEN which is already used elsewhere in this same workflow) is unchanged.

Test coverage

This is CI/build-infra config; there's no unit-testable surface here. The real verification is a green docker-images run (both arch legs) in CI, ideally confirming the "[sccache] enabled via ..." log line appears and a repeat build shows reduced compile time / cache hits via sccache --show-stats.

@abcxff
abcxff force-pushed the stack/feat-ci-selective-preview-publish-via-targets-input-ozmvkswy branch from 3709589 to 73f7f7b Compare September 1, 2026 23:04
Base automatically changed from stack/feat-ci-selective-preview-publish-via-targets-input-ozmvkswy to main September 1, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant