From: Fredrik Kuivinen <freku045@student.liu.se>
Date: Sun, 11 Dec 2005 09:25:31 GMT
Subject: Re: [PATCH] merge-recursive: leave unmerged entries in the index.
Message-ID: <20051211092531.GA4919@c165.ib.student.liu.se>
In-Reply-To: <7vek4ktai1.fsf@assigned-by-dhcp.cox.net>

On Sat, Dec 10, 2005 at 10:26:30PM -0800, Junio C Hamano wrote:
> This does two things.
> 
>  - When one branch renamed and the other branch did not, the
>    resulting half-merged file in the working tree used to swap
>    branches around and showed as if renaming side was "ours".
>    This was confusing and inconsistent (even though the conflict
>    markers were marked with branch names, it was not a good
>    enough excuse).  This changes the order of arguments to
>    mergeFile in such a case to make sure we always see "our"
>    change between <<< and ===, and "their" change between ===
>    and >>>.
> 
>  - When both branches renamed to the same path, and when one
>    branch renamed and the other branch did not, we attempt
>    mergeFile.  When this automerge conflicted, we used to
>    collapse the index.  Now we use update-index --index-info
>    to inject higher stage entries to leave the index in unmerged
>    state for these two cases.
> 
> What this still does _not_ do is to inject unmerged state into
> the index when the structural changes conflict.  I have not
> thought things through what to do in each case yet, but the
> cases this commit cover are the most common ones, so this would
> be a good start.
> 

Good work! I have one comment, see below.

> e5c23ede9ccc6ddd5f2cf13154f977bb1b687051
> diff --git a/git-merge-recursive.py b/git-merge-recursive.py
> index b7fb096..767b13c 100755
> --- a/git-merge-recursive.py
> +++ b/git-merge-recursive.py
> @@ -280,6 +280,22 @@ def updateFileExt(sha, mode, path, updat
>          runProgram(['git-update-index', '--add', '--cacheinfo',
>                      '0%o' % mode, sha, path])
>  
> +def setIndexStages(path,
> +                   oSHA1, oMode,
> +                   aSHA1, aMode,
> +                   bSHA1, bMode):
> +    prog = ['git-update-index', '-z', '--index-info']
> +    proc = subprocess.Popen(prog, stdin=subprocess.PIPE)
> +    pipe = proc.stdin
> +    # Clear stages first.
> +    pipe.write("0 " + ("0" * 40) + "\t" + path + "\0")
> +    # Set stages
> +    pipe.write("%o %s %d\t%s\0" % (oMode, oSHA1, 1, path))
> +    pipe.write("%o %s %d\t%s\0" % (aMode, aSHA1, 2, path))
> +    pipe.write("%o %s %d\t%s\0" % (bMode, bSHA1, 3, path))
> +    pipe.close()
> +    proc.wait()
> +

It might be cleaner to use

runProgram(['git-update-index', '-z', '--index-info'],
           input="0 " + ("0" * 40) + "\t" + path + "\0" + \
                 "%o %s %d\t%s\0" % (oMode, oSHA1, 1, path) + \
                 "%o %s %d\t%s\0" % (aMode, aSHA1, 2, path) + \
                 "%o %s %d\t%s\0" % (bMode, bSHA1, 3, path))

here instead. With this code we will get an exception if
git-update-index exits with an error code. It is also consistent with
the rest of the code.

- Fredrik