Skip to content

Accept default_features for default-features #3771

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

Merged
merged 1 commit into from
Feb 28, 2017

Conversation

alexcrichton
Copy link
Member

This was accepted by this historical TOML parser, so we'll need to preserve this
ability.

Closes #3768

This was accepted by this historical TOML parser, so we'll need to preserve this
ability.

Closes rust-lang#3768
@rust-highfive
Copy link

r? @brson

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

@alexcrichton
Copy link
Member Author

r? @matklad

@matklad
Copy link
Member

matklad commented Feb 28, 2017

Was this because RustcSerizliaze accepted both _ and -? We'll need to audit other RustcDecodable things then: for example, the same problem happens with dev-dependencies:

dev_dependencies: Option<HashMap<String, TomlDependency>>,
.

I wonder if Serde has a specific option for this, to avoid duplicating every field?

@nox
Copy link
Contributor

nox commented Feb 28, 2017

@matklad It doesn't, but I suggested to @dtolnay that we could make a new #[serde(alias)] attribute.

@nox
Copy link
Contributor

nox commented Feb 28, 2017

@matklad Also, I don't think we have other underscores right now in the Servo stack, and this was to let @nrc use the latest Rust and Cargo with Servo in his RLS experiments. Could we get a r+ here?

@matklad
Copy link
Member

matklad commented Feb 28, 2017

@matklad Also, I don't think we have other underscores right now in the Servo stack, and this was to let @nrc use the latest Rust and Cargo with Servo in his RLS experiments. Could we get a r+ here?

Sure! But we still need to fix other instances I think. Perhaps even run a crater with serde-based Cargo?

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 28, 2017

📌 Commit fd25593 has been approved by matklad

@bors
Copy link
Contributor

bors commented Feb 28, 2017

⌛ Testing commit fd25593 with merge 09b68aa...

bors added a commit that referenced this pull request Feb 28, 2017
Accept `default_features` for `default-features`

This was accepted by this historical TOML parser, so we'll need to preserve this
ability.

Closes #3768
@nox
Copy link
Contributor

nox commented Feb 28, 2017

Perhaps even run a crater with serde-based Cargo?

That would be nice.

@alexcrichton
Copy link
Member Author

@matklad yeah the old rustc-serialize implementation would try both, returning whichever succeeded first. My plan is to sort of just fix these up as they come in, but yes I would also like to schedule a crater run to happen now that this moved into nightlies to assess the breakage.

@bors
Copy link
Contributor

bors commented Feb 28, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 09b68aa to master...

@bors bors merged commit fd25593 into rust-lang:master Feb 28, 2017
@matklad
Copy link
Member

matklad commented Feb 28, 2017

My plan is to sort of just fix these up as they come in,

I kinda feel that maybe it's better to fix them upfront? grepping local ~/.cargo revealed at least one crate affected: https://github.com/hyperium/mime.rs/blob/master/Cargo.toml#L16

@alexcrichton alexcrichton deleted the read-more branch February 28, 2017 19:05
@alexcrichton
Copy link
Member Author

@matklad ah yeah I just don't know how much to fix! Ideally we wouldn't have to fix any... I'll send a PR for that and build-dependencies though

matklad added a commit to matklad/mime.rs that referenced this pull request Mar 1, 2017
It is intended that `-` is used in multi-word Cargo.toml keys. `_` was historically accepted and is not going away, but let's use `-` just in case.

See rust-lang/cargo#3771
@matklad
Copy link
Member

matklad commented Mar 1, 2017

Found more more affected package:

https://github.com/hansihe/rustler/blob/0ff275292f7c730a2e605d4bfad60eecfdb71275/rustler_codegen/Cargo.toml#L10

It's proc_macro this time.

@alexcrichton
Copy link
Member Author

@matklad added in #3782

@ehuss ehuss added this to the 1.17.0 milestone Feb 6, 2022
morr0ne pushed a commit to morr0ne/neo-mime that referenced this pull request Feb 22, 2025
It is intended that `-` is used in multi-word Cargo.toml keys. `_` was historically accepted and is not going away, but let's use `-` just in case.

See rust-lang/cargo#3771
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default features of dependencies get pulled in for no reason
7 participants