Why does git merge allow me to lose a line

Question

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);
            }
        }
    }
}

Answer 1

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.