Improve Git Merge mechanism #763
Replies: 41 comments
-
|
I've replicated this on my setup. If you edit a "stale" note on the app, it will overwrite any existing change in the repo that applied to that same area of text. Does the app do the same commit && pull --rebase && push cycle that is recommended for the desktop client? This does appear to be limited to the case where it's the same "chunk" of text that's edited - if I make changes in areas that are separate (ie, that have an unchanged chunk of text for diff to lock to in between the changed bits) then it seems to work as expected. |
Beta Was this translation helpful? Give feedback.
-
|
The app currently does a merge and not a rebase. It additionally does a merge by chosing the local version over the remote. It doesn't even attempt to merge the files at all. I know, this is quite bad - this is something I definitely want to fix, and soon. Ideally there should be some smart merge, and configurable options on what to do when there is a conflict. I'm going to rename this issue to reflect that a proper merge mechanism needs to be implemented. This is easily going to take me 1-2 months, as I would like to solve it by improving my git library written in Dart, instead of using libgit2 for performing the merge. With libgit2, each call goes via Flutter -> Android -> NDK -> Custom Lib -> libgit2. This is very cumbersome. (I can make configuring chosing local or remote changes very easily, I'll open another issue for it) |
Beta Was this translation helpful? Give feedback.
-
|
I can definitely work around this and recommend working on other more important features before taking on this rabbit hole. I do think the way VS Code does a GUI merge is the best example to follow, when you get to it. A high priority feature should be to just provide a warning to the user. The work around could be to offer to create a copy of the local version before loading the incoming changes. |
Beta Was this translation helpful? Give feedback.
-
|
An honest fix is to detect the conflict and create 2 copies of the file, each with its own changes.
In most cases, especially in long notes, I won't know which I want to keep. I won't remember. Probably both. |
Beta Was this translation helpful? Give feedback.
-
|
@ovichiro there is no data loss. You just have to go into the git repo to retrieve the previous commit. |
Beta Was this translation helpful? Give feedback.
-
|
@ginkgomzd You're right. But a notification is needed in this case, as you mentioned, so one would know there was a conflict. Otherwise, if I figure it out 2 weeks later, it might be cumbersome to dig through all the diffs. |
Beta Was this translation helpful? Give feedback.
-
|
Could everyone interested in this please vote on this issue. I can accordingly prioritize it. The simple options of -
These both can be done right now. A more complex solution will require more time. |
Beta Was this translation helpful? Give feedback.
-
|
I'm ambivalent but I think saving the local changes to an obviously named new file is slightly better because I think you could manually merge the changes without leaving the app (basic copy-paste). The other way, if you realize you want to merge the changes, you have to choose to keep the local changes, and then go into git to view the remote changes and decide if you want to restore them. |
Beta Was this translation helpful? Give feedback.
-
|
I think duplicating is a good choice. |
Beta Was this translation helpful? Give feedback.
-
|
Oh god, please no! Duplicate notes are a key reason why I LEFT evernote! (they have a 5 year old bug where a patchy network connection results in dozens of duplicated notes). Once you create a duplicate file, how is the user (on a phone) expected to resolve that conflict and merge it back into the repo in a single file? Between those two options, I'd strongly prefer (1) prompt the user. But I'd suggest not prompting to accept local vs remote, but prompt the user to tell them that it found a conflict, and take them to the file - with git's standard "merge fail" block that shows what each side has. That way both changesets are preserved in the visible file, and the user can tidy things up from there. I am not all that experienced with git, but my point is that this is a problem that git has already "solved" as much as it can, so let's use git's solution to the problem, and just add notification/assistance on top of that, rather than creating a new method of resolving it. |
Beta Was this translation helpful? Give feedback.
-
|
I just got bitten by this again. @ginkgomzd you mentioned that there is no data loss, that it's in the git repo - but I can't see it there (I'm talking about the github repo and my desktop - maybe it's present locally on my phone?). It looks to me like straight up data-loss - functionally, gitjournal has deleted a set of changes from github and from my desktop - this doesn't seem a good method. Am I making it worse in how I sync my desktop? I'm doing: git add --all
git commit -m "Autocommit from $(hostname)"
git pull --rebase
git pushIs that sensible, or is there something different I should do there? My reasoning is that if the rebase fails, I should have a merge conflict block sitting in the relevant file, with both the old and the new (or local and remote) changes, leaving it to me to sort out - which is perfect for me. It's visible (ie, it's right there in the file when I go to look at that data), it's complete (includes both conflicting blocks) and it's safe (no data is discarded). Am I doing something dumb on the desktop end of it, or is this because of how gitjournal is pushing the changes? |
Beta Was this translation helpful? Give feedback.
-
|
Sorry. I'm a very busy with personal stuff this week, but I've made a lot of progress in implementing this. I can give you a rough ETA of mid November. |
Beta Was this translation helpful? Give feedback.
-
|
That's totally cool! :-) Is the plan to have conflicts appear as a merge conflict block within the file, just like git usually behaves? I'd really prefer that as a solution. I have strong objections to duplicate files (they're invisible if you're using scripts/shortcuts/muscle-memory to edit notes), and to choosing local vs remote ("You have two children. Which one would you like me to destroy?"). A merge block I can address simply and at any time. Actually, I wouldn't mind if the merge conflict got comitted into the repo - it's text, not code, so doesn't matter if it's a bit broken-looking as long as everything's there. In the interim I think I'll setup a cron job to sync my desktop regularly, and I'll take extra care to refresh my android before starting any edits. Love your work! |
Beta Was this translation helpful? Give feedback.
-
|
The main change will be trying to do a smart word based merge by default. Additionally, I'm going to be exposing the history of the file in an easily navigable option. Regarding what to do when one cannot merge it, I don't know - for now, my plan is just to implement both options (copy a file, or show a dialog). I don't know which would make a better default. |
Beta Was this translation helpful? Give feedback.
-
|
@agittins that sucks you lost some changes. Since you are auto-committing from desktop in addition to the app, I don't see how you could have lost the data. There are cases where you can lose data in git, but that typically involves doing a force-push, which I don't see in your script. When I say "repo": git is distributed version control... every copy has a repo. You inspect it by viewing the |
Beta Was this translation helpful? Give feedback.
-
Hey MotaOcimar. The first step would be implementing a basic git merge in dart-git. I've already implemented Cheers |
Beta Was this translation helpful? Give feedback.
-
|
Oh, I have no knowledge of git internals, but I'm still interested. |
Beta Was this translation helpful? Give feedback.
-
Please don't apologize. All that matters is that you want to try and are willing to learn. How about you join the discord channel and we talk over there? It'll definitely be faster. |
Beta Was this translation helpful? Give feedback.
-
|
Hello wonderful people. I have news. If you look in Settings -> Experimental Features, there are now 2 new options "Dart only Git Implementation" and "Dart only merge implementation". These both shouldn't change the behavior of GitJournal in anyway, but they do stop using I would appreciate if you could test them out. They pass my tests, but you never know. The sooner I'm confident that dart-git works well, the sooner I can start using it and have a custom merge mechanism. |
Beta Was this translation helpful? Give feedback.
-
|
I can see the options, but I can only enable "merge implementation", the toggle for "Git implementation" doesn't move. |
Beta Was this translation helpful? Give feedback.
-
Copy paste error. Fixed. I'll make a new release over the weekend. |
Beta Was this translation helpful? Give feedback.
-
|
Hi, thanks for your work. I have a question, is an experimental "Dart only merge implementation" just makes precedence local files over remote ? |
Beta Was this translation helpful? Give feedback.
-
Yes. That's all for now. I was hoping to get feedback from users to know if they have encountered any problems, even though the code has tests. After which, I can implement an actual improvement.
What do you mean by deletes files? This sounds scary. |
Beta Was this translation helpful? Give feedback.
-
I mean my remote file was zeroed |
Beta Was this translation helpful? Give feedback.
-
|
@slayer : So if I understand correctly, after the merge the file was deleted instead of it choosing the remote file or local file? |
Beta Was this translation helpful? Give feedback.
-
Yes, there was no choosing |
Beta Was this translation helpful? Give feedback.
-
|
@slayer : I would love for you to share some of the info about your git repo. Here are the steps -
If you would prefer to share this information via email feel free to email me - vhanda at gitjournal.io |
Beta Was this translation helpful? Give feedback.
-
|
Sent by email. |
Beta Was this translation helpful? Give feedback.
-
|
After fighting with git for half a day I have decided to manually examine every merge because I don't trust any automatic merging anymore and also I don't trust
This complicated procedure is due to the fact that GitJournal pulls the changes and then overwrites them without checking for conflicts. Thus it creates a path of commits which tells Git that all conflicts have been resolved by GitJournal. I have found no way to tell Git that it should always produce a merge conflict if a file has been changed in both main and "git-journal-shift" reliably. Here's smerge.sh #!/usr/bin/env bash
# print executed commands
set -o xtrace
# fail fast
set -Eeuo pipefail
branch='git-journal-shift'
commit_message='m'
# abort if merge is ongoing
MERGE_INDICATOR_FILE="$(git rev-parse --git-dir)"/MERGE_HEAD
if test -f "$MERGE_INDICATOR_FILE"; then
echo "*********************** merge is ongoing, commit first. ***********************"
exit 1;
fi
# ref: https://stackoverflow.com/a/2658301
function evil_git_num_untracked_files {
expr `git status --porcelain 2>/dev/null| grep "^??" | wc -l`
}
# commit & pull & push
# git commit returns exit status 1 if nothing to commit so need to check first
if (( $(evil_git_num_untracked_files) > 0 )); then
# there are untracked files
git add -A
git commit -a -m 'm'
fi
# if HEAD is dirty commit
git diff-index --quiet HEAD || git commit -a -m "$commit_message"
git pull
git push
# make merge visible
git fetch origin $branch:$branch
git merge --no-commit --no-ff $branch
git checkout --conflict merge .
if test -f "$MERGE_INDICATOR_FILE"; then
echo "*********************** resolve merge and commit. ***********************"
exit 1;
fi
# push branch
git branch --force $branch
git push origin $branch
#
echo "*********************** ALL GOOD! **************************"EDIT: Here are two Stackoverflow questions I found concerning this problem: |
Beta Was this translation helpful? Give feedback.
-
|
I could not let it go and have thus found a solution. The magic keyword is "merge-base". It is the common ancestor used by simplified steps of the new sync script:
git replace --graft --force "$git_journal_branch" "$branch_last_merge"
References which helped me:
And here is my updated merge script: #!/usr/bin/env bash
# print executed commands
set -o xtrace
# fail fast
set -Eeuo pipefail
branch='git-journal-shift'
main_branch='main'
# branch that points to commit where last merge was performed (this is used as the merge-base!)
branch_last_merge='last-merge'
# temporary branch which indicates that a graft ref for that commit should automatically be removed on next script execution.
temp_todo_remove_graft='todo_remove_graft'
# auto commit main first
scripts/auto_commit.sh
# abort if merge is ongoing
MERGE_INDICATOR_FILE="$(git rev-parse --git-dir)"/MERGE_HEAD
if test -f "$MERGE_INDICATOR_FILE"; then
echo "*********************** merge is ongoing, commit first. ***********************"
exit 1;
fi
# create last-merge branch if not exists
if ! git rev-parse --verify "$branch_last_merge" ; then
# point last-merge to the most recent common ancestor of the two branches
git branch --force "$branch_last_merge" "$(git merge-base $main_branch $branch)"
fi
# fetch branch
git fetch origin $branch:$branch
# merge using $branch_last_merge as merge-base (using graft) if branch is not ancestor of main branch
if ! git merge-base --is-ancestor "$branch" "$main_branch" ; then
git branch --force "$temp_todo_remove_graft" "$branch"
# `git replace` fails when trying to make an unneccessary graft (i.e. if parent is already equal to last-merge)
if [ $(git rev-parse "$branch^") != $(git rev-parse "$branch_last_merge") ]; then
# `--graft <commit> [<parent>…]``
# Documentation: Create a graft commit. A new commit is created with the same content as <commit> except that its
# parents will be [<parent>…] instead of <commit>'s parents. A replacement ref is then created to replace <commit>
# with the newly created commit.
git replace --graft --force "$branch" "$branch_last_merge"
fi
git merge --no-ff "$branch"
fi
# clean up: remove graft ref and temporary branch (ignore errors)
if git rev-parse --verify "$temp_todo_remove_graft" ; then
git replace -d "$temp_todo_remove_graft" || true
git branch -D "$temp_todo_remove_graft" || true
fi
# fast forward branch to main (the code above ensures "branch" is an ancestor of main)
git branch --force "$branch" "$main_branch"
# push
git push origin "$branch"
git push origin "$main_branch"
# update branch which points to commit at last merge
git branch --force "$branch_last_merge" "$main_branch"
#
echo "*********************** ALL GOOD! **************************"And the auto commit script which is executed by the merge script: #!/usr/bin/env bash
# print executed commands
set -o xtrace
# fail fast
set -Eeuo pipefail
commit_message='m'
# abort if merge is ongoing
MERGE_INDICATOR_FILE="$(git rev-parse --git-dir)"/MERGE_HEAD
if test -f "$MERGE_INDICATOR_FILE"; then
echo "*********************** merge is ongoing, commit first. ***********************"
exit 1;
fi
# ref: https://stackoverflow.com/a/2658301
function evil_git_num_untracked_files {
expr `git status --porcelain 2>/dev/null| grep "^??" | wc -l`
}
# commit & pull & push
# git commit returns exit status 1 if nothing to commit so need to check first
if (( $(evil_git_num_untracked_files) > 0 )); then
# there are untracked files
git add -A
git commit -a -m 'm'
fi
# if HEAD is dirty commit
git diff-index --quiet HEAD || git commit -a -m "$commit_message"
git pull
git push |
Beta Was this translation helpful? Give feedback.
-
With either auto sync or manual sync, the app will overwrite changes pushed from the desktop.
Expected behavior:
The app should either lock the notes until the sync has finished in the case of auto-sync, or simply duplicate the file, one copy for desktop changes, one for mobile.
Or even attempt to merge.
Current behavior:
See title.
Steps to reproduce
Enable auto sync in GitJournal.
From the app, press the back button to exit to home screen.
My desktop is synced to the same git repo.
Add text to some files from the desktop text editor. One of the files is named 'test.md'.
Save, commit, push.
After some time, open GitJournal and edit test.md without waiting for the sync icon to finish.
Press 'back' to close the note and stay on the note list screen.
Git Journal will do a merge that overwrites the changes done on the desktop.
Manual sync case
Disable auto sync in GitJournal.
Create a test file named "text.txt" on the desktop.
Enter the contents:
Git add, commit, push.
Sync GitJournal so we have the file there.
From the desktop add a line between the existing 2.
Git add, commit, push.
From GitJournal add a line between the existing 2, without sync.
Sync GitJournal by swiping down.
Result: The line "Desktop test 1.1." will have been replaced by "Journal test 1.1."
This simulates editing a note when no cell signal is available, in a subway, in the countryside etc.
Beta Was this translation helpful? Give feedback.
All reactions