Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gate runtimes / TLS behind features #27

Closed
amesgen opened this issue Jan 11, 2020 · 9 comments
Closed

Gate runtimes / TLS behind features #27

amesgen opened this issue Jan 11, 2020 · 9 comments

Comments

@amesgen
Copy link

amesgen commented Jan 11, 2020

Currently, this crate is hard-coded to async-std and async-native-tls. There are other options for async runtimes and TLS libraries. Are you open to adding features to support them? I would be proposing sth similiar to async-tungstenite:

  • Async Runtimes: async-std-runtime, tokio-runtime, maybe gio-runtime
  • TLS libraries: async-tls, async-native-tls, tokio-tls, maybe tokio-rustls

I would be open to implementing this when I have time if you are not opposed.

Same thing for async-smtp.

@dignifiedquire
Copy link
Member

dignifiedquire commented Jan 11, 2020

In order to support other runtimes, I do not think we actually need to add any feature flags, only finishing #21.

For tls I think it would be nice to support async-tls, not sure about the need to support tokio-tls, given that async-native-tls is runtime agnostic already.

@dignifiedquire
Copy link
Member

But to nswer your question in a more general sense, I am very much interested in supporting different runtimes, though I do want to make sure that the maintenance overhead is not too high.

@amesgen
Copy link
Author

amesgen commented Jan 11, 2020

Ah, should have looked for existing PRs... Yeah, supporting tokio-tls and tokio-rustls makes not a lot of sense (except if the goal is to provide the ability to reduce dependencies if someone already depends on them, which is a very minor benefit IMO).

Apparantely both async-{imap,smtp} don't use Async{Read,Write} in their API (which surprised me, but streaming transport prbbly is annoying to implement), so it should be easier than in async-tungstenite where they have to convert between futures Async{Read,Write} and tokio Async{Read,Write}.

@dignifiedquire
Copy link
Member

The main trick is that async-imap uses async_std::net::TcpStream, but without requiring a specific runtime. This way we can use a single AsyncRead/Write trait under the hood, and simply let another runtime drive the execution. This is of course from a purist perspective not ideal, but it should solve most practical use cases

@dignifiedquire
Copy link
Member

I really hope tokio-rs/tokio-compat#2 gets shipped eventually, as that would make it even easier to accept Tokio AsyncRead/Write sources into the mix for anyone.

@dignifiedquire
Copy link
Member

It seems gio uses futures@0.3 AsyncRead/Write as well: https://docs.rs/gio/0.8.0/gio/struct.InputStreamAsyncRead.html

Looking a bit more at async-tungestenite, we might want to add some more features down the line, like the differnt connect methods for the different runtimes, but it is unclear how much benefit that actually has for the end user.

@dignifiedquire
Copy link
Member

It might also be interesting to put that into its own crate extracted from async-tungestinte, something like async-tcp-stream, which allows establishing tls/tcp streams across the whole board, feature gated, and could be used in the different projects.

cc @sdroege

@sdroege
Copy link

sdroege commented Jan 11, 2020

For tls I think it would be nice to support async-tls, not sure about the need to support tokio-tls, given that async-native-tls is runtime agnostic already.

async-native-tls is not yet runtime agnostic unfortunately async-email/async-native-tls#10 :) But it doesn't look like it needs a lot of effort to do so. (Also loosely related, I wouldn't consider anything runtime agnostic if it still pulls in one of the runtimes and only e.g. uses traits of that... if those traits are generally useful they should become part of some runtime agnostic crate).

It seems gio uses futures@0.3 AsyncRead/Write as well

glib/gio only use the traits from std/futures, yes. Similar to tokio it is however required that any async operations are being called from inside the glib runtime, or otherwise it will panic and not work.

It might also be interesting to put that into its own crate extracted from async-tungestinte, something like async-tcp-stream, which allows establishing tls/tcp streams across the whole board, feature gated, and could be used in the different projects.

I think that's a good idea, generally we should probably try to get more runtime-agnostic abstractions (and get more of the abstractions we have into std). It's not great that basically any library crate has to decide on one single async runtime or invent its own set of abstractions. This basically brings us to the current situation where half of the ecosystem depends on tokio and the other on async-std and you have to select your dependencies very carefully unless you don't care about the bloat and resource waste caused by pulling in two runtimes (and juggle them both without conflicts). This current situation is rather unsatisfying, at least for me.

That said, such a abstraction crate would have the problem that cargo features are currently rather useless for anything that is not exactly "additive". Either you would have to do it like async-tungstenite and have features enable separate modules per runtime, or you have non-additive features that select the implementation of your specific API and once your crate is pulled in from two places with different features everything falls apart. Maybe someone solved this already but it seems like a missing feature in cargo.

@link2xt
Copy link
Contributor

link2xt commented Aug 28, 2023

TLS is now moved outside of async-imap, the library allows to run IMAP over whatever stream you have. As for runtimes, tokio and async-std are supported and if someone wants to add another runtime PRs can be submitted.

@link2xt link2xt closed this as completed Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants