Comments on recursive merge..

Guys,

  I just hit my first real rename conflict, and very timidly tried the 
"recursive" strategy in the hopes that I wouldn't need to do things by 
hand.

It resolved things beautifully. Good job. 

My only worry is that I don't read python, so I don't really know how it 
does what it does, which makes me nervous. Can somebody (Fredrik?) add 
some documentation about the merge strategy and how it works.

Considering that the stupid resolve strategy really requires you to know 
how git works when rename conflicts happen (things left in unmerged state 
are really quite hard to handle by hand unless you know exactly what 
you're doing), I'd almost suggest making "recursive" the default. I'm a 
bit nervous about it, but knowing how it works would probably put most of 
that to rest.

		Linus

On Mon, 7 Nov 2005, Linus Torvalds wrote:

  I just hit my first real rename conflict, and very timidly tried the 
"recursive" strategy in the hopes that I wouldn't need to do things by 
hand.

It resolved things beautifully. Good job. 

Btw, one thing that it does is print out too much information.

In particular, I had renames on both sides of the merge (in case anybody 
wants to see which one I'm talking about: it's the current top-of-head 
commit in the kernel archives: 333c47c847c90aaefde8b593054d9344106333b5).

Now, renames that you've done yourself you really don't want to hear 
about, at least if the other side didn't change anything in that file.

Renames that the _other_ side has done (the one you're merging) you may or 
may not want to know about, regardless of whether they happened to files 
that are changed. But since "git pull" will do a "git-apply --stat" at the 
end and show the renames there, I'd argue that the merge strategy itself 
should be quiet about any renames that are trivial.

So how about talking about renames only if you end up also doing a 
file-level merge? As it is, doing the merge talked about renames that I 
had merged earlier in my own branch, which is just confusing.

		Linus

On Mon, Nov 07, 2005 at 08:48:06AM -0800, Linus Torvalds wrote:

Guys,

  I just hit my first real rename conflict, and very timidly tried the 
"recursive" strategy in the hopes that I wouldn't need to do things by 
hand.

It resolved things beautifully. Good job. 

I'm glad that it worked.

My only worry is that I don't read python, so I don't really know how it 
does what it does, which makes me nervous. Can somebody (Fredrik?) add 
some documentation about the merge strategy and how it works.

I will write something up.

Considering that the stupid resolve strategy really requires you to know 
how git works when rename conflicts happen (things left in unmerged state 
are really quite hard to handle by hand unless you know exactly what 
you're doing), I'd almost suggest making "recursive" the default. I'm a 
bit nervous about it, but knowing how it works would probably put most of 
that to rest.

It would be great if the recursive strategy could get some more
testing. I have tested it on a thousand commits or so in a few kernel
repositories and haven't found any bugs, but it could be due to errors
in the test setup, testing the wrong repositories or just being lucky. Some
real-world testing would be great.

- Fredrik

On Mon, Nov 07, 2005 at 08:56:07AM -0800, Linus Torvalds wrote:

Btw, one thing that it does is print out too much information.

In particular, I had renames on both sides of the merge (in case anybody 
wants to see which one I'm talking about: it's the current top-of-head 
commit in the kernel archives: 333c47c847c90aaefde8b593054d9344106333b5).

Now, renames that you've done yourself you really don't want to hear 
about, at least if the other side didn't change anything in that file.

Renames that the _other_ side has done (the one you're merging) you may or 
may not want to know about, regardless of whether they happened to files 
that are changed. But since "git pull" will do a "git-apply --stat" at the 
end and show the renames there, I'd argue that the merge strategy itself 
should be quiet about any renames that are trivial.

So how about talking about renames only if you end up also doing a 
file-level merge? As it is, doing the merge talked about renames that I 
had merged earlier in my own branch, which is just confusing.

Sounds like a good idea. How about something like the following?

--

It isn't really interesting to know about the renames that have
already been committed to the branch you are working on. Furthermore,
the 'git-apply --stat' at the end of git-(merge|pull) will tell us
about any renames in the other branch.

With this commit only renames which require a file-level merge will
be printed.

Signed-off-by: Fredrik Kuivinen <redacted>


---

 git-merge-recursive.py |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

applies-to: 5af1b5b93257ecfe993bb24975bf596faa342758
89c029b439603630a53ee4e4d0cb7931111afd2a

diff --git a/git-merge-recursive.py b/git-merge-recursive.py
index 626d854..9983cd9 100755
--- a/git-merge-recursive.py
+++ b/git-merge-recursive.py

@@ -162,10 +162,13 @@ def mergeTrees(head, merge, common, bran
 # Low level file merging, update and removal
 # ------------------------------------------
 
+MERGE_NONE = 0
+MERGE_TRIVIAL = 1
+MERGE_3WAY = 2
 def mergeFile(oPath, oSha, oMode, aPath, aSha, aMode, bPath, bSha, bMode,
               branch1Name, branch2Name):
 
-    merge = False
+    merge = MERGE_NONE
     clean = True
 
     if stat.S_IFMT(aMode) != stat.S_IFMT(bMode):

@@ -178,7 +181,7 @@ def mergeFile(oPath, oSha, oMode, aPath,
             sha = bSha
     else:
         if aSha != oSha and bSha != oSha:
-            merge = True
+            merge = MERGE_TRIVIAL
 
         if aMode == oMode:
             mode = bMode

@@ -207,7 +210,8 @@ def mergeFile(oPath, oSha, oMode, aPath,
             os.unlink(orig)
             os.unlink(src1)
             os.unlink(src2)
-            
+
+            merge = MERGE_3WAY
             clean = (code == 0)
         else:
             assert(stat.S_ISLNK(aMode) and stat.S_ISLNK(bMode))

@@ -577,14 +581,16 @@ def processRenames(renamesA, renamesB, b
                 updateFile(False, ren1.dstSha, ren1.dstMode, dstName1)
                 updateFile(False, ren2.dstSha, ren2.dstMode, dstName2)
             else:
-                print 'Renaming', fmtRename(path, ren1.dstName)
                 [resSha, resMode, clean, merge] = \
                          mergeFile(ren1.srcName, ren1.srcSha, ren1.srcMode,
                                    ren1.dstName, ren1.dstSha, ren1.dstMode,
                                    ren2.dstName, ren2.dstSha, ren2.dstMode,
                                    branchName1, branchName2)
 
-                if merge:
+                if merge or not clean:
+                    print 'Renaming', fmtRename(path, ren1.dstName)
+
+                if merge == MERGE_3WAY:
                     print 'Auto-merging', ren1.dstName
 
                 if not clean:

@@ -653,14 +659,16 @@ def processRenames(renamesA, renamesB, b
                 tryMerge = True
 
             if tryMerge:
-                print 'Renaming', fmtRename(ren1.srcName, ren1.dstName)
                 [resSha, resMode, clean, merge] = \
                          mergeFile(ren1.srcName, ren1.srcSha, ren1.srcMode,
                                    ren1.dstName, ren1.dstSha, ren1.dstMode,
                                    ren1.srcName, srcShaOtherBranch, srcModeOtherBranch,
                                    branchName1, branchName2)
 
-                if merge:
+                if merge or not clean:
+                    print 'Renaming', fmtRename(ren1.srcName, ren1.dstName)
+
+                if merge == MERGE_3WAY:
                     print 'Auto-merging', ren1.dstName
 
                 if not clean: