Git sync multiple improvements #1037
Conversation
N.B. Requires API >= 26. Apache MINA SSHD is now the standard SSH transport library for JGit. It supports more modern key algorithms than Jsch. I have added a notification prompt to the user upon new or unexpected SSH server host key. I have no idea what I'm doing, but it seems to work.
A generated key can optionally be protected by biometric auth or device credential. This makes it harder to steal, but will obviously not play well with auto-sync. The default key type is EDCSA. ED25519 keys are faster, but not supported natively by the Android key store. The methods currently called when unlocking a ED25519 key do not respect the validity duration setting, which means that the key needs to be unlocked before each use. This may be twice during a sync, if we need to both fetch and push. RSA and EDCSA keys respect the validity duration setting, meaning we leave them unlocked for 15 seconds. A way to speed up Git syncing while requiring auth upon each key use would be to use SSH multiplexing and keep the SSH session open until we push (or decide not to push). I raised the minimum SDK version from 21 to 23. Otherwise we cannot include android-crypto in the manifest. N.B. Much of this code has been taken and re-worked from https://github.com/android-password-store/Android-Password-Store. That project is also GPL-3.0, but I don't know how to properly attribute those authors in the Orgzly code base.
- Make "git push" run blocking. Mostly to make the "syncing in progress" status information more reliable. - Show more informative snackbar when "git push" goes wrong. The user will now find out if pushing fails because of e.g. missing write permission. - Remove two unused methods and an unused return value. - Tidy up some variables.
The TransportException class path was wrong, so the exception type was not being recognized. Make an attempt to find the cause if JGit throws the generic "remote hung up unexpectedly" exception.
We were always ending up in RepoCloneTask. Separate two different error cases, so that we can actually distinguish between them.
Also, use more specific inputTypes. Also, fix the branch name always showing as the default value
This allows the user to easily set up a new "origin" remote (by initalizing an empty repository) if the current one is somehow lost. A Git repo which is being synced between two computers should provide a degree of redundancy. Without this code, there was no simple way to sync the repo contents using Orgzly as the source.
The method had several unused variables and seemingly pointless logic, considering how it is used. The method is only called in the current scenarios: - A remote book has no corresponding local book - When force loading a book from the repo Hence, I see no reason to "safely" retrieve the latest version of the book from the repo. We know that we just want the latest version from the repo.
This method seems to be very expensive, we probably need to optimize this stuff.
This may explain why the local repo kept growing quickly and temporary branches were not being deleted. Also, add some more exception handling, debug logging and elaborate error messages.
We should be able to trust the namesake status. If there are only local changes, there is no need for complex two-way syncing. The handleTwoWaySync method call resulted in the local notebook always being re-loaded into the database from the Git repo, even though we just wrote that version ourselves. This was a frustrating user experience, since each sync of local changes resulted in all notes objects being replaced in the database, which made it impossible to initiate sync and keep on adding notes in the same book. If there are only local changes, we now use the dataRepository::saveBookToRepo method, just like other repo types. The only drawback of this change is that we have to "git push" in the GitRepo::storeBook method, since it is also used during force save of a book. This means that the GitRepo::tryPushIfHeadDiffersFromRemote is later run in vain, as it is run after each syncing of a Git repo. No push will be performed, though, and I have now removed the pointless looping over remote branches which I once added.
If nothing has changed in the Git repo, we can save a lot of sync time by reading vrook information from the database.
|
Hey, I tried your apk whic you graciously provided but I am able to generate a key but when i go to repo option to set up the git option. Upon touching it the application carshes Devive: oppo reno 6 5g [Not rooted] |
|
@Xanaxus Yeah, I get the same behavior now... I will take a look at it. |
|
@Xanaxus I found the issue. New APK: https://koloni.info/orgzly-fdroid-debug-amberin-8e583282.apk |
|
Hey it works!! |
|
Indeed. Thank you so much @amberin 🙏 I've changed application ID in my fork (so that one can run official Orgzly and yours) and also enabled GitHub actions in fork and the APK is built by GitHub: https://github.com/wiktor-k/orgzly-android/actions/runs/4466053393 (Currently the workflow file builds artifacts only for Thanks again and have a great day! 👋 Edit: ugh, changing package name was not a good idea as it's hardcoded in the app in many places and the app crashes here and there :-/ |
This build definitely works. I'm running it right now without problems. Thank you so much! |
Hi @Xanaxus. I've just installed your APK and I get an error when I want to add a Git repo:
The rest of the error message is truncated unfortunately :( |
Hey I just tried it and the apk still works though the main apk is yet to.... the error you mentioned is because u havvent added the ssh key to you git account |
All I know is that he made the last commits on master in Nov 2022. I'm afraid I am too inexperienced and lack the time to maintain a fork of such a mature project. But I agree with you that a fork would be desirable if Nevenz does not reappear... |
|
I need this code merged now! :c |
I can't tell if you're saying you approve or don't of a fork. I think it could be done in an unobtrusive way such that if @nevenz reemerges he can sort of take control again. It's just that at this point there's a ton of stuff outstanding that needs to be fixed. |
|
@colonelpanic8 Edit: Actually i think that make a fork just for publish "temporal" versions as github releases can be enough. |
|
Fast question, @amberin Have you tried the ssh key generation in a release build? I got the error message: Error while trying to generate the SSH key pair Message: "ia.m" after the biometric authentication. I think it can happen because the minify/shrink or proguard config because i put the release config in the debug buldType and starts to show the error: Error while trying to generate the SSH key pair Message: "org.apache.sshd.common.config.keys.KeyUtils" |
@Nojipiz I have not. So you may have found a bug. |
|
I can volunteer to be part of a committee that maintains a fork of orgzly and tries to merge and release some of the pull requests that are seen as most urgent. What do people think about this? Just going off of the contributor information that I didn't look at to closely, it seems like @amberin @pxsalehi @tulth @maikol-solis and myself are probably in the best position to do something like this. Obviously, none of us are going to know the codebase as well as @nevenz and I'm happy to relinquish any sort of control the moment he shows up. anyway, I've created https://github.com/orgs/orgzly-community to enable this project. Please let me know if you think I've left anyone obvious out, or if there is information that I'm not aware of about @nevenz status. |
Thank you @colonelpanic8, that sounds excellent to me. I am currently very pressed for time, myself, but I would be happy to join such a committee, and it seems there are a bunch of people willing to contribute code. |
yeah I mean, thats the thing, I'm also very pressed for time, but I think I can do a reasonably good job coordinating merges and making sure they are of a reasonable enough quality with some help from some other people, and I think its important that we keep the ball rolling before all enthusiasm dies out. |
|
I will create some issues in the new repo about stuff that's been bothering me, just to get the ball rolling. 😀🚀 |
|
Sorry for posting here as I don't know what the least intrusive way would be to ask this: how do I compile the new APK? The generate APK button in the actions page produces the error: "The logs for this run have expired and are no longer available." |
yeah, for sure! |
Actually the repo already contains workflow files needed to produce that file. I think I had to press some buttons in the repo config to enable actions and they worked. Example from my ancient repo: https://github.com/wiktor-k/orgzly-android/actions |
This is one PR gathering all of my unmerged Git-related changes. I will close the others.
I am opening this because I now have some more work to share, and I have done some rebasing of changes in the other unmerged branches.
As before, anyone wanting to run this code must also merge #916, as local storage permissions are broken without it.
I will happily split this PR into smaller pieces, if needed, once someone gets around to reviewing it.