fix(make): build the guest ELFs the prover targets read - #918
Open
Oppen wants to merge 7 commits into
Open
Conversation
test-prover, test-prover-all, test-prover-debug, test-prover-cuda and test-disk-spill depended on compile-recursion-elfs only, so on a clean checkout every test reading executor/program_artifacts/rust/*.elf or */asm/*.elf panicked with "elf not found". test-fast additionally runs the executor suite, which reads most of the Rust guests. compile-prover-test-elfs builds the asm guests, the recursion guests, and the eight Rust guests the prover suite reads — not all 35, since the .elf rules are FORCE and would re-enter cargo per guest on every run. check-prover-test-elfs greps the prover sources and fails if one is missing from the list, so the next test to read a new guest cannot silently reintroduce the failure; it runs from lint, which CI already invokes.
The pattern matched [A-Za-z0-9_] only, so a guest directory with a hyphen would have been skipped silently. Also states what the guard does not cover.
check-prover-test-elfs only caught guests a prover test reads that are missing from PROVER_TEST_GUESTS. The mirror case is just as breaking and nothing flagged it: an entry left in the list after its guest directory is deleted makes compile-prover-test-elfs ask cargo to build a path that no longer exists, so it surfaces as a build error instead of a lint failure. feat/hint-arena (#942) deletes hint_min and hint_multi and adds prover reads of ecrecover_hints and hint_arena, so both halves of that collision are already scheduled. The guard now also fails when the grep matches nothing at all: an empty result means the artifact path moved or the tests build the guest name at runtime, and either way the check has silently stopped checking anything. prover/benches joins prover/src and prover/tests in the scan, since a bench reading a guest hits the same missing-ELF failure.
The prover targets were not the only ones reading prebuilt artifacts without asking for them. Six more run tests that read ELFs off disk with no prerequisite to build them, so on a clean checkout they fail the same way: - test-flamegraph runs executor/tests/flamegraph.rs, which reads both asm and Rust guests. - test-cuda-integration, test-cuda-fallback, test-prover-comprehensive-cuda, bench-prover and bench-prover-cuda each prove a prebuilt asm guest through asm_elf_bytes (bench_single uses fib_iterative_1M). The GPU ones survive today only because scripts/gpu_test.sh happens to run compile-programs-asm and compile-programs-rust before calling them; invoked directly they fail like the rest. Adding compile-programs-asm is free when the artifacts are current: unlike the Rust guests, the asm rule has a real file prerequisite (%.elf: %.s), so make skips it instead of re-entering a compiler.
The comment justified PROVER_TEST_GUESTS by the cost of re-entering cargo once per guest "on every test run". Measured warm on this tree that re-entry is about 0.1s per guest: `make compile-programs-rust` with all 35 artifacts up to date takes 3.35s against 2.53s for `make compile-prover-test-elfs`, so the subset is worth roughly a second per run and the framing oversells it. What the subset really buys is the case the target exists for, a clean checkout, where the full set compiles 27 extra guest crates from scratch.
Four conflicts, all in the Makefile and all the same shape: main wrapped the GPU test recipes in $(GPU_TEST_TIMEOUT) while this branch added the ELF prerequisites those targets were missing. Both sides are kept. test-cuda-integration, test-cuda-fallback and test-prover-comprehensive-cuda get the timeout and compile-programs-asm; test-prover-cuda gets the timeout and this branch's compile-prover-test-elfs in place of compile-recursion-elfs. The .PHONY block merged on its own: main's test-cuda-d1 and this branch's compile-prover-test-elfs / check-prover-test-elfs are on different lines.
Two arrivals from main read prebuilt guests the same way the targets fixed earlier in this branch do. test-cuda-d1 proves all_instructions_64, an asm guest. test-cuda-fallback grew a second leg, gpu_force_downgrade, which reads executor/program_artifacts/rust/ethrex.elf — a Rust guest, so compile-programs-asm alone does not cover that target any more; naming $(RUST_ARTIFACTS_DIR)/ethrex.elf builds the one artifact it needs instead of pulling in the other seven through compile-prover-test-elfs. check-prover-test-elfs already covered the new read: gpu_force_downgrade lives in prover/tests and ethrex was in PROVER_TEST_GUESTS, so the guard stayed green across the merge without touching the list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The prover
maketargets now build every guest ELF the prover suite reads, and alintguard keeps that list from drifting.Why
test-prover,test-prover-all,test-prover-debug,test-prover-cudaandtest-disk-spilldepended oncompile-recursion-elfsonly. Everything else thesuite reads off disk —
executor/program_artifacts/rust/*.elf(8 guests, viastd::fs::read) andexecutor/program_artifacts/asm/*.elf(viatest_utils::run_asm_elf) — was never built, so on a clean checkout those targetspanic with
Failed to read ELF/ "run make compile-programs-rust". CI does not seeit: its jobs call
make compile-programs-rustexplicitly first.test-fastadditionally runs-p executor, whosetests/rust.rsreads ~30 of theRust guests, so it needs the full set rather than the prover subset.
How
PROVER_TEST_GUESTS— the 8 Rust guests prover tests read (allocator,commit_sum,ecsm,ef_io_demo,ethrex,hint_min,hint_multi,pure_commit) — andcompile-prover-test-elfs, which builds those plus the asmand recursion guests.
$(RUST_ARTIFACTS): the.elfrules areFORCE(cargo owns the dep graph),so depending on all 35 re-enters cargo 35 times on every local test run.
check-prover-test-elfsgrepsprover/srcandprover/testsforprogram_artifacts/rust/*.elfand fails on any name missing from the list. Itruns from
lint, which CI already invokes, and costs one grep. Without it thenext test that reads a new guest silently reintroduces the failure — which is how
this got here.
Validation
Clean checkout in a fresh worktree (no
program_artifacts/, noshared_target/):make compile-prover-test-elfs→ 8 Rust + 9 recursion + 218 asm artifacts, nothing else.make test-prover→ 557 passed, 0 failed, 24 ignored.make lint→ clean (includes the new guard).make check-prover-test-elfs PROVER_TEST_GUESTS="allocator"lists the 7 others and exits 1.
Split out of #874 review feedback; no code change, Makefile only.