3

We recently went through some merge issues with git, and while it probably did 'The Right Thing'(TM) it wasn't what I expected. I have reduced an issue to a small public github repository located at https://github.com/geretz/merge-quirk.

quirk.c exists on master. merge-src and merge-dst are both branched from the same version on master, which is the common ancestor for the eventual merge. merge-src adds a comment and some code. merge-dst reformats the code and changes the exiting comments but does not have the added comment or code.

git merge detects and declares a conflict.

git clone https://github.com/geretz/merge-quirk.git cd merge-quirk git checkout merge-dst git merge origin/merge-src Auto-merging quirk.c CONFLICT (content): Merge conflict in quirk.c Automatic merge failed; fix conflicts and then commit the result.

However the thisLineMightDisapper (sic) function call will get lost if a naive/trusting developer chooses the origin/merge-src code block in the conflict flagged quirk.c .

In my mental model, if the thisLineMightDisapper function call exists in both HEAD and origin/src in conflicting states it should exist in both of the conflict blocks, if it doesn't exist in conflicting states it should exist outside of both conflict blocks. Why does it appear only inside the HEAD block ?

<<<<<<< HEAD // a few comment lines - change 1 // that existed - change 2 // in the common ancestor - change 3 // that get changed - change 4 if(1) { if (f(1, 2)) { if (thisLineMightDisapper(42)) ======= // a few comment lines // that existed // in the common ancestor // added this line in merge-src branch // that get changed if(1) { if (f(1, 2)) >>>>>>> origin/merge-src { t = time(0); } } } if (anotherFunction(1,2)) { t = time(0) f(0); } }

The files

master/quirk.c

 // a few comment lines // that existed // in the common ancestor // that get changed if(1) { if (f(1, 2)) { if (thisLineMightDisapper(42)) { t = time(0); } } } }

merge-src/quirk.c

 // a few comment lines // that existed // in the common ancestor // added this line in merge-src branch // that get changed if(1) { if (f(1, 2)) { if (thisLineMightDisapper(42)) { t = time(0); } } } if (anotherFunction(1,2)) { t = time(0) f(0); } }

merge-dst/quirk.c

 // a few comment lines - change 1 // that existed - change 2 // in the common ancestor - change 3 // that get changed - change 4 if(1) { if (f(1, 2)) { if (thisLineMightDisapper(42)) { t = time(0); } } } }
Improve this question
7
  • 3
    A merge conflict happens when git can't figure out how to automatically resolve changes. You, the developer, must actually read it and figure out how to resolve. Commented Jul 11, 2017 at 21:15
  • 1
    I recommend setting merge.conflictStyle to diff3 as well: this shows what Git saw in the original (common base) file, vs the two end-points being merged, i.e., HEAD / --ours and other / --theirs. Note that if you are looking at the conflict now, and have not resolved it yet, you can also do git checkout --conflict=diff3 quirk.c to replace its contents with a new conflicted-merge, but this time with the chosen style. Commented Jul 11, 2017 at 21:37
  • 1
    @torek diff3 dosn't resolve the underlying problem. thisLineMightDisapper call is missing from the diff3 common ancestors block as well, only appears in the HEAD block. Commented Jul 11, 2017 at 21:53
  • Well, see Dietrich Epp's answer; but the above is a comment, not an answer, because I find it much easier to reason about what Git saw (and thus did) when I can see the original base as well. Git only knows to match bases to both tips, and combine what it sees as changes. I will also note here that even if Git thought it resolved two diffs correctly, that's no guarantee that it did. Commented Jul 11, 2017 at 21:59
  • @torek Thanks for pointing me in the right direction (git matching base to both tips). since merge-dst reformatted there was an addition as well as a deletion of thisLineMightDisapper code in two distinct hunks. The deletion evidently didn't conflict so that part of the merge 'worked', the addition conflicted with a different hunk that didn't contain that line, so the function only appeared in one conflict block. Commented Jul 11, 2017 at 22:12

1 Answer 1

Reset to default
3

What's happening here is you're confusing semantic with textual identity. Git sees the results of shifting and splitting as entirely different lines. You wind up with a conflicting hunk and an unconflicting hunk, and a line added in the conflicting hunk that's (very) confusingly similar to a line deleted in the unconflicting one: even though they're semantically identical, 

 if (thisLineMightDisapper(42)) {

and

 if (thisLineMightDisapper(42)) {

are not the same, and git would ordinarily be very wrong to act as if they are. The second one was untouched in master and merge-src and deleted in merge-dst, separated from the conflicting hunk with the addition by the introduction of a spurious match. So git regarded the deletion as unconflicted and automerged it.

When, as here, even git checkout -m --conflict diff3 is confusing, you can pull the hunks merge is looking at with

$ sh -xc 'git diff ...MERGE_HEAD; git diff MERGE_HEAD...' + git diff ...MERGE_HEAD diff --git a/quirk.c b/quirk.c index c149623..79dc4a2 100644 --- a/quirk.c +++ b/quirk.c @@ -2,6 +2,7 @@ // a few comment lines // that existed // in the common ancestor + // added this line in merge-src branch // that get changed if(1) { if (f(1, 2)) @@ -11,4 +12,10 @@ } } } + + if (anotherFunction(1,2)) + { + t = time(0) + f(0); + } } + git diff MERGE_HEAD... diff --git a/quirk.c b/quirk.c index c149623..0de7516 100644 --- a/quirk.c +++ b/quirk.c @@ -1,14 +1,16 @@ - // a few comment lines - // that existed - // in the common ancestor - // that get changed - if(1) { - if (f(1, 2)) + // a few comment lines - change 1 + // that existed - change 2 + // in the common ancestor - change 3 + // that get changed - change 4 + if(1) + { + if (f(1, 2)) + { + if (thisLineMightDisapper(42)) { - if (thisLineMightDisapper(42)) { - t = time(0); - } + t = time(0); } + } } } + git diff ...MERGE_HEAD

and a bit of study will show you git identified a lone brace as common content, causing it to treat 

- if (thisLineMightDisapper(42)) { - t = time(0); - } + t = time(0);

as an unconflicted change.

One thing that works for me here is

git merge --abort git merge -Xignore-space-change origin/merge-src

which properly ignores the reformat when identifying change boundaries. As with hammering anything together, hammering source changes together sometimes requires exactly the right tool, and if you try it with the wrong one it won't end well. So it is here: you needed a slightly different head on your hammer, a slightly different option on your merge. In more common cases, ignoring space changes is a recipe for tab damage.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.