Skip to content

Improve Rust cache reuse with revision-isolated targets (breaking build contract) - #417

Open
aram356 wants to merge 3 commits into
mainfrom
feat/rust-cache-revision-isolation
Open

aram356 wants to merge 3 commits into
mainfrom
feat/rust-cache-revision-isolation

Conversation

@aram356

@aram356 aram356 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Improve dependency-source and compiler-cache reuse across application commits
while keeping writable Cargo target archives strictly revision-isolated.

Breaking build/cache contract: callers must supply the application's full
SHA when enabling target restores, explicitly opt into target saves, and stop
assuming builds write to <application>/target. Production/staging deployment
semantics, credential boundaries, CLI packaging and release verification are
unchanged.

Closes #416.

Review feedback addressed

  • Target saves are opt-in through save-target (default false). Readers use
    actions/cache/restore; writers use actions/cache's success-only post-job
    save, after compilation. Neither restores targets from another revision.
  • Full store-aware/store-free fixture target writes are restricted to main.
    Fixed Git dates make unchanged fixture revisions stable across reruns; a
    tracked upstream SHA accounts for EdgeZero path-dependency changes.
  • The cross-job seed now compiles both native and WASM libraries. A run-specific
    library identity excludes accidental older compiler hits. The next revision
    changes only README and requires error-free hits on its first builds with a
    fresh target directory, not a same-job build/clean/rebuild.
  • Both downstream jobs reconstruct the seed's exported fixture identity, so
    retrying only failed jobs does not change their expected source SHA. All
    three jobs install/select the same .tool-versions Rust compiler.
  • Identical setup repeats skip all remote restores and compiler-cache setup.
    Cache-disabled Git workspaces can use local HEAD for ownership; unidentified
    non-Git repeats still fail. Mixed source/workspace/lock/cache settings, unsafe
    paths and unowned targets remain rejected.
  • Output contracts recognize cache action identity independently of version/SHA
    pinning; the separate pin gate still applies. Prepare uses append_output.
  • Shared test assertions, executable probe scripts, permission-safe cleanup,
    control-character rejection and unused-output removal address the cleanup nits.
  • The adoption guide includes complete checkout/SHA/cache/diagnostic steps,
    explicit migration, GitHub branch scope and named sccache limitations. CLI
    comments distinguish a shared job-local target from remote target caching.

The former lockfile-only target policy is intentionally replaced, as required
by #416. Cargo fingerprinting is not a reason to relax this isolation contract.
This does not guarantee a warm complete application build: linker-invoking
crate types, build-script execution and linking still cost work on cold targets.

Migration

Use a stable app-name and capture the checked-out application's full Git SHA,
not the deployment repository SHA. Configure:

with:
  app-name: example-app
  cache-target: true
  save-target: ${{ github.ref == 'refs/heads/main' }}
  target-cache-revision: ${{ steps.source.outputs.sha }}
  working-directory: application

save-target: true requires target caching enabled. Its default is restore-only.
Main-only saves select the workflow ref, not the application branch being built.
Use CARGO_TARGET_DIR or the cargo-target-dir output for build paths, including
when remote target caching is disabled. Prefer one setup per app per job; an
identical repeat is safe but performs no cache restore. First use of v2 target
keys, and each new source revision, has a cold target cache.

Verification

  • Cargo workspace/all-targets: 1,520 passed, 0 failed, 1 ignored.
  • Cargo fmt, strict all-feature Clippy, feature-combination checks and Spin WASM.
  • Cache behavior, retry fixture, CI wiring and output-contract regressions.
  • Deployment contracts: 643 passed, 0 failed; five existing Linux-only
    checks skipped locally on macOS and covered by PR CI.
  • Local sccache v0.16.0 probe: restarted server, new Git revision and fresh
    target directory; first native and WASM builds each hit with zero cache errors.
  • Full ShellCheck, actionlint, third-party pin gate, offline Zizmor scan,
    docs format/lint/build and git diff --check.
  • Independent plan and code review; no outstanding correctness findings.
  • GitHub-hosted exact-target restore and cross-revision native/WASM
    first-build compiler-reuse probes on commit ff18102.
  • Application fixture CI
    observed 204 Rust hits and 17 misses (92.3%), zero cache read/write errors,
    and no full target archive save on this PR run. This is a warmed CI fixture,
    not a production deployment benchmark or a promised hit ratio.
  • Final deployment-action CI, including Linux contracts and static checks.
  • Remaining final GitHub-hosted CI checks.

No cache deletion, live deployment, merge, release, Trusted Server source change
or Rust dependency upgrade is included. Deployer adoption is in
trusted-server-deployer #36.

Separate stable dependency source keys from source-revision target keys, keep build paths stable, and add local isolation guards and real cache/compiler probes.

Closes #416
@aram356 aram356 self-assigned this Oct 13, 2026
@aram356 aram356 changed the title Improve Rust cache reuse with revision-isolated target artifacts Improve Rust cache reuse with revision-isolated targets (breaking build contract) Oct 13, 2026

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The revision-isolated target cache contract, opt-in saves, and migration requirements are clearly documented.

@dhruv8sh dhruv8sh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

This keeps dependency and sccache reuse across commits, but keys the saved target/ archive to the application's exact revision, with an explicit save-target opt-in and a stable CARGO_TARGET_DIR. The design is solid and thoroughly tested, and all 28 checks pass. The findings below are non-blocking.

😃 Praise

  • 😃 Retry-safe compiler-reuse checks: the seed job exports its fixture ID, so a "rerun failed jobs" rebuilds the same SHA. The fixture library includes the run ID, so leftover cache from an older run can't produce a false pass. assert-compiler-reuse.sh requires hits on the very first native and WASM builds, with no build/clean/rebuild shortcut, and fixture.sh checks all of these properties itself.
  • 😃 Thorough validation in prepare.sh, with a negative test for each check (see inline).

Findings

Non-blocking

  • 🤔 Fixture target saves on main will rarely be restored (.github/workflows/deploy-action.yml:264, :325, inline)
  • 🤔 Several apps set up in one job overwrite each other's CARGO_TARGET_DIR (.github/actions/setup-rust-build-cache/scripts/prepare.sh:160, inline)
  • ⛏ Missing yq gives a misleading error (.github/actions/deploy-core/tests/run.sh:1996, inline)
  • ⛏ The plan docs have every checkbox unticked: docs/superpowers/plans/2026-10-12-… and 2026-10-13-… have 48 open - [ ] items, though the work is done. Tick them or mark the plans complete so they aren't read as pending.
  • 📝 Both hit outputs read false on a repeated setup call: on an identical second call, source-cache-hit and target-cache-hit are false even if the first call hit. The guide already says this; callers should read hit status from the first call.
  • 🌱 The cross-job seed saves a new small target entry on every run (deploy-action.yml:140): the fixture ID changes per run, so every run (PRs included) saves a new entry. They're tiny now and GitHub's 7-day eviction cleans them up, but worth watching if the fixture grows.

CI Status

  • fmt: PASS
  • clippy: PASS on GitHub. A local-only clippy::missing_trait_methods unfulfilled-expectation error in unchanged edgezero-core code is a toolchain quirk on my machine, not this PR.
  • tests: PASS (1,520 passed). setup-rust-build-cache/tests/run.sh: PASS. deploy-core/tests/run.sh: 658 passed, 1 failed. The failure (healthcheck-fastly public surface) is a locale sort-order difference in an action this PR doesn't touch.
  • GitHub checks: all 28 PASS

with:
app-name: store-aware-fixture
cache-target: true
save-target: ${{ github.ref == 'refs/heads/main' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Fixture target saves on main will rarely be restored

save-target: ${{ github.ref == 'refs/heads/main' }} is keyed to the fixture's commit SHA. That SHA covers .edgezero-source-revision, which holds EdgeZero's own HEAD (make-smoke-fixture.sh:159), so every qualifying push to main gets a new key. PR runs use a merge-commit HEAD, so they never match a saved key either.

The result is two full release target/ archives (fixture-release and store-free-release; the fixture CLI depends on edgezero-cli) saved per main push. Only a rerun of that same main commit would restore them. They count against the repo's 10 GB cache limit and can push out the dependency and sccache entries that actually give cross-commit reuse. It also goes against the PR's own guide (docs/guide/deploy-action-adoption.md:97: "Do not save a full target on every disposable fixture/PR run").

Fix: rely on sccache and dependency caching for these fixtures:

with:
  app-name: store-aware-fixture
  cache-target: false
  working-directory: fixture-app

Or keep restore-only and drop save-target, then update the fixture.sh:148-154 assertion that requires the main-only save expression. The same applies at line 325.

with:
app-name: store-free-fixture
cache-target: true
save-target: ${{ github.ref == 'refs/heads/main' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Same as the comment at line 264: this full-release target save will be keyed to a new fixture SHA on each main push and is unlikely ever to be restored.

printf 'RUSTC_WRAPPER=sccache\n'
printf 'SCCACHE_GHA_ENABLED=true\n'
printf 'CARGO_INCREMENTAL=0\n'
printf 'CARGO_TARGET_DIR=%s\n' "$target_dir"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Several apps set up in one job overwrite each other's CARGO_TARGET_DIR

Every setup writes CARGO_TARGET_DIR and EDGEZERO__RUST_CACHE__TARGET_DIR to GITHUB_ENV, and the last call wins. With apps A and B set up in one job, later builds of A (including build-app-cli) go into B's folder. If B has save-target: true, B's revision-keyed archive is saved with A's artifacts inside it, which defeats the per-revision isolation. On main this only happened with cache-target: true; now it happens in every mode.

The guide's wording "Prefer one setup per application per job" (:109, :142) suggests several apps per job is fine.

Fix: either document "one application per job" as a hard rule, or make prepare.sh refuse a second setup when a different app's selection marker already exists under $RUNNER_TEMP/edgezero-rust-cache/.

if [[ -z "$script" ]]; then
# actions/cache declares cache-hit itself, not through a local script.
# Ref pinning is enforced separately; accept only this documented output.
if [[ "$out_name" == cache-hit ]] && command -v yq >/dev/null 2>&1; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏ Missing yq gives a misleading error

Without yq, a legitimate cache-hit output falls through to "names step '…', which runs no script", which hides the real cause. Suggest failing clearly instead:

command -v yq >/dev/null || fail "yq v4 is required to check $name_of output '$out_name'"

local source_prefix source_key target_key=''
source_prefix="edgezero-app-$app_name-rust-sources-$runner_os-$runner_arch-"
source_key="$source_prefix$lock_sha"
[[ -z "$revision" ]] || target_key="edgezero-app-$app_name-rust-target-v2-$runner_os-$runner_arch-$lock_sha-$revision-exact"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😃 Thorough validation

  • The -exact key suffix stops a 40-character SHA from prefix-matching a 64-character one, and there's a test for it.
  • realpath output is checked for control characters while keeping trailing newlines.
  • It rejects symlinks, refuses to adopt non-empty targets it doesn't own, and keeps the ownership marker outside the cached folder.
  • Everything is validated before anything is written to GITHUB_OUTPUT/GITHUB_ENV.

Each of these has a negative test in revision.sh/review.sh.

This branch has not been deployed

No deployments
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.

Separate Rust source cache reuse from revision-isolated target artifacts

3 participants