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

Compatibility features #269

Merged
merged 6 commits into from
Sep 2, 2016
Merged

Compatibility features #269

merged 6 commits into from
Sep 2, 2016

Conversation

solind
Copy link

@solind solind commented Sep 2, 2016

These are the remaining changes I had to add to sshj for compatibility with Joval's jSAF provider code. They're all relatively minor so I hope it makes sense to group them together:

  1. Re-wrote try-with-resources and inferred generics constructors (no functional changes) so that the project can be built using Java 6.
  2. Added a method to the Channel interface to make it possible to determine whether an EOF has been received.
  3. Added some missing SFTP response codes.
  4. Fixed a typo in an SFTP enum.

David Solin and others added 4 commits August 26, 2016 13:02
…AF provider.

1) Added boolean Channel.isEOF() method, for checking whether the server has sent EOF.
2) Added SFTP response constants to the Response enumeration
3) Spelling correction for the SYMLINK FileMode enum constant
@hierynomus
Copy link
Owner

Hi David, unfortunately Java6 compatibility is a no-go, as per #234, the ed-25519 library we depend on is java7 only.

@solind
Copy link
Author

solind commented Sep 2, 2016

Interesting. I actually compiled the ed25519-java project using Java 6, made a Java 6 build of sshj and it worked fine. (We don't allow build frameworks to pull down dependencies for production builds, we compile everything from source).

@hierynomus
Copy link
Owner

Hi David,

Unfortunately the ed25519-java project is released only with a Java7 build. So I also only support java7. Also because Java6 is EOL, and Java7 is also officially EOL-ed by Oracle, see: http://www.oracle.com/technetwork/java/eol-135779.html

So for me, adding back in Java6 support is a step backwards, also because my CI builds run with java7+, and do not test for java6 compatibility anymore. So there is absolutely no guarantee that these changes will be kept in.

@solind
Copy link
Author

solind commented Sep 2, 2016

I know, they're dinosaurs, but I think we have customers who are still using it. I see it more on AIX and old versions of Linux using OpenJDK (not Oracle's VM).

How about this, what if I take responsibility for any Java 6 compatibility work? Feel free to break Java 6 compatibility and if it bothers me, I'll fix it? You won't have to worry about it, and the project can officially still require Java 7.

(Obviously though, you'll have to leave me the option of eventually giving up on trying to maintain compatibility.)

@hierynomus
Copy link
Owner

Hi David,

I had another quick look at the code. Essentially the ed25519-java library indeed is released with Java6, however the VRallev/ECC-25519 library is released with Java7. The latter one bundles the former one.

Looking through the code. I just use the djb.Curve25519 class from that library. Which is a Public Domain class.

What I would propose is that we can add back Java6 compatibility by including the djb.Curve25519 class in the codebase, and depending on the net.i2p.crypto:ed25519-java library. I'll fix this momentarily and then with your changes I'll re-add java6 to the compilation targets on the TravisCI server.

Sound like a plan?

@solind
Copy link
Author

solind commented Sep 2, 2016

Sounds awesome! Thanks!

@@ -394,6 +401,7 @@ private void gotEOF()
/** Called when EOF has been received. Subclasses can override but must call super. */
protected void eofInputStreams() {
in.eof();
eof = true;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow your indenting is off sometimes ;)

@hierynomus
Copy link
Owner

Can you merge in master so that the checks are green ;)

@solind
Copy link
Author

solind commented Sep 2, 2016

I'm sorry man. I actually write code in vi (speaking of dinosaurs), and I normally use 1 tab for 8 spaces.

@hierynomus
Copy link
Owner

I think the right dinosaur settings are:

set autoindent
set smartindent
set expandtab
set shiftwidth=4
set tabstop=4

😉

@hierynomus hierynomus closed this Sep 2, 2016
@hierynomus hierynomus reopened this Sep 2, 2016
@hierynomus
Copy link
Owner

Sorry clicked wrong button there... If you can get the branch up-to-date then we're good to go!

@solind
Copy link
Author

solind commented Sep 2, 2016

I think I just did. :)

@hierynomus hierynomus merged commit 766d292 into hierynomus:master Sep 2, 2016
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

Successfully merging this pull request may close these issues.

None yet

2 participants