-
Notifications
You must be signed in to change notification settings - Fork 119
fix: do not sanitize CARGO_HOME & RUSTUP_HOME
#609
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
Adjusts the cargo-wdk integration test harness to avoid stripping rustup/cargo-related environment variables, fixing local test failures when rustup is installed in a non-default location (Issue #606).
Changes:
- Replace broad env-var sanitization (
CARGO*/RUST*) with targetedPATHsanitization. - Stop removing
.rustup/toolchain-related entries fromPATH. - Rename
sanitize_env_varstosanitize_pathand update its intent/docs accordingly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
48d1346 to
9fc7af5
Compare
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.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d25a3b0 to
c9b6ce9
Compare
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4f55db9 to
492b728
Compare
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.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c18fb46 to
9e6861c
Compare
9e6861c to
b8b5677
Compare
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removing these variables may cause some tests to fail when `rustup` or `cargo` is installed at non-standard paths (issue microsoft#606).
b8b5677 to
355c931
Compare
CARGO_HOME & RUSTUP_HOME
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
=======================================
Coverage 77.41% 77.41%
=======================================
Files 24 24
Lines 4853 4853
Branches 4853 4853
=======================================
Hits 3757 3757
Misses 979 979
Partials 117 117 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #606
Earlier we removed all the
rustupandcargorelated env vars before invokingcargo wdk buildin tests to prevent them from influencing latter's behaviour. However, it caused some tests to fail in situations whererustuporcargoare installed at non-standard paths. So in this PR we skip removingCARGO_HOMEandRUSTUP_HOME.