Skip to content
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

Improvements to StringList config handling. #7891

Merged
merged 1 commit into from Feb 17, 2020
Merged

Conversation

@ehuss
Copy link
Contributor

ehuss commented Feb 16, 2020

StringList was using an untagged enum to deserialize a string or list. Unfortunately, serde does not handle untagged enums very well. The error messages are not very good, and it doesn't interact with untyped deserializers (like our environment variables). This switches it to a newtype struct, and then has hard-coded support for it in the deserializer. This fixes some deserialization errors (like treating true as a boolean) and provides better error messages.

StringList is currently used for build.rustflags, target.*.rustflags, and target.*.runner.

Fixes #7780
Fixes #7781

@rust-highfive

This comment has been minimized.

Copy link

rust-highfive commented Feb 16, 2020

r? @Eh2406

(rust_highfive has picked a reviewer for you, use r? to override)

@Eh2406

This comment has been minimized.

Copy link
Contributor

Eh2406 commented Feb 17, 2020

Looks like an improvement.
@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 17, 2020

📌 Commit fd25863 has been approved by Eh2406

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 17, 2020

⌛️ Testing commit fd25863 with merge 914e31d...

bors added a commit that referenced this pull request Feb 17, 2020
Improvements to StringList config handling.

`StringList` was using an untagged enum to deserialize a string or list.  Unfortunately, serde does not handle untagged enums very well.  The error messages are not very good, and it doesn't interact with untyped deserializers (like our environment variables).  This switches it to a newtype struct, and then has hard-coded support for it in the deserializer.  This fixes some deserialization errors (like treating `true` as a boolean) and provides better error messages.

`StringList` is currently used for `build.rustflags`, `target.*.rustflags`, and `target.*.runner`.

Fixes #7780
Fixes #7781
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 17, 2020

☀️ Test successful - checks-azure
Approved by: Eh2406
Pushing 914e31d to master...

@bors bors merged commit fd25863 into rust-lang:master Feb 17, 2020
11 checks passed
11 checks passed
homu Test successful
Details
rust-lang.cargo Build #20200216.7 succeeded
Details
rust-lang.cargo (Linux beta) Linux beta succeeded
Details
rust-lang.cargo (Linux nightly) Linux nightly succeeded
Details
rust-lang.cargo (Linux stable) Linux stable succeeded
Details
rust-lang.cargo (Windows x86_64-msvc) Windows x86_64-msvc succeeded
Details
rust-lang.cargo (build_std) build_std succeeded
Details
rust-lang.cargo (docs) docs succeeded
Details
rust-lang.cargo (macOS) macOS succeeded
Details
rust-lang.cargo (resolver) resolver succeeded
Details
rust-lang.cargo (rustfmt) rustfmt succeeded
Details
@ehuss ehuss mentioned this pull request Feb 18, 2020
bors added a commit to rust-lang/rust that referenced this pull request Feb 19, 2020
Update cargo

9 commits in 3c53211c3d7fee4f430f170115af5baad17a3da9..e02974078a692d7484f510eaec0e88d1b6cc0203
2020-02-07 15:35:03 +0000 to 2020-02-18 15:24:43 +0000
- Set an environment variable for tests to find executables. (rust-lang/cargo#7697)
- Rework internal errors. (rust-lang/cargo#7896)
- Improvements to StringList config handling. (rust-lang/cargo#7891)
- Add new/old rustflags to fingerprint log. (rust-lang/cargo#7890)
- Fix inaccurate doc comment on `env_args`. (rust-lang/cargo#7889)
- Add some extra fingerprint debug information. (rust-lang/cargo#7888)
- Link the licenses into crates/cargo-platform (rust-lang/cargo#7886)
- Modify test to make `rustc` PR mergeable (rust-lang/cargo#7883)
- Keep environment variables in a BTreeMap to preserve sort order (rust-lang/cargo#7877)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.