Skip to content

ci: run cached-packages freshness check on every PR - #394

Open
apenzk wants to merge 3 commits into
m1from
chore/regenerate-framework-artifacts
Open

ci: run cached-packages freshness check on every PR#394
apenzk wants to merge 3 commits into
m1from
chore/regenerate-framework-artifacts

Conversation

@apenzk

@apenzk apenzk commented Jul 13, 2026

Copy link
Copy Markdown

Framework-generated artifacts (Move docs and the SDK transaction builder) frequently drift from their sources on m1 because the CI freshness check that should catch it doesn't run on ordinary PRs.

rust-build-cached-packages runs cargo_build_aptos_cached_packages.sh --check and fails if the tree is left dirty, but its if: only lets it run on auto_merge / push to m1 / workflow_dispatch / the CICD:run-e2e-tests label — not on ordinary PR events — so it was skipped on both #386 and #382 and never flagged their drift.

This PR removes that gate so it runs on every PR (like rust-lints), and commits the currently-stale artifacts to resync m1. Added CI cost is small — roughly $0.10 per push.

Evidence

The gap already let drift land: two PRs where the check was skipped, whose artifacts this PR regenerates (cargo build -p aptos-cached-packages, no source changes):

Follow-up

(repo setting, after this merges)

Once rust-build-cached-packages is running on PRs, add it to m1's required-status-checks ruleset (currently general-lints, rust-lints, rust-cargo-deny, rust-targeted-unit-tests) so a red result blocks the merge. Ungating alone only makes it go red — it doesn't block until it's required.

apenzk added 2 commits July 13, 2026 16:20
The Move docs and SDK builder had drifted from their framework sources: two prior PRs changed the sources without regenerating the committed artifacts.
- #386 (delegation-pool partial governance voting) — governance/delegation/staking docs and the SDK builder
- #382 (ALLOW_SERIALIZED_SCRIPT_ARGS, flag 72) — features.md

Regenerated via `cargo build -p aptos-cached-packages`.
- remove the event-gate on rust-build-cached-packages so it runs on all PRs like rust-lints, instead of only at merge time — it was skipped on #386 and #382 and let stale framework artifacts land
@apenzk

apenzk commented Jul 13, 2026

Copy link
Copy Markdown
Author

Proposing to remove the gate so the freshness check runs on every PR.

The generated artifacts drift (dirty tree) fairly often, and the gated check doesn't catch it pre-merge.

@ganymedio ganymedio 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.

Ungating the job is the right call, but as-is this will make rust-build-cached-packages red on every PR for a reason unrelated to any PR's contents.

The job doesn't only fail on artifact drift — it also fails because CI can't format the generated bindings. Two runs from before any of the current drift existed:

Both predate the staking_contract and delegation_pool source changes, so there was no drift to catch, and both still failed — with the dirty list containing only aptos_token_sdk_builder.rs and aptos_token_objects_sdk_builder.rs. The diff on those is pure formatting (unmerged use statements, stray leading blank line), i.e. raw generator output.

Cause: rust-toolchain.toml sets components = ["cargo", "clippy", "rustc", "rust-docs", "rust-std"] with no rustfmt, and rust-ci-image-setup never adds it. release_builder.rs:106-124 writes the bindings then shells out to rustfmt; the rustup shim exists so the spawn succeeds, but it exits non-zero ("not installed for the toolchain"), and line 117 uses .output() without checking status.success() — so the failure is swallowed and the unformatted file is left on disk. Locally rustfmt is usually present, which is why the tree comes out clean off-CI and dirty on it.

Suggested change, in rust-toolchain.toml:

-components = ["cargo", "clippy", "rustc", "rust-docs", "rust-std"]
+components = ["cargo", "clippy", "rustc", "rust-docs", "rust-std", "rustfmt"]

Stable rustfmt is sufficient — rustfmt --config imports_granularity=crate on 1.86.0 reproduces the committed merged-import form exactly, so no nightly toolchain is needed. With that in place, one CI run should confirm the two token builders come back clean and the check goes green on the regenerated artifacts in this PR.

Worth adding separately: check status.success() on the rustfmt call in release_builder.rs. A silent formatting failure is what let this sit red on m1 since at least June without being noticed.

One ordering note — the required-status-checks follow-up should wait until the check is actually green, otherwise it blocks all merges.

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.

2 participants