-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Take advantage of weak features in Rust 1.60 for TLS enablement #454
Conversation
a061b23
to
a3f872c
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.
Agree that this would be a nice improvement. Though you correctly note that we can't just change this right now and need to wait a while.
d2d3be0
to
b7b62af
Compare
@MarijnS95 Its been a while ;-) |
acc56b3
to
10b737e
Compare
@Swatinem cool, sure! That was a bit more work than expected, with both conflicts and more crates in other Let's see how the CI likes this :) |
10b737e
to
1b61984
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.
nice!
there is a few places that still has pkg = { package = "pkg", …
It might be worth removing that, but its also fine to merge like this.
Previously it was impossible to have a unified feature named `rustls` or `native-tls` that would turn on the respective TLS backend in the chosen transport (`reqwest` or `ureq`) as a feature of `<crate>/tls` would implicitly turn on `<crate>`. Since Rust 1.60 it is now possible to specify this crate-feature enablement through the use of the question mark in `<crate>?/tls`, which will only enable the `tls` feature if `<crate>` was enabled through other means (another feature). Secondly we can now also let optional crates have the same name as a feature, and use `dep:<crate>` to refer to the crate instead of the dependency, making for a more pleasant experience without renames to underscore suffixes (that subsequently have to be dealt with in Rust code, e.g. with `use log_ as log;` renames).
1b61984
to
7b1f3cd
Compare
@Swatinem Good catch, looks like I missed those while resolving conflicts! There was also a stray |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #454 +/- ##
==========================================
+ Coverage 78.60% 78.65% +0.05%
==========================================
Files 76 76
Lines 9206 9192 -14
==========================================
- Hits 7236 7230 -6
+ Misses 1970 1962 -8 |
Previously it was impossible to have a unified feature named
rustls
ornative-tls
that would turn on the respective TLS backend in the chosen transport (reqwest
orureq
) as a feature of<crate>/tls
would implicitly turn on<crate>
. Since Rust 1.60 it is now possible to specify this crate-feature enablement through the use of the question mark in<crate>?/tls
, which will only enable thetls
feature if<crate>
was enabled through other means (another feature).Secondly we can now also let optional crates have the same name as a feature, and use
dep:<crate>
to refer to the crate instead of the dependency, making for a more pleasant experience without renames to underscore suffixes.Filing this as draft just to get the word out and discuss this for a bit, as it will only be mergeable in 6 months in accordance with the MSRV policy. It goes without saying that this is a breaking change.
Also note that I haven't renamed other optional crates like
anyhow_
,log_
etc yet, until this is accepted as a (future) solution.