rustls / webpki-roots

CA certificates for use with webpki
Apache License 2.0
89 stars 47 forks source link

Allow building for the web #69

Closed Mubelotix closed 5 months ago

ctz commented 6 months ago

I don't think this is necessary? There's no reason that a crate which takes a dependency must re-export or otherwise control the dependency's features.

If you want to set pki-types/web in your application, just write in your Cargo.toml: pki-types = { version = "*", features = ["web"] } -- job done, no need for any other crates to change.

(The version = "*" is only appropriate if you do not use any of the APIs from pki-types yourself.)

Mubelotix commented 6 months ago

I don't think this is necessary? There's no reason that a crate which takes a dependency must re-export or otherwise control the dependency's features.

If you want to set pki-types/web in your application, just write in your Cargo.toml: pki-types = { version = "*", features = ["web"] } -- job done, no need for any other crates to change.

(The version = "*" is only appropriate if you do not use any of the APIs from pki-types yourself.)

This is what I have been doing before but it doesn't feel right having to dig into the dependency tree to fix all compile errors by adding features with no standardized name.

I removed the feature on webpki-ccadb as it wasn't enough to get it compiling and I only needed webpki-roots anyway

Mubelotix commented 6 months ago

I made a similar PR on rustls#1921 btw

djc commented 6 months ago

I don't think this is necessary? There's no reason that a crate which takes a dependency must re-export or otherwise control the dependency's features.

I agree it's not strictly necessary, but it usually makes things easier for the consumer at little cost to the maintainer?

ctz commented 6 months ago

I agree it's not strictly necessary, but it usually makes things easier for the consumer at little cost to the maintainer?

I think a basic level of testing is that all combinations of a crate's features are tested. If we have features which do not vary the code or dependencies, that adds new dimensions to that testing for no benefit and eventually makes that style of testing implausible.

I think a crate should only have features that either change its dependencies or behaviour; but not just pass through to some other crate's features. I especially think that webpki-roots is an odd choice to do that.

djc commented 6 months ago

I think a crate should only have features that either change its dependencies or behaviour; but not just pass through to some other crate's features. I especially think that webpki-roots is an odd choice to do that.

That is fair. And if we add a flag for this in rustls, we probably wouldn't strictly need it here?

cpu commented 5 months ago

Since a change on the Rustls side will probably require more thought/care are we OK with merging this or should it be closed pending that work? Since it sounds like it wouldn't be necessary if that work happened, and it likely isn't very valuable on its own, I was leaning towards close. WDYT?

djc commented 5 months ago

Agreed, let's close this for now.