Re: [PATCHv2 00/57] Re-roll of en/merge-recursive from pu

From: Elijah Newren <newren@gmail.com>
Date: Aug 13, 2011, 02:37 UTC
Message-ID: <CABPp-BH3eoYAM9-beQv57Byaf73hQECORxJnqt3JFXTvBpgPRw@mail.gmail.com>
In-Reply-To: <CABPp-BE_cqtOBC_0tsy1nSOfH1XPGM0vdv9fHWp38Q0GBr7OhQ@mail.gmail.com>

On Fri, Aug 12, 2011 at 3:59 PM, Elijah Newren <newren@gmail.com> wrote:

Show 12 quoted lines

> On Thu, Aug 11, 2011 at 11:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Avoiding silent mismerges is of course one of the most important criteria,
>> but we also need to make sure that a conflicted state left in the index
>> and the working tree files must not be harder to reconcile than what we
>> have been giving our users---otherwise the change will be seen as a
>> regression by them.
>
> Yeah, good point.  I tried re-running my previous re-merge all merges
> in git.git testcase, modified so that when both versions of git
> reported a conflict I would compare the output of 'git ls-files  -s'.
> That uncovered a regression.  I think I know the fix but I need to
> retest and do some more checks.

With the new patch that I just submitted ("[PATCHv2 57/57] merge-recursive: Don't re-sort a list whose order we depend upon"), I now have fixed up the only issue I found with the more thorough tests you suggest. In other words:

I used the new patch on top of my series and redid my testing.  For
each two-parent merge commit in git.git (about 5000 of them), I redid
my tests.  I verified that for each redone commit,  both versions of
git would:
  (A) EITHER both fail to merge cleanly OR both merged cleanly
  (B) had identical 'git ls-files -s' output
  (C) had identical output from
       find . -type f -print0 | grep -z -v ^./.git | sort -z -u |
xargs -0 sha1sum
(Note that (B) is different than before; it should be useful in
comparing failed merges as well as successful ones, making sure the
contents of the index match.  Also, the purpose of (C) is to ensure
that the contents of the working tree match.)

In all cases, (A) and (B) were true. There were 16 commits for which (C) was not true; the working tree contents were different. However, each and every such difference were due to the change in behavior to provide more info in conflict markers (branchname:filename instead of just branchname) -- see patch 36 in the series.

So with the new patch, and the commit message changes Johannes suggested for patch 27, I'm hoping this series is finally in good shape. But, as before, comments on the changes and other ideas for more thorough testing are welcome.