-
Notifications
You must be signed in to change notification settings - Fork 224
Test crc-fast 1.4 to verify canary catches dependency issues #4447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit temporarily pins crc-fast to 1.4 to prove that the new architecture tests would have caught the illegal instruction fault on ARM before merge. This will be reverted after CI demonstrates the failure.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
The canary was using stale dependency versions from committed Cargo.lock files instead of the versions specified in PRs. This caused the canary to miss the crc-fast 1.4 SIGILL issue. Solution: Delete workspace Cargo.lock files before running the canary to force fresh dependency resolution. This ensures the canary tests the exact dependency versions from the PR. Fixes #3981
Update: Applied Cargo.lock deletion fixAll CIs passed in the previous run, proving the canary bug exists. Now applied commit 330601c which deletes workspace Cargo.lock files before canary runs. Expected result: Canary should now FAIL on ARM architectures because it will use crc-fast 1.4.0 from this PR (which causes SIGILL on ARM). This will prove the fix works. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
Closing this - turns out we can't actually reproduce the crc-fast 1.4 issue in CI. The problem is that crc-fast 1.4 only crashes on x86_64 CPUs that don't have AVX-512. GitHub Actions runners have AVX-512, and QEMU emulates it, so our tests pass even with the broken version. That said, the Cargo.lock deletion fix (330601c) is still valid. It makes sure the canary actually tests the dependency versions from the PR instead of whatever's in the committed lock files. aws-sdk-rust's canary caught the crc-fast issue because they run on real AWS instances, some of which don't have AVX-512. We just can't prove it would catch this specific bug in our CI environment. But the fix is still worth having to prevent similar issues. |
Purpose
This PR demonstrates the canary dependency resolution bug.
What this PR does
crc-fast = "1.4"in aws-smithy-checksums/Cargo.tomlExpected behavior
Canary should FAIL because crc-fast 1.4.0 causes SIGILL on ARM (awesomized/crc-fast-rust#14)
Actual behavior (bug)
Canary will PASS because it uses stale Cargo.lock files with crc-fast 1.6.0
Proof of bug
The fix
See commit 8ef68d5 on the
crc-fastbranch:Related