veeso / suppaftp

a super FTP/FTPS client library for Rust with support for both passive and active mode
Apache License 2.0
121 stars 31 forks source link

[BUG] - features are not additive #33

Closed OvermindDL1 closed 1 year ago

OvermindDL1 commented 1 year ago

Description

Features are not additive, for example, if a dependency uses the 'async' functionality then it breaks the sync functionality

Steps to reproduce

Enable the async feature and try using the sync API

Expected behaviour

Features should be purely additive, they should not break API's otherwise.

Environment

Additional information

Features should only ever be additive. One library that's being used uses it with the sync API, another is using it via the async, yet the async runner here is tokio and on initial testing it shows there's some issues with things because it appears to be hardcoded to async-std (which is not well used in the ecosystem at this time)?

veeso commented 1 year ago

Yes, I know, but there's no way to do that otherwise, so I don't care.

That's just a trip I mean. Why one should ever need both clients? I know the cargo reference says that, but it's also true that the language doesn't allow me to do otherwise. I mean, I could rename the stream to asyncstream, but idk.

OvermindDL1 commented 1 year ago

Generally they are alternate types (or tag types) for the different types of interfaces yeah. Most commonly the community and rust authors would tend to design something like this like FtpStream<IO> or so, where the different implementations of IO (a sync, a tokio, and an async-std generally, if anyone bothers with async-std nowadays) expose the relevantly useful API's for their own use. Then usually some pub type SyncFTPStream = FtpStream<SyncFTPIO> and TokioFTPStream and AsyncStdFTPStream as well, though not always.

That's just a trip I mean. Why one should ever need both clients?

Oh I know, the program here is split among a lot of libraries that are combined together into a single tower server. One is using the sync API because other stuff they are doing is forced sync on them so why not, and the other is using the async api, but it apparently is causing yet other issues because it's using the mostly-dead async-std instead of tokio for some reason? Trying to convince the async one to switch to the sync since can't seem to get suppaftp to use tokio and just put it behind a spawn_blocking or so like the other person but finding free time to do that is always fun.


But anyway, features must always be additive (sync feature, tokio feature, async-std features sound best for here? perhaps with default having sync but that can still be opted out of), everything about cargo assumes that and it will break things if it's not except for the most trivial of programs.

veeso commented 1 year ago

Okay then, I'll manage to fix all these things in the next version then. Thanks for reporting this info.