merge: land #8629 (cargo-test prune tolerates a missing target/debug/deps) - #8631
Merged
Conversation
… step The full cargo-test job pruned linked test binaries between packages with a bare 'find target/debug/deps ... -delete', but that directory only exists once something has built into it. When the scope's first package builds straight to --release (the perry/perry-stdlib branch above it), or before any cargo test -p <package> has run in the per-package loop, the directory is absent and find exits 1, failing the whole job even though every test passed. This has been failing main's scheduled and push CI runs repeatedly and is unrelated to any specific PR's changes.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe full ChangesDependency cleanup tolerance
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Lands #8629 (external contributor, fork PR) plus the changelog fragment it was
missing.
Audit
The mechanism checks out:
findon a missing directory exits 1, and GitHubActions runs
run:blocks asbash -e {0}, so that rc kills the step eventhough every test passed. Verified locally — rc 1 before the guard, rc 0 after.
actionlintfindings are identical onorigin/mainand on this branch(same 3, at lines 1011 and 3489; the PR touches 1078 and 1088), so the
contributor's "no new findings" claim is verified rather than taken on trust.
One noted tradeoff (not blocking)
2>/dev/null || truealso suppresses genuinefinderrors. The prune is diskhygiene and should never fail the job, so swallowing is the right call here —
but if it ever silently stops working, the symptom becomes a later disk
exhaustion rather than a pointed error. Recorded in the changelog fragment.
Fork PR, so this lands as a branch rather than a push to the contributor's
head ref.
Summary by CodeRabbit
Bug Fixes
Documentation