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

upgrade jgit library #182

Closed
maks opened this issue May 26, 2016 · 10 comments
Closed

upgrade jgit library #182

maks opened this issue May 26, 2016 · 10 comments
Assignees
Milestone

Comments

@maks
Copy link
Collaborator

maks commented May 26, 2016

Currently using jgit:3.6.2.201501210735 whereas maven central already has available jgit:4.4.0.201605250940-rc1

@maks maks self-assigned this May 26, 2016
@maks maks added this to the v1.3.2 milestone May 26, 2016
@bigdavedev
Copy link
Contributor

Bad news! JGit requires Java 1.7 and, while Android does say they are JDK7 compatible, they are not 100% compatible. File.toPath has not been implemented in libart-core.jar, thus JGit 4.0+ will not work. The best we can do is upgrade to the latest 3.x branch (short of sending a patch to AOSP).

@maks
Copy link
Collaborator Author

maks commented Jun 2, 2016

@bigdavedev thanks for looking into this and the PR.
I'm happy to move up at least to 3.7 for now.
With JGit 4, if File.toPath() is the only incompatibliity, I'd think submitting a patch to jgit may be a good approach as then we could maintain a temporary fork of jgit with it and once (if?) the change is accepted upstream, then everyone would be able to make use of it, while even if a patch does go into AOSP it would only be devices that receive updates (post-N at this stage) that would ever be able to make use of it.
What are your thoughts?

@bigdavedev
Copy link
Contributor

Interesting. While doable, it presents a challenge, since File.toPath() returns an instance of Path which doesn't exist at all in Android. I can't figure out what the alternative would be. I'll dig a little and see what I come up with.

@maks maks modified the milestones: v1.3.3, v1.3.2 Aug 4, 2016
@maks maks modified the milestones: v1.3.4, v1.3.3 Aug 26, 2016
@maks maks modified the milestones: v1.3.4, v1.3.5 Sep 26, 2016
@henrik242
Copy link

You can just add a back-ported java/nio/file/Path to SGit, or submit a patch to JGit...

@maks
Copy link
Collaborator Author

maks commented Sep 27, 2016

@henrik242 thats what I thought at first too, but unfortunately its not that simple - turns out its actually the usage of Path in File that is the issue and as per my earlier comment the only way to do this now is to patch jgit to not use it and then porting in Path or just not using it at all.

@henrik242
Copy link

henrik242 commented Sep 27, 2016

@maks OK, makes sense. I tried back-porting the File.toPath() usages here: henrik242/jgit@9b3ef34

It builds (with -Dmaven.test.skip=true), and SGit doesn't complain about toPath anymore. But it has problems creating folders (at least in my emulator), e.g.:

09-27 12:00:03.495  1382  1398 W ContextImpl: Failed to ensure /sdcard/Android/data/me.sheimi.sgit/files: java.lang.SecurityException: Invalid mkdirs path: /storage/self/primary/Android/data/me.sheimi.sgit/files
09-27 12:00:03.495  1382  1398 D maks    : PRESET repo path:/repo/stickmap
09-27 12:00:03.519  1382  1382 W System.err: org.eclipse.jgit.api.errors.JGitInternalException: Creating directories for /repo/stickmap/.git failed
09-27 12:00:03.519  1382  1382 W System.err:    at org.eclipse.jgit.api.InitCommand.call(InitCommand.java:118)
...
09-27 12:00:03.521  1382  1382 W System.err:    at java.lang.Thread.run(Thread.java:818)
09-27 12:00:03.521  1382  1382 W System.err: Caused by: java.io.IOException: Creating directories for /repo/stickmap/.git failed
09-27 12:00:03.530  1382  1382 W System.err:    at org.eclipse.jgit.util.FileUtils.mkdirs(FileUtils.java:346)

It's probably related to symlink handling. Or something.

@maks
Copy link
Collaborator Author

maks commented Sep 28, 2016

@henrik242 thanks for working on this! That's a great start. And it looks like the changes required are not too big.
I think given that these are runtime errors, it would be a good idea to get the jgit unit tests working with any changes to give confidence that nothing else like this gets broken as a side effect and that would also be a prerequisite for submitting a patch back to the the eclipse maintainers too.
I'm a bit tied up with other things at the moment but I'll pick this up again in a couple of weeks time if you don't get to it first.

@henrik242
Copy link

@maks I don't think the eclipse maintainers will agree to support jdk6 again, so we'll probably have to maintain our own patch set.

But I fully agree that we should make the unit tests run properly.

@henrik242
Copy link

@maks We might try to go another route instead, by using NNIO: A Java NIO.2 Substitute Library

@maks
Copy link
Collaborator Author

maks commented Nov 2, 2016

This issue was moved to maks/MGit#26

@maks maks closed this as completed Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants