Add 'h1-client-rustls' feature relying on rustls

[JEnoch]

This PR is a follow-up of http-rs/http-client#53 and addresses #40.
It adds a h1-client-rustls feature using http-client/h1_client_rustls feature introduced by http-rs/http-client#53.

TODO:
For testing purpose I set the dependency to http-client to directly use its Git repo.
Once http-client is released with http-rs/http-client#53, update the dependency to use the latest version.

[JEnoch]

CI failures for curl-client feature are caused by http-rs/http-client#57.
The workflow should be re-started after merge of http-rs/http-client#58

[Fishrock123]

I will look more at the failures soon, but I think this would perhaps be better named as "h1-rustls-client"?

Ideally I'd like to separate out the tls backend flags... hyper has the same thing, I think. I haven't been able to figure out a good way to do that though...

[JEnoch]

@Fishrock123 I don't see such separation in hyper's features.
And I'm not sure how to achieve this:

• if h1-client feature is selected, http-client/h1_client feature is used, that depends on async-native-tls (and thus on OpenSSL)
• but if in addition a rustls feature is selected:
  • either the http-client/h1_client has to be replaced with http-client/h1_client_rustls
  • either you keep the http-client/h1_client usage, adding a usage of a new http-client/rustls feature that would replace the dependency to async-native-tls with a dependency to async-tls

I don't think Cargo provides a way to declare such "feature replacement in case of another feature usage".

Thinking further, a solution could be such reorg in http-client features:

[features]
default = ["h1_client", "async-native-tls"]
h1_client = ["async-h1", "async-std"]
h1_rustls = ["async-tls"]

And re-exposing the same in Surf.

A user wanting h1 + rustls would use such dependency:

surf = { version = "2.1.0", default-features = false, features = ["h1-client", "h1-rustls"] }

The drawback is that using h1-client as only feature will fail to build since a TLS impl is missing. This would be a regression wrt. current behaviour.

What do you think ?

[Fishrock123]

I don't see such separation in hyper's features.

reqwest, the hyper client wrapper, has this through. Their feature flags are pretty comprehensive. Maybe there's something to learn from them:
https://github.com/seanmonstar/reqwest/blob/bd9ff9f371b756decf26fbbde1687433b0f63774/Cargo.toml#L30-L41

[Fishrock123]

Thinking further, a solution could be such reorg in http-client features:

I also think we should re-organize http-client features. It's a big reason i never did another http-client release yet, because I wasn't sure what to do.

The drawback is that using h1-client as only feature will fail to build since a TLS impl is missing. This would be a regression wrt. current behaviour.

This actually isn't so bad, we can make http-client compile without tls support if nothing is provided.

Ideally, I think, http-client would have native-tls and rustls features, but this is a problem because cargo has no "join" on features e.g. make h1_client+native-tls pull in something different than maybe curl_client+native-tls. But maybe this isn't an issue if both hyper and isahc just use the same tls implementations as us anyways?

[Fishrock123]

I have created a zulip thread about this to discuss with the cargo team... https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/co-dependant.20feature.20flag.20dependencies

[Fishrock123]

Maybe we just need to stick with h1-client-rustls for now though... this is also blocking nlopes/libhoney-rust#61

[Fishrock123]

@JEnoch mind to rebase this?

[Fishrock123]

It seems like rust-lang/cargo#8832 will eventually allow h1-client+rustls-tls to work, but no timeline on when.

[JEnoch]

@JEnoch mind to rebase this?

Sure! Doing this shortly.

[JEnoch]

Note that this PR relies on main branch of http-client (for http-rs/http-client#53 and http-rs/http-client#58).
You might want to release http-client before merging this PR. Just let me know and I'll update the Cargo.toml.

[Fishrock123]

I've released http-client 6.3.0

[Fishrock123]

Merged a working set of features as #292, which is more like the original without my poor suggestion on feature separation (which is not yet supported by cargo in the way which we'd need).

Thanks for your work and patience here!