Skip to content

fix(ci): add rust program cache in ci#513

Merged
MauroToscano merged 6 commits into
mainfrom
ci/cache-rust-elfs-for-prover-tests
Apr 20, 2026
Merged

fix(ci): add rust program cache in ci#513
MauroToscano merged 6 commits into
mainfrom
ci/cache-rust-elfs-for-prover-tests

Conversation

@ColoCarletti

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

Copy link
Copy Markdown

Codex Code Review

  1. Medium (Bug / CI correctness): stale Rust ELF cache can bypass recompilation
    • Location: .github/workflows/pr_main.yaml:197, .github/workflows/pr_main.yaml:255
    • The cache key only hashes executor/programs/rust/**, the target JSON, and syscalls/**, but compile behavior is also controlled by Makefile (toolchain pin, build flags, sysroot usage).
    • Because compile is skipped on exact cache hit, changes like nightly-... version/flags in Makefile can leave CI running tests against stale ELF artifacts.
    • Fix: include build-logic inputs in the key (at least Makefile; optionally workflow/toolchain config), or always run make compile-programs-rust and rely on cached executor/shared_target for speed.

No direct Rust/VM runtime security vulnerability is introduced by this diff itself.

Comment thread .github/workflows/pr_main.yaml
Comment thread .github/workflows/pr_main.yaml

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

I would replace the .github/actions/setup-rust/action.yaml with Makefile to force recompiling Rust programs when the Rust version set in the Makefile changes.

@MauroToscano
MauroToscano added this pull request to the merge queue Apr 20, 2026
Merged via the queue into main with commit c66f029 Apr 20, 2026
10 checks passed
@MauroToscano
MauroToscano deleted the ci/cache-rust-elfs-for-prover-tests branch April 20, 2026 21:05
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.

3 participants