-
Notifications
You must be signed in to change notification settings - Fork 200
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
Fix false negatives in dcou by using patched cargo #1913
Conversation
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
|
||
# Use `cargo "+${rust_nightly}" hack ..` once we stop using custom-built one. | ||
PATH="./cargo-for-dcou/target/release:$PATH" \ | ||
RUSTUP_TOOLCHAIN="${rust_nightly}" _ cargo hack check --lib --bins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole setup is needed just to add --lib
, which otherwise would cause errors in cargo...
also, cargo build adds ~1min, which i think it's acceptable.
the actual check run won't become any noticeably longer, i guess (need to compare after ci finishes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪖
06642aa
to
f9d8738
Compare
_ git clone --depth 1 --no-tags \ | ||
--branch "no-no-library-target-error-${rust_nightly}" \ | ||
https://github.com/anza-xyz/cargo.git cargo-for-dcou | ||
|
||
# Now build the patched cargo | ||
( | ||
cd ./cargo-for-dcou/ | ||
_ cargo "+${rust_nightly}" build --release --bin cargo | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip this if cargo is already built.
# 4. Push new branch with its name updated with the new nightly version. | ||
_ git clone --depth 1 --no-tags \ | ||
--branch "no-no-library-target-error-${rust_nightly}" \ | ||
https://github.com/anza-xyz/cargo.git cargo-for-dcou |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also add nightly version to ./cargo-for-dcou/
dir..
f9d8738
to
4c5ce6e
Compare
11702af
to
6aed8e0
Compare
6aed8e0
to
3c4086c
Compare
closing in favor of #2127 |
Problem
scripts/check-dev-context-only-utils.sh has a bug, causing false negatives in edge cases.
as an example, #687 introduced dcou misuse because ci incorrectly passed due to the false negative, which is fixed by #1831..
Summary of Changes
Fix it.
Unfortunately, this needs to fix cargo (rust-lang/cargo#14163).
sample now correctly detected errors: https://buildkite.com/anza/agave/builds/6770#01905db5-39a1-4a8a-972e-1161950f9e6f/639-3443