webpki-roots icon indicating copy to clipboard operation
webpki-roots copied to clipboard

Allow building for the web

Open Mubelotix opened this issue 2 years ago • 6 comments

Mubelotix avatar Apr 25 '24 17:04 Mubelotix

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.)

ctz avatar Apr 26 '24 07:04 ctz

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 avatar Apr 26 '24 07:04 Mubelotix

I made a similar PR on rustls#1921 btw

Mubelotix avatar Apr 26 '24 07:04 Mubelotix

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?

djc avatar Apr 26 '24 08:04 djc

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.

ctz avatar Apr 26 '24 08:04 ctz

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?

djc avatar Apr 26 '24 08:04 djc

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?

cpu avatar Jun 03 '24 14:06 cpu

Agreed, let's close this for now.

djc avatar Jun 03 '24 15:06 djc