-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 bug with PathAndArg config values #8629
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
📌 Commit ddc7090 has been approved by |
☀️ Test successful - checks-actions |
No worries, thanks for the quick merge! |
Update cargo 5 commits in ab32ee88dade1b50c77347599e82ca2de3fb8a51..51b66125ba97d2906f461b3f4e0408f206299bb6 2020-08-10 17:44:43 +0000 to 2020-08-19 20:22:52 +0000 - Add chapters on dependency resolution and SemVer compatibility. (rust-lang/cargo#8609) - Renames SourceId::into_url -> SourceId::as_url (rust-lang/cargo#8611) - Fix bug with PathAndArg config values (rust-lang/cargo#8629) - Show full error context on `cargo run` error. (rust-lang/cargo#8627) - Fix typo in SIGQUIT description (rust-lang/cargo#8615)
This fixes an issue I noticed when trying to specify a target runner via the
CARGO_TARGET_{triplet}_RUNNER
environment variable, expecting it to override the value in our.cargo/config.toml
file, which was giving quite strange errors until I figured out that cargo was actually merging the config file's values with the environment's values.This change adds a small hack to use and
UnmergedStringList
fromPathAndArgs
instead of just plainStringList
, which uses the type in the deserializer to determine ifConfig::get_list_or_string
should merge the values from the config file(s) with the environment as it was doing before, or else only use the environment to the exclusion of the config file(s) if the key was set in the environment.I also added a test for this to confirm both the bug and the fix.