ci: enable cargo cache in Earthly for CI runs; use branch as cache key locally - #2033
ci: enable cargo cache in Earthly for CI runs; use branch as cache key locally#2033ozgb wants to merge 5 commits into
Conversation
…y locally Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c5a141293
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if [[ "${CI:-}" != "true" ]]; then | ||
| export EARTHLY_BUILD_ARGS="${EARTHLY_BUILD_ARGS:+$EARTHLY_BUILD_ARGS,}CACHE_KEY=$(git rev-parse --abbrev-ref HEAD 2>/dev/null | tr '/' '-' || echo local)" |
There was a problem hiding this comment.
When the self-hosted continuous-integration.yml build job invokes +images and the main.yml publish job invokes +node-image/+toolkit-image, they source this file with GitHub's CI=true but do not pass --CACHE_KEY. This condition therefore leaves the Earthfile default of local, so the newly unconditional /target mounts become target-local-$TARGETARCH for every PR and release branch on those persistent runners, recreating the cross-branch stale-artifact failures that the cache key is intended to prevent. Ensure these callers derive and pass a sanitized ref, or derive it here from the GitHub ref variables instead of skipping CI.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
The CI branch of .envrc skipped CACHE_KEY entirely, so only the four jobs that pass --CACHE_KEY by hand were scoped; +images (build node and images) and main.yml's +node-image/+toolkit-image fell back to the "local" default and shared one /target across every branch on the persistent runners — exactly the cross-branch staleness the key exists to prevent. Derive it from GITHUB_HEAD_REF/GITHUB_REF_NAME (rev-parse is useless under actions/checkout's detached HEAD) with the same sanitizing the workflows use. A target arg on the command line still overrides it, verified against earthly 0.8.16, so the compact-contract-tests fixed key is unaffected. Assisted-by: Claude:claude-opus-5 Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>
… caches
Enabling the cargo caches on CI made concurrent builds on one buildkit
daemon corrupt each other: `failed to unpack windows v0.52.0 ... .cargo-ok:
File exists` and `failed to stat <polkadot-sdk git checkout>/westend.json`,
which took out all 3 retries of the amd64 build.
cargo already serializes precisely the section that must not overlap —
downloading/unpacking crates and updating git checkouts — with an flock on
$CARGO_HOME/.package-cache, and downgrades to a shared lock for the compile.
That lock was inert here: it lives in the image layer while registry/ and
git/ are shared CACHE mounts, so each concurrent container flocked its own
private file. Symlink it onto the registry mount so the exclusion spans them.
Measured with two concurrent `cargo fetch` runs (277 crates) sharing one
registry mount, earthly 0.8.16:
before: both ran the whole fetch window concurrently, 0 lock waits
after: each reports "Blocking waiting for file lock on package cache",
both still complete in 4s — compiles stay parallel
Buildkit's CACHE --sharing locked is not an alternative: measured, a mount
held locked by one build is not waited on by others; they are handed a fresh
empty cache dir instead, which would mean an empty registry mid-build.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d5b408887
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| END | ||
| CACHE --sharing shared --id cargo-git /usr/local/cargo/git | ||
| CACHE --sharing shared --id cargo-reg /usr/local/cargo/registry | ||
| CACHE --id target-${CACHE_KEY}-${TARGETARCH} /target |
There was a problem hiding this comment.
The final tree fixes the previously named .envrc-based +images callers, but the self-hosted rebuild-metadata-bot.yml and rebuild-chainspec-bot.yml jobs still do not source .envrc, and this commit removes their cache-disabling EARTHLY_BUILD_ARGS without supplying CACHE_KEY. Consequently +rebuild-metadata reaches +build-node-only, and an unversioned +rebuild-chainspec reaches +node-image, with this newly unconditional target-local-$TARGETARCH mount shared across PR branches; stale artifacts can therefore be committed as metadata or chainspec output. Pass a sanitized steps.pr.outputs.ref as --CACHE_KEY in both supported bot workflows.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
| # EARTHLY_BUILD_ARGS survives; a `+target --CACHE_KEY=...` on the command line still wins | ||
| # over this (that is how the compact-contract-tests job keeps its fixed shared key). | ||
| if [[ "${EARTHLY_BUILD_ARGS:-}" != *CACHE_KEY=* ]]; then | ||
| _cache_key=$(printf '%s' "${GITHUB_HEAD_REF:-${GITHUB_REF_NAME:-$(git rev-parse --abbrev-ref HEAD 2>/dev/null)}}" | tr -c 'A-Za-z0-9._-' '-') |
There was a problem hiding this comment.
When two valid refs differ only by a character replaced by tr—for example feature/foo and feature-foo—both become the same cache key. If those branches build on the shared self-hosted daemon, they again share /target, defeating the isolation intended to prevent cross-branch stale Cargo artifacts. Append a hash of the original ref, or otherwise use an injective encoding rather than replacement alone.
Useful? React with 👍 / 👎.
.envrc now derives CACHE_KEY from the GitHub ref vars, so the four jobs that computed it by hand were duplicating it. Verified equivalent: none of these workflows trigger on pull_request_target, the only event where GITHUB_HEAD_REF is set but `github.event_name == 'pull_request'` is false, and every one of these steps sources .envrc before invoking earthly. compact-contract-tests keeps its explicit --CACHE_KEY: a target arg on the command line overrides EARTHLY_BUILD_ARGS (verified against earthly 0.8.16), so its deliberately fixed, cross-branch key still wins. The rebuild-*-bot workflows were the remaining hole — they pin EARTHLY_CONFIG themselves rather than sourcing .envrc, so removing their CI=true (which used to disable the caches outright) left them on the "local" key while running on self-hosted runners; +rebuild-metadata reaches +build-node-only, which mounts /target. They cannot just source .envrc either: on issue_comment events the GitHub ref vars name the default branch, so a metadata rebuild would write the PR's artifacts into main's cache. Use the PR head ref they already resolve. Checked: actionlint clean, and the derivation yields the head ref on pull_request, the queue ref on merge_group, `main` on push, the local branch off-CI, and leaves a preset value untouched. Assisted-by: Claude:claude-opus-5 Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7a5178a33
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # EARTHLY_BUILD_ARGS survives; a `+target --CACHE_KEY=...` on the command line still wins | ||
| # over this (that is how the compact-contract-tests job keeps its fixed shared key). | ||
| if [[ "${EARTHLY_BUILD_ARGS:-}" != *CACHE_KEY=* ]]; then | ||
| _cache_key=$(printf '%s' "${GITHUB_HEAD_REF:-${GITHUB_REF_NAME:-$(git rev-parse --abbrev-ref HEAD 2>/dev/null)}}" | tr -c 'A-Za-z0-9._-' '-') |
There was a problem hiding this comment.
When main.yml is manually dispatched with its branch input set to a branch other than the workflow's selected ref, the checkouts at main.yml:45 and main.yml:263 build the input branch, but GITHUB_REF_NAME still identifies the workflow-run ref and takes precedence over Git's checked-out state here. Both publish jobs can therefore reuse another branch's /target cache and push binaries containing stale artifacts. The fresh final-tree evidence beyond the earlier missing-key comment is that a key is now derived, but it is derived from a ref that can differ from the explicit checkout input; pass that input as the cache key or derive the checked-out commit/ref instead.
Useful? React with 👍 / 👎.
2 participants
Overview
Follow-up to #1814
🗹 TODO before merging
📌 Submission Checklist
git commit -s) for the DCO🧪 Testing Evidence
Please describe any additional testing aside from CI:
🔱 Fork Strategy
Links