Sorry for the delay, but I fail to see the issue.

Is there any way we can reproduce the issue ?

Whatever the bug, I feel like it's a non fixable issue since the issue would arise with the other feature enabled. I think the solution provided in code is the solution, if both are specified, user needs to install the crypto provider manually (or the default). No ?
This is just how features work, they need to be able to be built with both enabled, right?

I think the solution provided in code is the solution, if both are specified, user needs to install the crypto provider manually

Yes, this is basically the issue. All other crates in the ecosystem have their features set up in a way, that either the aws OR the ring provider is used and never both, depending on which features you set for those crates.

Let me describe the issue in a bit more detail:

rustls picks the provider automatically here, if either ring OR aws-lc-rs is set as a feature flag. If both features are active, it can't auto-select a provider and the user of any crate that depends on rustls (transitively) needs to select the provider. With the rustls default features enabled, aws-lc-rs is enabled as a provider.

Why is this a problem for hf-hub? Well, it isn't really your problem. But changing the rustls dependency, like I did in this PR, would make it fit better in the ecosystem.

As hf-hub with the rustls-tls feature

So when hf-hub now enables the default features of rustls, while at the same time using reqwest as a dependency with ring as the provider, hf-hub forces all of its users to depend on both ring AND aws-lc-rs. This installs unnecessary dependencies for downstream users that only want to use ring, and forces them to specify a provider.

As the rustls only gets installed with the rustls-tls feature enabled, I think the best fix is to go the ureq route and choose ring as the provider.

To reproduce:

Using main branch of this repo:

$ cargo new hf-hub-dep-test
$ cd hf-hub-dep-test
$ cargo add --git https://github.com/huggingface/hf-hub --branch main --no-default-features --features rustls-tls,tokio,ureq
$ rg "aws-lc-rs" Cargo.lock
42:name = "aws-lc-rs"
1239: "aws-lc-rs",
1273: "aws-lc-rs",

Using the branch of this PR:

$ cargo new hf-hub-dep-test
$ cd hf-hub-dep-test
$ cargo add --git https://github.com/flip1995/hf-hub --branch rustls-features --no-default-features --features rustls-tls,tokio,ureq
$ rg "aws-lc-rs" Cargo.lock
# No output

Can you provide an example where this causes an issue ? I wasn't able to reproduce anything.

rustls has their default set on aws-lc-sys for good reason. I don't thinking changing around the default makes sense.
The other projects have ring as default most likely because of legacy.

tokio has aws-lc as a default: https://github.com/rustls/tokio-rustls/blob/main/Cargo.toml#L20.

You PR comes with the same caveat as the current state. You also force ring and cause issue if someone wants to use aws-lc.
I think using our dependencies defaults (even if they do not agree) makes it simpler than re-exposing every possible feature, also violating ourselves the promise of additive nature of features.

Now if there's an example of an actual issue not solvable in the current state we can look into it.

rustls has their default set on aws-lc-sys for good reason. I don't thinking changing around the default makes sense.
The other projects have ring as default most likely because of legacy.

Correct, aws-lc-sys is the default for ring and for most of the above mentioned crates. However all crates listed above are providing a way to opt-out of aws-lc-sys and into ring. They usually do this by providing a rustls-tls feature and disabling their default features, which uses aws-lc-sys. The same thing this PR is doing. See for example reqwest, hyper-rustls, and tokio-rustls. And hf-hub is already using this feature-functionality with reqwest and tries to provide the same functionality.

You PR comes with the same caveat as the current state. You also force ring and cause issue if someone wants to use aws-lc.

Not quite: The current state forces the ring feature through enabling the reqwest/rustls-tls feature while disabling reqwest's default features, when using hf-hub with the rustls-tls feature. However, by not disabling default features in rustls, it also forces aws-lc-rs. This PR now only forces ring with the rustls-tls feature and only aws-ls-rs when used with the native-tls feature/default features, bringing it in line with the other crates.

Now if there's an example of an actual issue not solvable in the current state we can look into it.

rustls provides a way to choose the CryptoProvider manually with the builder_with_provider, if the "crate features are ambigous". So in theory it is solvable in the current state. But I'd argue that the current state is still flawed, which this PR is trying to fix:

You rarely use those builders or setup rustls yourself, but rather through one on those dependencies tokio/hyper/tonic/reqwest/... This means, that the following code works and chooses ring as the crypto provider:

[package]
name = "hf-hub-repro"
version = "0.1.0"
edition = "2021"

[dependencies]
secret-vault = { version = "^1", default-features = false, features = ["gcp-secretmanager"] }
tokio = { version = "1.42.0", features = ["rt", "macros"] }

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    secret_vault::gcp::GcpSecretManagerSource::new("my-project").await?;

    Ok(())
}

But, if then adding hf-hub as a dependency, choosing rustls-tls over native-tls as a feature¹, the same code as above stops working.

cargo add hf-hub --features rustls-tls,tokio,ureq --no-default-features

$ RUST_BACKTRACE=1 cargo run
thread 'main' panicked at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.23.20/src/crypto/mod.rs:249:14:
no process-level CryptoProvider available -- call CryptoProvider::install_default() before this point
stack backtrace:
   0: rust_begin_unwind
             at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/std/src/panicking.rs:662:5
   1: core::panicking::panic_fmt
             at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/panicking.rs:74:14
   2: core::panicking::panic_display
             at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/panicking.rs:264:5
   3: core::option::expect_failed
             at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/option.rs:2025:5
   4: core::option::Option<T>::expect
             at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/option.rs:928:21
   5: rustls::crypto::CryptoProvider::get_default_or_install_from_crate_features
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.23.20/src/crypto/mod.rs:248:24
   6: rustls::client::client_conn::ClientConfig::builder_with_protocol_versions
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.23.20/src/client/client_conn.rs:300:13
   7: rustls::client::client_conn::ClientConfig::builder
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.23.20/src/client/client_conn.rs:277:9
   8: tonic::transport::channel::service::tls::TlsConnector::new
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tonic-0.12.3/src/transport/channel/service/tls.rs:36:23
   9: tonic::transport::channel::tls::ClientTlsConfig::into_tls_connector
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tonic-0.12.3/src/transport/channel/tls.rs:120:9
  10: tonic::transport::channel::endpoint::Endpoint::tls_config
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tonic-0.12.3/src/transport/channel/endpoint.rs:251:17
  11: gcloud_sdk::api_client::GoogleEnvironment::init_google_services_channel::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gcloud-sdk-0.26.1/src/api_client.rs:208:13
  12: gcloud_sdk::api_client::GoogleApiClient<B,C>::with_token_source::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gcloud-sdk-0.26.1/src/api_client.rs:51:87
  13: gcloud_sdk::api_client::GoogleApiClient<gcloud_sdk::api_client::GoogleApiClientBuilderFunction<C>,C>::from_function_with_token_source::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gcloud-sdk-0.26.1/src/api_client.rs:141:10
  14: gcloud_sdk::api_client::GoogleApiClient<gcloud_sdk::api_client::GoogleApiClientBuilderFunction<C>,C>::from_function_with_scopes::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gcloud-sdk-0.26.1/src/api_client.rs:121:10
  15: gcloud_sdk::api_client::GoogleApiClient<gcloud_sdk::api_client::GoogleApiClientBuilderFunction<C>,C>::from_function::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gcloud-sdk-0.26.1/src/api_client.rs:105:10
  16: secret_vault::gcp::gcp_secret_manager_source::GcpSecretManagerSource::with_options::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/secret-vault-1.15.0/src/gcp/gcp_secret_manager_source.rs:44:14
  17: secret_vault::gcp::gcp_secret_manager_source::GcpSecretManagerSource::new::{{closure}}
             at /home/pkrones/.cargo/registry/src/index.crates.io-6f17d22bba15001f/secret-vault-1.15.0/src/gcp/gcp_secret_manager_source.rs:34:10
  18: hf_hub_repro::main::{{closure}}
             at ./src/main.rs:3:66

Fixing this in code looks like this: Adding rustls as a direct dependency and then call install_default on the ring provider. But adding rustls as a direct dep also means that one must be careful with managing rustls versions, so that it doesn't get installed with two different versions.

diff --git a/Cargo.toml b/Cargo.toml
index 97dcc26..72658aa 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -5,5 +5,6 @@ edition = "2021"
 
 [dependencies]
 hf-hub = { version = "0.4.1", features = ["rustls-tls", "tokio", "ureq"], default-features = false }
+rustls = { version = "0.23.20", default-features = false, features = ["ring"] }
 secret-vault = { version = "^1", default-features = false, features = ["gcp-secretmanager"] }
 tokio = { version = "1.42.0", features = ["rt", "macros"] }
diff --git a/src/main.rs b/src/main.rs
index 041e110..570192a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,5 +1,8 @@
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
+    rustls::crypto::ring::default_provider()
+        .install_default()
+        .unwrap();
     secret_vault::gcp::GcpSecretManagerSource::new("my-project").await?;
 
     Ok(())

Thanks for the reproducer, much easier to get, I'm not sure why the test don't trigger the bug since they also enable both features and should be triggerring SSL installation. Do you have ideas why we can't see the same behavior ?
It could be something because test compilation has different flags ?

Again, I stand still that your solution also makes it harder since I can't really use aws-lc-rs without implementing the crypto provider.

But I don't care one way or the other which crypto backend is used, ultimately it is up to users to care about this.

Here is another attempt which fixes your issue without forcing any implementation :#88
So whenever this passes: seanmonstar/reqwest#2423 then at least we will seamlessly change too.

Would this work for your use case ? This remove the rustls dependency altogether from this crate and it just transitively let other crates check. I'm not sure why we added rustls in the first place.

Again, I stand still that your solution also makes it harder since I can't really use aws-lc-rs without implementing the crypto provider.

My solution brings it in line with all the other crates in the ecosystem. aws-lc-rs can easily be chosen by adding the rustls dependency on user side, enabling the aws-lc-rs feature and setting the provider. But currently hf-hub forces this decision onto the user.

Here is another attempt which fixes your issue without forcing any implementation

Enabling reqwest/rustls-tls already forces the ring implementation. But that is expected. The unexpected part is, that hf-hub additionally forces aws-lc-rs by depending on rustls with default features enabled.

Would this work for your use case ?

Yes, removing the direct dep on rustls would solve the problem as well. If that works, I think it is the better solution!

Closing in favor of #88 which also removes the implicit aws-lc-rs dep, letting either reqwest or ureq choose the default.

Thanks for fixing this! Is there a release process for hf-hub? Not sure if you think this change is worth to do a release for, but I'd like to avoid git dependencies in my Cargo.toml file :)