passforios icon indicating copy to clipboard operation
passforios copied to clipboard

Merge conflicts

Open supermarin opened this issue 6 years ago • 13 comments

There's a couple issues if you're using Pass on iOS and your other computers, and create merge conflicts on accident (edit the same gpg file for example):

  • You get the merge conflict error, and that invalidates the SSH keys. You're prompted to enter your SSH password again every time this happens. For this case, I think the fix should be not to invalidate the key; the operation should be idempotent and every time present the same message.
  • There's no way to see which files are causing conflicts. I don't think Pass should gain a fully fledged text editor (Pass being fast and lightweight is one of it's best features IMO). But having a way of pushing into a conflict resolution branch and resolving on your computer would be amazing. Right now, the password store is unusable since there's no way to pull new passwords.

supermarin avatar Oct 02 '17 22:10 supermarin

The first issue is now fixed in the develop branch (d0bad86). We will upload a new TestFlight version soon. Previously, we assume that the only reason for seeing a syncing error is "wrong git password / SSH key passphrase". Thus, we remove the "wrong" password/passphrase if there is a syncing error. Now, we remove the previous SSH key passphrase only if the error message contains "wrong passphrase".

For the second issue, one possible workaround that I have in mind is like this. 1. Check local commit logs (Settings->General->About Repo->Commit Logs); 2. Keep the local changes somewhere else (e.g., by copying the new raw files); 3. Discard all local changes (Settings->Advanced->Discard); 4. Fix things on a computer; 5. Sync Pass for iOS again. The above process would be extremely cumbersome and time-consuming if there are more than a few local commits. I have put "pushing into a conflict resolution branch" in the todo list.

yishilin14 avatar Oct 08 '17 13:10 yishilin14

Amazing, thanks for getting on this fast @yishilin14!

supermarin avatar Oct 09 '17 22:10 supermarin

What's a workaround for this now? Manually copy any change in Pass for iOS to a computer and then re-clone the whole repository?

(Doing exactly this seems to have worked.)

kenny-evitt avatar Nov 28 '18 22:11 kenny-evitt

workaround:

  1. On your computer:
  • cd /path/to/your/passwords
  • git push (make sure your computer and remote are clean)
  • git checkout -b conflicts
  1. On your phone:
  • Open Pass.app.
  • Go to Settings->General->About Repository->Commit Logs and see what's the last commit your phone has
  1. On your computer:
  • git checkout master
  • git log (find the $SHA of the last common commit)
  • git reset --hard $SHA
  • git push --force origin master (reset remote to the version your phone has)
  1. On your phone:
  • Back to Pass.app
  • Pull to refresh, your phone's changes should be now in remote but missing your computer's changes
  1. Back on your computer
  • git pull origin master (everything should be same as your phone, minus your computer changes)
  • git checkout conflicts
  • git rebase master
  • git checkout master
  • git merge --no-ff conflicts
  • git push origin master

This bug is somewhere in the underlying ObjectiveGit I think, or in Pass's usage of it. The contents should be decrypted before merging, which I'm not sure it's the case (haven't looked into it yet).

I've resolved this many times with the above technique, and never had to manually resolve conflicts on the computer. Git just handled it all.

supermarin avatar Nov 28 '18 23:11 supermarin

@supermarin Thanks! I think I did that, but the sync continued to fail even after I'd reset the shared repo (the common remote for both my phone and my computer). I didn't check the commit log on my phone tho. I'll do that if this happens again.

Generally, as a Git user, I've opted to always git rebase ... instead of git merge .... It'd be nice if Pass for iOS could be configured to do so too, as a default at least.

kenny-evitt avatar Nov 29 '18 13:11 kenny-evitt

@supermarin your steps are great for resolving that type of conflict, but in order to avoid making that kind of mistake in the future, I made my ios pass store track a different branch, namely I chose to:

  • pass git push origin master:ios to create a new branch tracking master
  • change iphone to track ios

In the future, now, I can simply manage and resolve merge conflicts from my computer and then push a new copy of master onto the ios branch whenever I make a change from both devices. This works for me since I rarely modify things from my phone.

norcalli avatar Apr 18 '19 15:04 norcalli

I think those are workarounds for a symptom that needs to be solved in the first place. I believe the core of the issue is merge strategy. Your computer's git knows how to use gpg --decrypt for merge strategy before comparing diffs. The one in passforios is probably trying to diff the encrypted binaries and thus runs in conflicts easily.

I haven't had time to dig deep into this, but a rewrite to a core piece might be needed to eliminate this all together.

supermarin avatar Apr 23 '19 15:04 supermarin

@supermarin Based on a search in GitHub for "merge", it doesn't seem like the app is currently trying to do any merges at all. It's (maybe) implicitly, potentially, merging by pulling from the remote, but nothing beyond that that I could find (in a very cursory search).

I'm pretty sure this isn't only an issue with how Pass for iOS is merging. I for one don't want it to try to just merge automatically. Until there's a bulletproof UI for merging arbitrary conflicts, with would require a nice text editor, I think the idea mentioned by @yishilin14 to push local changes to a branch (with some way to push that branch to the remote), would be the best way to handle merge conflicts.

One possibly relevant consideration is that some people, myself included, prefer to rebase among branches instead of merge. I don't like that Pass itself merges automatically already. With passwords specifically, I can't even think of a scenario where I'd want to merge conflicts. I've always wanted to rather 'accept' one version (the one corresponding to the most recent password change) over another.

kenny-evitt avatar Apr 23 '19 18:04 kenny-evitt

@kenny-evitt it's doing git pull, which is equivalent to git fetch and git merge. When you pull to refresh it's "Syncing", and there's no Sync with git.

In this particular repo, pull invocation is done here, which uses ObjectiveGit's pull here and eventually merges remote tracking (master by default) into your tracking (master by default) here.

In order to pull (fetch+merge), git needs to know how to properly merge contents and apply the proper additions / deletions. On your computer you might have something like this in your ~/.gitconfig:

[diff "gpg"]
    textconv = gpg --no-tty --decrypt

... or ...

[diff "plist"]
    textconv = plutil -convert xml1 -o -

Personally I don't see what's the issue with your phone being able to do the same thing as your laptop. I use git pull --rebase everywhere which rebases instead of merging, but that still needs to know the diff before applying rebased patches.

supermarin avatar Apr 24 '19 01:04 supermarin

tl;dr - I don't think normal users should have more than 1 branch if they're not doing super advanced stuff, and things should "just work" on their end.

supermarin avatar Apr 24 '19 01:04 supermarin

@supermarin Thanks for the details! That's the same code I found in my quick search. I agree, in principle, that there isn't an issue in requesting this – it would be ideal to be able to explicitly handle merge or rebase conflicts directly in the app.

But, personally, I've run into this because I changed a password twice, once from my phone, updating it in this app, and again, at a different time, from one of my computers, updating it in the local Pass repo there (before pushing to my shared remote). Which version I'd want to retain depended on when I updated the password on the two devices. Neither merging or rebasing, even with a diff that decrypts the contents of the two conflicting commits, would seem to always leave me with the 'correct' (most recently updated) password.

I don't think it's too much for users of Pass to handle an occasional 'conflict branch' on a computer – purely as an initial, incremental improvement over the current status quo. Doing so also has the advantage of being much easier for the developers to implement (thus much more likely to be done at all versus a more ideal set of features).

I've used the iOS app Working Copy before to work with Git repos on my phone and I it has a 'resolve tool' for handling merge or rebase conflicts. It also supports 'Git shortcuts' which I believe are ways that this app could call that one to automatically merge conflicts. I'm not sure it makes a lot of sense to tie this app to that one, but it could be, at least, an available option.

kenny-evitt avatar Apr 24 '19 14:04 kenny-evitt

But, personally, I've run into this because I changed a password twice, once from my phone, updating it in this app, and again, at a different time, from one of my computers, updating it in the local Pass repo there (before pushing to my shared remote). Which version I'd want to retain depended on when I updated the password on the two devices. Neither merging or rebasing, even with a diff that decrypts the contents of the two conflicting commits, would seem to always leave me with the 'correct' (most recently updated) password.

I think this is a different case, where you legitimately introduced a conflict; something that gets detected after decrypting. I'm OK if developers need to resolve stuff like this on the computer.

On the other hand, IIRC 1Password would keep both versions of the password and you'd be able to see them both on your phone once you synced. I think this is the ideal scenario; some manual git wrapping might be needed for that.

From the user perspective, I think the last chronological password should be kept as current, and user should be able to see the previous values.

supermarin avatar Apr 24 '19 18:04 supermarin

Wouldn’t it be possible to adopt a strategy that “accepts everything”? If there are two files that both change, just store both in a commit, adding a suffix to their names to reflect that (eg a.iOS.gpg and a.remote.gpg). That way, iOS pass doesn’t lead to any annoying issues during sync. Users can then delete manually (from iOS or computer) one of the files.

Is there any other pass use case that this strategy cannot resolve?

hyiltiz avatar Dec 12 '21 19:12 hyiltiz