-
Notifications
You must be signed in to change notification settings - Fork 100
feat: make CryptoProvider selectable via crate features #4174
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
Conversation
- Add two new optional features:
- `rustls-ring` – enables `rustls/ring`, re-exports `rustls::crypto::ring::default_provider`
- `rustls-aws-lc-rs` – enables `rustls/aws-lc-rs`, re-exports `rustls::crypto::aws_lc_rs::default_provider`
- Default feature set is `["rustls-ring"]`, keeping the current behaviour unchanged.
- The two features are mutually exclusive; enabling both raises a clear
compile-time error.
- README updated to show how to switch provider:
cargo add <crate> --no-default-features --features rustls-aws-lc-rs
Downstream crates can now pick their rustls crypto provider without
patching or forking.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
How does this work for something like |
Thank you for your guidance in case you are ignored and left unprocessed. In addition to the above three crates, which other related crates need to be processed together. Thank you! |
Basically every crate in this repo. There are about 200 of them. And most of them are generated. Even the This is why we have hesitated to just add some features. We need to design the features so they work for all crates, even crates that depend on each other. For example: We like what you did with the default provider in this PR, but we need to design something that works across all crates, including the changes in the code generator. |
|
@coryan Upgrade reqwest to 0.13.x, okay? |
@coryan After query, only 'google-cloud-storage' crate relies on 'google-cloud-auth' crate. |
But they all depend on |
|
I want to thank you for the PR, but I think we will use a different approach. I think we will just use a feature (enabled by default) to use the default provider in Please give me a couple of days and I will ping you once my proposal is ready for review. |
|
FWIW, I think #4182 is closer to what we need. That is still missing the changes to the generated crates, and more tests, but seems (to me) easier to maintain and test. |
Support users can choose to enable different engines. As well as the current 'google-cloud-storage' relies on 'google-cloud-auth' and 'gaxi', we need to consider the enabling of 'google-cloud-storage' relying on 'features'. It would be best if 'aws-lc-rs' is finally enabled by default. thank |
feat: make CryptoProvider selectable via crate features
rustls-ring– enablesrustls/ring, re-exportsrustls::crypto::ring::default_providerrustls-aws-lc-rs– enablesrustls/aws-lc-rs, re-exportsrustls::crypto::aws_lc_rs::default_provider["rustls-ring"], keeping the current behaviour unchanged.compile-time error.
cargo add --no-default-features --features rustls-aws-lc-rs
Downstream crates can now pick their rustls crypto provider without
patching or forking.
#4170