Skip to content

ci: extract shared setup-rust and setup-sp1 composite actions#141

Open
prajwolrg wants to merge 2 commits into
mainfrom
ci-rust-composite-actions
Open

ci: extract shared setup-rust and setup-sp1 composite actions#141
prajwolrg wants to merge 2 commits into
mainfrom
ci-rust-composite-actions

Conversation

@prajwolrg

Copy link
Copy Markdown
Collaborator

Description

Stacked on #140 (base branch ci-exclude-sp1-from-clippy); will retarget to main once #140 merges.

Every workflow repeated the same Rust setup (toolchain install + Swatinem/rust-cache + arduino/setup-protoc) with only small variations, and prover.yml/release.yml duplicated the SP1 toolchain install + cargo prove --version check verbatim. The pinned action SHAs and the rust-toolchain.toml parse were spread across nine jobs.

This extracts the setup into two local composite actions:

  • .github/actions/setup-rust (inputs: channel, components, cache, protoc) — installs the toolchain and, by default, restores the cache and installs protoc. channel: rust-toolchain.toml reads the pinned channel from that file.
  • .github/actions/setup-sp1 — sp1up install + PATH + cargo prove --version.

Per-job behavior is preserved via inputs:

job channel components cache protoc
lint/clippy nightly clippy
lint/fmt nightly rustfmt
security nightly clippy
docs nightly
functional/run nightly llvm-tools-preview
unit/test nightly llvm-tools-preview
unit/doc nightly
prover rust-toolchain.toml
release rust-toolchain.toml

Type of Change

  • Refactor

Notes to Reviewers

  • No behavior change intended — same actions/SHAs, same toolchain/components/cache/protoc per job, just relocated.
  • release.yml only runs on tags, so it isn't exercised by PR CI. But every input combination it uses (channel: rust-toolchain.toml, cache: "false") is also used by a PR-triggered job (prover, lint/fmt), so the composite is covered indirectly.
  • Composite-internal $GITHUB_PATH propagation is relied on in setup-sp1 (the version-check step reads the PATH set by the install step) — confirmed by the prover-perf job going green.

Related Issues

Jira: STR-3722 (CI cleanup split out of the manifest-MMR reorg work; follow-up to #140).

@codecov

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The clippy job runs `--all-features`, which pulls `sp1-core-executor-runner`
into the lint graph. Its build.rs runs a nested `cargo build` and embeds the
result via `include_bytes!(env!("SP1_CORE_RUNNER_BINARY"))`. Since clippy runs
build scripts, that native build executes even for a lint, and it breaks on a
`Swatinem/rust-cache` hit: the helper binary lives under the registry source
dir (not preserved by the cache), while the build-script output recording its
path is cached, so cargo skips the rebuild and `include_bytes!` can't read it.

The crate's build.rs honors a `SP1_CORE_RUNNER_OVERRIDE_BINARY` env var
(source-only; not in its README/docs): when set it skips the nested build and
points the runtime at an external binary instead. A `prebuild-sp1-runner`
composite action pre-builds the helper into `$RUNNER_TEMP` and returns its
path as an output; lint.yml and prover.yml set it as the override env only on
the steps that compile the crate. Returning a path rather than writing
`$GITHUB_ENV` keeps the override scoped to those steps instead of the whole
job. This is cache-stable and keeps the heavy build out of the lint, unlike
the previous `cargo clean` workaround it replaces in prover.yml.

The helper version is read from Cargo.lock, not the `sp1-sdk` requirement,
which is a caret that floats up to a later patch in the lock.
@prajwolrg prajwolrg force-pushed the ci-exclude-sp1-from-clippy branch from ce40d6b to 7c9c43f Compare June 7, 2026 04:54
Every workflow repeated the same Rust setup (toolchain install + Swatinem
cache + protoc) with only small variations, and prover/release duplicated the
SP1 toolchain install + version check verbatim. That spread the pinned action
SHAs and the rust-toolchain.toml parse across nine jobs, so a bump had to be
made in many places.

Factor the setup into two local composite actions:

- `setup-rust` (inputs: channel, components, cache, protoc) — installs the
  toolchain and, by default, restores the cache and installs protoc. `channel:
  rust-toolchain.toml` reads the pinned channel from that file.
- `setup-sp1` — sp1up install + PATH + `cargo prove --version`.

Behavior is preserved per job via inputs: components (clippy/rustfmt/
llvm-tools-preview), cache disabled for the fmt and release jobs, protoc
disabled for the fmt and security jobs, and the rust-toolchain.toml channel for
prover/release. Every input combination release.yml uses is also exercised by a
PR-triggered job, so the tag-only release workflow is covered indirectly.

setup-sp1 carries a `zizmor: ignore[github-env]` for its $GITHUB_PATH append:
persisting $HOME/.sp1/bin onto PATH for later steps requires $GITHUB_PATH and
the value is fixed and trusted. zizmor flags env-file writes in composite
actions (context-agnostic) even though the same write was unflagged inline in
the workflows.
@prajwolrg prajwolrg force-pushed the ci-rust-composite-actions branch from dcaf346 to e013da1 Compare June 7, 2026 04:57
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

Commit: 0c61f91
SP1 Execution Results

program cycles gas
asm-stf 136,367,055 141,452,326
moho 5,223,555 5,525,314

@prajwolrg prajwolrg marked this pull request as ready for review June 7, 2026 05:13
@prajwolrg prajwolrg marked this pull request as draft June 7, 2026 05:14
@storopoli

Copy link
Copy Markdown
Member

This is very useful. I think we should move this to strata-common or even a new strata-ci or just ci repo. What you think?

@prajwolrg prajwolrg marked this pull request as ready for review June 9, 2026 01:04
@prajwolrg prajwolrg changed the base branch from ci-exclude-sp1-from-clippy to main June 9, 2026 01:05
@prajwolrg

Copy link
Copy Markdown
Collaborator Author

This is very useful. I think we should move this to strata-common or even a new strata-ci or just ci repo. What you think?

Thanks. We can do that if it's useful, although I'm not sure how would that actually work.

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