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

Jump to bottom

Add support for deserializing enums in config files #8454

Merged
merged 2 commits into from Jul 7, 2020
Merged

Add support for deserializing enums in config files #8454

merged 2 commits into from Jul 7, 2020

Conversation

GabrielMajeri
Copy link
Contributor

Implements deserialize_enum functionality to allow config options which are Rust enums.

@ehuss The code currently has some todo!s because I'm not sure how the custom Deserializer is supposed to do error handling.

Fixes #8450

@rust-highfive
Copy link

r? @Eh2406

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 5, 2020
@GabrielMajeri
Copy link
Contributor Author

r? @ehuss

@rust-highfive rust-highfive assigned ehuss and unassigned Eh2406 Jul 5, 2020
@alexcrichton
Copy link
Member

Could this perhaps try to deserialize all enums as strings? I'm not really sure how we'd represent something like enum A { Foo { field: usize } } in TOML anyway, so we'd likely only use this with strings regardless.

@GabrielMajeri
Copy link
Contributor Author

@alexcrichton I'd love to, but I'm not very good with serde. That's why there's some todos in the code 😅 I've requested @ehuss's review since they mentioned they could help.

Could this perhaps try to deserialize all enums as strings?

Does that mean I should call visit_string instead of visit_enum 🤔 ?

ehuss
ehuss reviewed Jul 6, 2020
View reviewed changes
src/cargo/util/config/de.rs Outdated Show resolved Hide resolved
src/cargo/util/config/de.rs Outdated Show resolved Hide resolved
src/cargo/util/config/de.rs Outdated Show resolved Hide resolved
@GabrielMajeri GabrielMajeri marked this pull request as ready for review July 6, 2020 19:12
@GabrielMajeri
Copy link
Contributor Author

@ehuss I've applied your suggestions, and also added a test to ensure the code graciously fails on an invalid enum variant.

@ehuss
Copy link
Contributor

ehuss commented Jul 7, 2020

Looks great, thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jul 7, 2020

📌 Commit 65fc4ce has been approved by ehuss

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 7, 2020
@bors
Copy link
Collaborator

bors commented Jul 7, 2020

⌛ Testing commit 65fc4ce with merge 729e567...

@bors
Copy link
Collaborator

bors commented Jul 7, 2020

☀️ Test successful - checks-azure
Approved by: ehuss
Pushing 729e567 to master...

@bors bors merged commit 729e567 into rust-lang:master Jul 7, 2020
@bors bors mentioned this pull request Jul 7, 2020
Closed
@GabrielMajeri GabrielMajeri deleted the config-option-enum branch July 7, 2020 16:47
@ehuss ehuss mentioned this pull request Jul 7, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 7, 2020
@Manishearth
Rollup merge of rust-lang#74137 - ehuss:update-cargo, r=ehuss
08f8624 
Update cargo

5 commits in fede83ccf973457de319ba6fa0e36ead454d2e20..729e5676a02404b1d745013f8b280945cfa2d50d
2020-07-02 21:51:34 +0000 to 2020-07-07 15:51:28 +0000
- Add support for deserializing enums in config files (rust-lang/cargo#8454)
- Write GNU tar files, supporting long names. (rust-lang/cargo#8453)
- Don't overwrite existing `rustdoc` args with --document-private-items (rust-lang/cargo#8449)
- Add some help about rustup's +toolchain syntax. (rust-lang/cargo#8455)
- Update metadata man page. (rust-lang/cargo#8451)
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
@Manishearth
Rollup merge of rust-lang#74137 - ehuss:update-cargo, r=ehuss
2f6e169 
Update cargo

6 commits in fede83ccf973457de319ba6fa0e36ead454d2e20..4f74d9b2a771c58b7ef4906b2668afd075bc8081
2020-07-02 21:51:34 +0000 to 2020-07-08 17:13:00 +0000
- Disable long_file_names test if not supported on Windows. (rust-lang/cargo#8469)
- Add support for deserializing enums in config files (rust-lang/cargo#8454)
- Write GNU tar files, supporting long names. (rust-lang/cargo#8453)
- Don't overwrite existing `rustdoc` args with --document-private-items (rust-lang/cargo#8449)
- Add some help about rustup's +toolchain syntax. (rust-lang/cargo#8455)
- Update metadata man page. (rust-lang/cargo#8451)
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
@Manishearth
Rollup merge of rust-lang#74137 - ehuss:update-cargo, r=ehuss
9646357 
Update cargo

6 commits in fede83ccf973457de319ba6fa0e36ead454d2e20..4f74d9b2a771c58b7ef4906b2668afd075bc8081
2020-07-02 21:51:34 +0000 to 2020-07-08 17:13:00 +0000
- Disable long_file_names test if not supported on Windows. (rust-lang/cargo#8469)
- Add support for deserializing enums in config files (rust-lang/cargo#8454)
- Write GNU tar files, supporting long names. (rust-lang/cargo#8453)
- Don't overwrite existing `rustdoc` args with --document-private-items (rust-lang/cargo#8449)
- Add some help about rustup's +toolchain syntax. (rust-lang/cargo#8455)
- Update metadata man page. (rust-lang/cargo#8451)
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
@Manishearth
Rollup merge of rust-lang#74137 - ehuss:update-cargo, r=ehuss
178cdc5 
Update cargo

6 commits in fede83ccf973457de319ba6fa0e36ead454d2e20..4f74d9b2a771c58b7ef4906b2668afd075bc8081
2020-07-02 21:51:34 +0000 to 2020-07-08 17:13:00 +0000
- Disable long_file_names test if not supported on Windows. (rust-lang/cargo#8469)
- Add support for deserializing enums in config files (rust-lang/cargo#8454)
- Write GNU tar files, supporting long names. (rust-lang/cargo#8453)
- Don't overwrite existing `rustdoc` args with --document-private-items (rust-lang/cargo#8449)
- Add some help about rustup's +toolchain syntax. (rust-lang/cargo#8455)
- Update metadata man page. (rust-lang/cargo#8451)
@ehuss ehuss added this to the 1.46.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss ehuss left review comments

Assignees

@ehuss ehuss

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.46.0
Development

Successfully merging this pull request may close these issues.

invalid type: string "debuginfo", expected enum Strip
6 participants
@GabrielMajeri @rust-highfive @alexcrichton @ehuss @bors @Eh2406