maks / MGit

A Git client for Android.
https://manichord.com/projects/mgit.html
GNU General Public License v3.0
1.23k stars 166 forks source link

upgrade jgit library #26

Open maks opened 7 years ago

maks commented 7 years ago

From @maks on May 26, 2016 4:56

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

Copied from original issue: sheimi/SGit#182

maks commented 7 years ago

From @bigdavedev on May 30, 2016 19:17

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 commented 7 years ago

@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?

maks commented 7 years ago

From @bigdavedev on June 3, 2016 5:9

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 commented 7 years ago

From @henrik242 on September 26, 2016 8:50

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

maks commented 7 years ago

@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.

maks commented 7 years ago

From @henrik242 on September 27, 2016 9:57

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

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 commented 7 years ago

@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.

maks commented 7 years ago

From @henrik242 on September 28, 2016 9:44

@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.

maks commented 7 years ago

From @henrik242 on October 25, 2016 14:53

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

kb-1000 commented 6 years ago

We also could try retropiler.

maks commented 6 years ago

Thank you for the link, yes that does look very interesting and I think might be just what we need for this.

maks commented 4 years ago

According to todays Google Play stats, currently 80.3% active users of MGit are on Android 8 (API26) or higher. Given that, it now becomes possible to consider choosing to do ship newer versions of MGit with latest versions of JGit, keeping existing version for users with Android 7 and lower.

aivanovski commented 3 years ago

@maks Do you have any updates since? I've tried to update JGit in the project to 5.13 by myself (just to have LFS support for my personal needs) and it was easy. In order to compile the project, I've changed only a couple of files. But, as a result, I haven't been able to clone LFS repository successfully. Repository has been cloned but without lfs support. Meanwhile, I've tried to execute code from me.sheimi.sgit.repo.tasks.repo.CloneTask#cloneRepo() on Desktop Java with the same libraries and it works exactly as it should, I mean the result of cloning is a valid LFS repository. So, I try to figure out why JGit doesn't work on Android and why it works on Java. Do you have any advice for that? I haven't been able to see any errors in a log, probably I will be able to find something in a debugger, but who knows how much time it take(

maks commented 3 years ago

@aivanovski if you are using Android 8 or newer it should work but older than that, Android is missing Java APIs. However I haven't kept tabs on newer version of JGit so it could be they have started using other newer Java standard library APIs that are not available in even newer versions of Android.

aivanovski commented 3 years ago

Small update. Unfortunately JGit is not a "pure" Java git client implementation, because under the hood it may execute system binaries and without those binaries it will not work. For example, in case of cloning LFS repository, JGit will run "git-lfs smudge ...." command on my Desktop machine, and without 'git-lfs' binary it won't work. (it happens here: org.eclipse.jgit.dircache.DirCacheCheckout#runExternalFilterCommand() and requires several lines in .gitconfig file) So, probably that means, that LFS support can't be implemented on Android with JGit.

maks commented 3 years ago

Good to know though JGit originally was a pure Java git implementation. I suspect that it's only for LFS that this was done as LFS is a relatively recent addition and still not widely used so they took the easy option of not implementing it in Java.

kb-1000 commented 3 years ago

There's an additional LFS module as far as I know. Anyways, updating JGit isn't as easy as you may think... It requires APIs introduced in Android 7 or 8, crashing on all older Android versions (It's not like I didn't already try that). In addition to that, there needs to be support for the new file access API (I think MediaStore?), else you can't update the targetSdk without breaking new devices, although I'm not even sure if using an older targetSdk would change that on those devices.

KOLANICH commented 3 years ago

It requires APIs introduced in Android 7 or 8, crashing on all older Android versions (It's not like I didn't already try that).

Thanks for the info. I have modified the sources of MGit to build it for Kit-Kat (and also modernized the configs to use the latest gradle), but it still crashes (but currently within mgit, while trying to serialize fragment state, because a fragment has a Repo object of jgit, which is not serializeable). Unfortunately I cannot even build the old versions that have officially supported kit-kat using the modern toolchains. Bunch of errors from aapt2.

maks commented 1 year ago

Looking at the most recent stats from Google Play, around 90% of MGit users are now on Android 10 or newer so it's now feasible to look at updating the version of jgit used in MGit.

I've done some quick tests and while jgit 6.x requires at least 1 Java std lib API that is only available in Android 33 onwards, jgit 5.x seems to be fine on Android 10+ so that would be a big step forward already in easily modernising the jgit used.