Re: [PATCH] Use git-update-ref in scripts.

From: Junio C Hamano <junkio@cox.net>
Date: Sep 28, 2005, 19:47 UTC
Message-ID: <7vek79q8jj.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7vk6h1rr9l.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

Show 15 quoted lines

> Linus Torvalds <torvalds@osdl.org> writes:
>
>> On Wed, 28 Sep 2005, Junio C Hamano wrote:
>>> 
>> I don't like doing the diff before-hand, but it looked like the default 
>> was to try just one strategy, and avoid the diff in that case.
>
> By 'diff before-hand' I take it to mean the savestate for later
> rounds to keep the pre-merge state.  You are correct that it is
> not done in a single strategy case, and 'git pull' by default
> would use only one of Daniel's git-merge-resolve or in the
> multi-remote case git-merge-octopus, depending on the number of
> heads being merged.  BTW, I decided not to use diff, just in
> case somebody has binary blob we cannot reproduce with diff and
> patch.

I see two more diffs that turns out to be problematic in git-merge.sh code.

 (1) As a safety measure I have a check to make sure the index
     is in sync with $head.  This visibly hurts; on my slow disk
     and CPU with a couple of locally modified paths in the
     working tree, this check takes about a second in the kernel
     tree with hot cache.

     git-merge-resolve uses "git-read-tree -u -m O A B" form, so
     this is totally unnecessary.  I am not so sure about
     Fredrik's git-merge-recursive (I haven't looked at it for a
     while).

 (2) savestate uses "git diff $head" to find out the list of
     paths that have local modifications, but the current code
     calls it after the check described above, so "git-ls-files
     -m" is enough.  On a kernel tree with hot cache and index
     in sync with HEAD, "git-ls-files -m" is about 3-4 times as
     fast with a couple of locally modified files.

Keeping (1) and using 'git-ls-files -m' in (2) is optimizing for a wrong path. On the other hand, (1) is a safety measure, and if it is kept (2) becomes a quite cheap operation.

I am inclined to just remove the check in (1), and make it the responsibility of merge strategies to make sure it does not commit unrelated changes.

Comments?