search

rustls / tokio-rustls

Async TLS for the Tokio runtime
Apache License 2.0
125 stars 70 forks source link

Proposal: Share code between tokio-rustls and futures-rustls #58

Open jbr opened 8 months ago

jbr commented 8 months ago

Currently, tokio-rustls and futures-rustls share very similar code duplicated across two different repositories. Every PR that is opened to one of them must be manually duplicated in other in order to ensure parity. As it currently stands, the two test suites have diverged and futures-rustls has significantly less coverage.

It seems like it would make maintenance easier if the two crates shared as much implementation code as possible. I could imagine a few ways of doing this, and would happily contribute any of them.

The easiest path forward would probably be introducing a third crate that has feature flags for tokio and futures. The two existing crates would reexport everything from the third crate, allowing users to continue depending on tokio-rustls and futures-rustls as is, but they would be light shims on top of the unified crate.

If the smol team were willing to part with it, the common crate could take the name async-rustls and the shim crates of tokio-rustls and futures-rustls could be deprecated in preference to just setting the desired feature flags on the new common-implementation async-rustls.

If all three crates were in a shared workspace, as an additional distinct step from the above, CI could ensure that every change to the shared implementation crate worked correctly with both flavors of async trait. The challenge here would be retaining history, but it seems like the histories of the two repositories are very similar, so someone looking into the past could find answers just as well by looking at tokio-rustls' history if the futures-rustls repository were archived

quininer commented 8 months ago

Simply adding features is not feasible, see https://github.com/tokio-rs/tls/pull/116#issuecomment-1206180561

I've tried support both tokio-io and futures-io, which means there are two ways to implement:

  1. Repeat most of code, because AsyncRead/AsyncWrite traits are used from TlsStream to common::Stream
  2. Define our own AsyncRead/AsyncWrite trait

I don't want to go either way, so I'd rather keep things status quo. i don't think futures-rustls is a maintenance burden, since most of code is easily migrated.

Also you don't need to worry about futures-rustls forget to update, I believe historically futures-rustls has a better maintenance record than any futures-io fork of tokio-rustls.