Bug 553625 - Remove Jcraft dependency as a requirement
Summary: Remove Jcraft dependency as a requirement
Status: RESOLVED FIXED
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: 5.6   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 5.8   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-29 14:54 EST by Ashley Abdel-Sayed CLA
Modified: 2020-06-05 16:57 EDT (History)
5 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ashley Abdel-Sayed CLA 2019-11-29 14:54:49 EST 
Our project is exclusively using the SshdSessionFactory and never using using the DefaultSshSessionFactory and so we were wondering if it would be possible to remove the need for the JCraft dependency. Thank you!
Comment 1 Thomas Wolf CLA 2019-11-30 04:37:05 EST 
Maybe in a future major release 6.x. I don't think it's possible without breaking API.
Comment 2 Emmanuel Hugonnet CLA 2019-12-02 08:58:38 EST 
I was wondering if we could use ServiceLoader and provides the META-INF property in a separate artifact that would point to DefaultSshSessionFactory thus we could simply exclude this artefact and use one with the Mina ssh factory.
Comment 3 Emmanuel Hugonnet CLA 2019-12-02 09:01:54 EST 
The service property file could even be in jgit core artifact.
Comment 4 Michael D CLA 2020-01-20 02:40:09 EST 
My change does this, but yeah minor API break.

https://git.eclipse.org/r/156153
Comment 5 Emmanuel Hugonnet CLA 2020-01-20 03:05:49 EST 
My ServiceLoader patch doesn't suit you ?
Comment 6 Michael D CLA 2020-01-20 04:28:53 EST 
I mean yeah, but it’d be nice to have the ssh implementation completely decoupled from jgit core by default (as my change also does with bouncycastle).
Comment 7 Emmanuel Hugonnet CLA 2020-01-20 04:49:42 EST 
Ok, I was wondering about retro compatibility. But this level of decoupling is really nice. Thanks a lot
Comment 8 Eclipse Genie CLA 2020-01-25 22:09:14 EST 
Gerrit change https://git.eclipse.org/r/155782 was merged to [master].
Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=54b1c7cc6a2ba6031b97d8a5c455d93ffd4ce5f5
Comment 9 Thomas Wolf CLA 2020-01-27 06:44:52 EST 
Using a service loader is one thing, but the maven/bazel/OSGi dependencies are still there. Could they become compile time / test dependencies only? 

If so: what if jsch is not present and the default instance is attempted to be used? Looks to me that one would get an NPE. Should JGit produce a more meaningful error in this case?
Comment 10 Michael D CLA 2020-01-27 17:01:42 EST 
Thomas, I believe it will actually throw a relatively clear ServiceConfigurationError not an NPE.

& My change set https://git.eclipse.org/r/156153 completely decouples it from JGit core so its not even a compile depend
Comment 11 Eclipse Genie CLA 2020-05-23 05:09:02 EDT 
New Gerrit change created: https://git.eclipse.org/r/163454
Comment 12 Eclipse Genie CLA 2020-06-01 18:36:39 EDT 
Gerrit change https://git.eclipse.org/r/156153 was merged to [master].
Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=8d2d683655e2de17cf465fa46af10e0e56b3aaed
Comment 13 Eclipse Genie CLA 2020-06-05 16:57:04 EDT 
Gerrit change https://git.eclipse.org/r/163454 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=165f8231d43ea406a452c894861679e9ab29ac3c