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

Forwarding the rustls provider settings #288

Open
LecrisUT opened this issue Sep 16, 2024 · 7 comments
Open

Forwarding the rustls provider settings #288

LecrisUT opened this issue Sep 16, 2024 · 7 comments

Comments

@LecrisUT
Copy link

It would be good to use a design of the features similar to axum-server, i.e.:

tls-rustls = ["tls-rustls-no-provider", "rustls/aws-lc-rs"]
tls-rustls-no-provider = [...]

This would make it easier to forward these options down the chain of dependencies.

@djc
Copy link
Member

djc commented Sep 17, 2024

It is pretty unclear what you are trying to achieve. What's the use case here?

@LecrisUT
Copy link
Author

The issue is with propagationg the choice of rustls/ring vs rustls/aws-lc-rs that needs to be propagated to all dependencies until the leaf package. E.g. in atuin it has dependencies on metrics-exporter-prometheus and axum-server, the former having a dependency on hyper-rustls. axum-server allows to import the tls-rustls-no-provider feature in which case you can manually add rustls/aws-lc-rs or rustls/ring avoiding conflicts of each package having a different default tls provider. In the case of atuin after rustls switched the default provider, their CI is no longer buildable and it blocks the upgrade path because of this.

@djc
Copy link
Member

djc commented Sep 17, 2024

So why can't you depend on hyper-rustls with default-features = false? hyper-rustls unconditionally depends on rustls already, so the setup in axum-server doesn't necessarily translate well to this crate.

@LecrisUT
Copy link
Author

Of course that's possible, but then we would have to manage the non-aws-lc-rs feature, i.e. we would have to manually inlcude ["http1", "tls12", "logging"]. The proposal is to move all of the non-provider features into a group so that downstream can more easily compose it.

@djc
Copy link
Member

djc commented Sep 17, 2024

Honestly I don't think this makes much sense. Just repeat the features downstream where it makes sense.

@cpu
Copy link
Member

cpu commented Sep 17, 2024

This feels like a place where cargo could be more helpful, allowing expressing something like "defaults minus feature X". In absence of that, and for a crate like hyper-rustls where the default features seldom change, I think I concur with Djc that repeating the features you want from the default set, without the ones you don't, is likely the best solution.

@djc
Copy link
Member

djc commented Sep 17, 2024

This feels like a place where cargo could be more helpful, allowing expressing something like "defaults minus feature X".

That would be:

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

No branches or pull requests

3 participants