Rationale for deprecation?

[harmic]

I saw that this package was deprecated a few days ago in favor of async-ssh2-lite. Since I am using async-ssh2 I thought I had better check the other package out - but I found it distinctly under-cooked.

Some examples:

• The AsyncFile implementation merely implements AsyncRead/AsyncWrite, but does not expose any of the methods of the underlying libssh2 file, such as stat, readdir, etc.

• The sftp readdir method does not work at all for me - it either hangs or returns an empty list. When I inspected the source, it just calls the inner readdir within the Async write_with callback - I can't see how that is ever going to work.

• In fact almost all the methods do something similar, even though many of them involve writing a command to the ssh connection, then waiting for a response which has to be read. The 'lite' package seems simplistically to assume that once a command can be written, the response will be available for reading.

At least for me this package seems a lot more capable than the one you are deprecating it in favor of, so I am wondering what the rationale for deprecation is?

(I was also confused as to why/how the commit which deprecated this package came from some user called 'bold' who is not a contributor and I can't even click on their name to see who they are ... that might just be my lack of experience with github though).

[spebern]

Hi,

First of all thank you for this comment.

I am not really using async-ssh2 and all I did regarding testing is assuring that the ssh2 test suite passes in an async context. When I had a peak at the lite version it looked very similar while keeping the external dependencies up to date and I currently lack the time of updating the dependencies of this package. Async libraries are updating really fast and I did not see the point of having two libraries that are so similar.

Now that I see your comment I am rethinking the decision to deprecate the package. Maybe I'll find some time to update the external dependencies.

(regarding your confusion the bold account is also my account)

[harmic]

Thanks for the reply.

I have a fork over here where I have been trying to update the dependencies - see branch async-io. I am a few commits behind you.

I've taken the approach of trying to depend on async-io instead of smol, since you are currently only depending on the re-exported async-io anyway.

Unfortunately it is not working right now - I am finding that it hangs on sftp file reads, for one thing.

I suspect part of the reason is that with_read and with_write is being used to determine when to call into the libssh2 functions: in many cases libssh2 could be waiting for a read or a write, depending on protocol state, so we might be using read_with but libssh2 actually wants to do a write. For example, when doing an sftp read operation, libssh2 has to send one or more read requests to the server, and only then can it wait for the responses. If we use read_with it may never get the chance to send the requests.

I raised smol-rs/async-io#49 to ask the smol team how that is supposed to be handled.

There might be more to it than that though - I have tried working around that by making my own equivalent to the old with method, but that is also hanging.

If I ever get it all working I will send you a PR.

In regard to the deprecation: it would definitely be a good idea to have a single async interface to libssh2 rather than multiple, as long as it's fully functional! The lite crate does seem to be getting some commits, but it looks to me like it is starting from a long way back.

[harmic]

I've done quite a bit of work on this over the last week or so, and resolved many of the issues:

• Refactor to depend only on async-io, which reduces the amount of code imported in case you are using async-std or tokio in place of smol
• Update to latest version of dependancies
• Call block_directions to find out which direction libssh2 is actually waiting on, rather than blindly assuming.
• Rewrite the AsyncRead and AsyncWrite implementations, which I eventually found were the ultimate cause of the hangings I was having before. Previous code called with_read or with_write (or earlier, just with) which returns a future, and then called poll_unpin on that future. The problem is that resulted in a single poll being performed; after that the whole future is dropped.

I ended up undoing the changes made to Listener::accept in #25. That change was to switch the implementation of accept into a polling loop with a 10ms delay. I don't understand the rationale given in the original fix (that the object you need to poll is on the remote server) - the way it works is that the remote server does the accept, then then sends us an ssh packet containing the result of that, so it is still just a case of waiting for the remote server to send something. If there were problems with the original implementation, I suspect they were to do with the fact that we may not have been waiting on the correct direction.

I will send you a PR shortly with my changes, let me know what you think.

[harmic]

Hello @spebern

Any chance to review my PR #29 ?
If you don't have the time right now - would you perhaps consider adding me as a co-maintainer of this package?

[spebern]

Hi,

I added you as a co maintainer. What's your username on crates.io so that I can add you there as well?

[harmic]

Hi, Thanks for that! It's the same as my github account - harmic. Cheers Mike

…

On Sun, 6 Jun 2021 at 19:44, bold ***@***.***> wrote: Hi, I added you as a co maintainer. What's your username on crates.io so that I can add you there as well? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#27 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBQWCYPBK2KDMMUUP27N6LTRM7Q5ANCNFSM4U3PQESQ> .