fix: unbreak CI (clippy lint, and the linker on the test job) - #459
gopikannappan wants to merge 2 commits into
Conversation
`cargo clippy --all-targets -- -D warnings` fails on main with twenty `clippy::manual_slice_size_calculation`-style rejections of `chunks_exact` at a constant size, so the lint job has been red for everyone and any new pull request inherits the failure. `as_chunks` gives the same slices with their length in the type, which is what the lint asks for: the loop body destructures `[low, high]` rather than indexing a slice the compiler cannot bound. The remainder is dropped exactly as `chunks_exact` dropped it, so every call keeps its behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The workspace test job does not fail a test. Its linker dies:
error: linking with `cc` failed: exit status: 1
collect2: fatal error: ld terminated with signal 7 [Bus error]
Signal 7 out of `ld` on a hosted runner is the shape of exhausted
memory, and `--all-targets` asks it to link every test binary in the
workspace with full debug info. The env already carries
`CARGO_INCREMENTAL: 0` for the neighbouring reason, so this sits beside
it: nothing in CI reads a backtrace, and dropping debug info is the
cheapest thing that makes those binaries smaller.
The AVX2 leg used to replace `RUSTFLAGS` rather than extend it, which
would have dropped the new flag on the one leg that also fails, so it
now appends.
I cannot reproduce the crash to prove this fixes it: the same workspace
links and runs clean on aarch64-darwin, 996 tests across 20 binaries.
Drop this commit if you would rather solve it another way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
CI on this branch says the diagnosis was right and turns up one thing underneath it. Clippy passes, so the lint half is settled and #458's red check clears with it. The linker survives. That reveals exactly one failing test, which the crash had been hiding: It is not environmental. It fails the same way on The mutation is still rejected, which is the property that matters. It is rejected as It dates from I have not touched the assertion, because the two readings need someone who knows the intent:
I cannot tell which from outside, and this is a test about a mutation being detected, which is the wrong place to guess. Happy to push either change once you say which it is, or to leave it to you entirely. To be explicit about what that means for this PR: |
|
I said earlier I could not tell which of the two readings applied. I can now, and it is the one that argues against the obvious fix. The two mutations in this test are deliberately different. Their messages say so:
So the test is not merely checking that tampering is rejected. It is pinning which layer rejects it: the first case says the physical leaf-hash row is constrained locally, the second says a disconnected witness is caught by the global lookup. The first mutation can no longer be caught locally, by construction. It edits I confirmed the second half of that: changing the expectation to Which is why I do not think flipping that word is the right fix. It would turn the first case into a second copy of the one below it, and quietly retire the only check that the physical leaf-hash row is locally constrained. The test would go green while testing less than it was written to test. Two honest options, and the choice is yours:
Either way it is a deliberate decision about what the test guarantees, which is why I have left it alone. Happy to push whichever you prefer. Worth noting this failure predates this branch: it is on |
main's CI is red on two jobs, so every pull request opened against it inherits the failure. #458 has carried a red "Formatting and Clippy" since 10 September for this reason rather than anything in its own diff.The two failures have nothing to do with each other.
Clippy
cargo clippy --all-targets -- -D warningsrejects twenty uses ofchunks_exactat a constant size, acrosscircuit,circuit-proverand their tests. A recent stable promoted that lint; nothing in the repository changed.as_chunksis what it asks for, and it reads better where the pair is destructured:The remainder is dropped exactly as
chunks_exactdropped it, so behaviour is unchanged at every call. Plonky3 took the same route for the same lint last week.The workspace test job
This one is not a failing test. The linker dies:
Signal 7 out of
ldon a hosted runner is the shape of exhausted memory, andcargo nextest run --workspace --all-targetsasks it to link every test binary in the workspace with full debug info.The second commit drops debug info in CI, beside the
CARGO_INCREMENTAL: 0you already carry for the neighbouring reason. It also makes the AVX2 leg append toRUSTFLAGSinstead of replacing it, since replacing would have dropped the new flag on the one leg that also fails.I want to be straight about what I can and cannot show here. I cannot reproduce the crash, so I cannot prove that fixes it. What I can say is that the same workspace links and runs clean on
aarch64-apple-darwin: 996 tests across 20 binaries, exit 0, on8f9876ewith these changes. If you would rather solve the runner pressure another way, drop that commit and take the clippy one; it is the half that unblocks #458's red check either way.Checked
cargo clippy --all-targets -- -D warnings: clean.cargo +nightly fmt --all -- --check: clean.cargo test --workspace: 996 passed, 0 failed.p3-circuitlib and itsblake3_compresstest after the edits: 418 and 7 passing.🤖 Generated with Claude Code