ymnk / jsch-agent-proxy

Other
70 stars 41 forks source link

Add module with support for (patched) Trilead SSH2 (SVNKit fork) #9

Closed charles-dyfis-net closed 11 years ago

charles-dyfis-net commented 11 years ago

This is, perhaps, more a request for comments than a patch submitted as ready for merge. That said, it has been tested end-to-end against the version of Trilead SSH2 at http://svn.svnkit.com/repos/3rdparty/com.trilead.ssh2/trunk/ with the patch given in the diff applied.

Is there interest in merging this? If so, what work would be required to make it acceptable?

ymnk commented 11 years ago

If possible, we want to include your contributions in the next release. Unfortunately, we are not familiar with Trilead ssh2, so it is helpful if there is an example like https://github.com/ymnk/jsch-agent-proxy/blob/master/examples/src/main/java/com/jcraft/agentproxy/examples/SshjWithAgentProxy.java to try it.

charles-dyfis-net commented 11 years ago

I'm going to try to get the corresponding patch merged to the Trilead tree (rather, SVNKit's fork thereof, which appears to be the only actively maintained one). Once that's done, I'll update the code here with any necessary changes, including an end-to-end example.

ymnk commented 11 years ago

I have been checking following issues,

http://youtrack.jetbrains.com/issue/IDEA-52034
http://issues.tmatesoft.com/issue/SVNKIT-178
http://issues.tmatesoft.com/issue/SVNKIT-375

but it seems there have been no responses and progresses.

charles-dyfis-net commented 11 years ago

I've heard back from TMateSoftware indicating that they intend to review the patches to Trilead SSH2 and SVNKit soon -- over the weekend if possible. The patch to IDEA is on hold until my employer's in-house legal staff has had a chance to review the contributor agreement JetBrains requires; it is separately tracked at JetBrains/intellij-community#79.

charles-dyfis-net commented 11 years ago

As implied by the commit message for 2368be8, the Trilead SSH2 patch is now merged into the SVNKit repo, and published as part of 1.0.0-build217.

ymnk commented 11 years ago

I have succeeded to run an example TrileadWithAgentProxy, but needed to add some dependencies to "examples/pom.xml" for executing "cd examples && mvn compile"

ymnk commented 11 years ago

I have not understood the reason why following two modules are required,

jsch-agent-proxy-svnkit-trilead-ssh2
jsch-agent-proxy-svnkit-trilead-ssh2-thick

Is it difficult to merge those into one module?

charles-dyfis-net commented 11 years ago

The -thick one is a separate module because it provides less control over dependencies -- the typical design of jsch-agent-proxy allows someone to only incorporate dependencies for communicating with SSH agent if only targeting UNIX, or to only incorporate dependencies for Pageant if only targeting Windows.

The advantage it provides is ease-of-use, and I expect that its behavior is desirable in a majority of cases, but I didn't want to deviate from convention by making it mandatory.

ymnk commented 11 years ago

Ok, I understand your motivation. I think -thick module may be useful for an example program, but for the real use-case it may not. For example, on Windows platform, NCUSocketFactory must be used for ssh-agent of cygwin environment. Anyway, I'll only add jsch-agent-proxy-svnkit-trilead-ssh2 module.

charles-dyfis-net commented 11 years ago

Do you really want every user of your software to need to have this real-world knowledge, and be separately responsible for which platforms their software works on?

I agree that jsch-agent-proxy-svnkit-trilead-ssh2-thick is not a particularly good idea (as it doesn't benefit folks using any library other than Trilead SSH2), but having some piece of code which pulls in all the dependencies and tries every appropriate method to find a working agent is going to lead to greater uniformity of platform support amongst jsch-agent-proxy users.

ymnk commented 11 years ago

The branch https://github.com/ymnk/jsch-agent-proxy/tree/issue9/support_for_trileadssh2 has been merged into master.