On Wed, Mar 27, 2013 at 03:42:40PM -0700, Junio C Hamano wrote:
> John Keeping <john@xxxxxxxxxxxxx> writes:
>
> > When the same file is added with identical content at the top level,
> > git-merge-tree prints "added in both" with the details. But if the file
> > is added in an existing subdirectory, threeway_callback() bails out early
> > because the two trees have been modified identically.
> >
> > In order to detect this, we need to fall through and recurse into the
> > subtree in this case.
> >
> > Signed-off-by: John Keeping <john@xxxxxxxxxxxxx>
>
> The rationale the above description gives is internally consistent,
> but it is rather sad to see this optimization go. The primary
> motivation behind this program, which does not use the usual
> unpack-trees machinery, is to allow us to cull the identical result
> at a shallow level of the traversal when the both sides changed (not
> added) a file deep in a subdirectory hierarchy.
>
> The patch makes me wonder if we should go the other way around,
> resolving the "both added identically" case at the top cleanly
> without complaint.
I don't use merge-tree so I have no opinion on this, just wanted to fix
an inconsistency :-)
I'll try to have a look at doing the other change tomorrow if no one
else gets there first.
> > builtin/merge-tree.c | 9 +++++++--
> > t/t4300-merge-tree.sh | 17 +++++++++++++++++
> > 2 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> > index e0d0b7d..ca97fbd 100644
> > --- a/builtin/merge-tree.c
> > +++ b/builtin/merge-tree.c
> > @@ -298,12 +298,17 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
> > {
> > /* Same in both? */
> > if (same_entry(entry+1, entry+2)) {
> > - if (entry[0].sha1) {
> > + if (entry[0].sha1 && !S_ISDIR(entry[0].mode)) {
> > /* Modified identically */
> > resolve(info, NULL, entry+1);
> > return mask;
> > }
> > - /* "Both added the same" is left unresolved */
> > + /*
> > + * "Both added the same" is left unresolved. We also leave
> > + * "Both directories modified identically" unresolved in
> > + * order to catch changes where the same file (with the same
> > + * content) has been added to both directories.
> > + */
> > }
> >
> > if (same_entry(entry+0, entry+1)) {
> > diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
> > index d0b2a45..be0737e 100755
> > --- a/t/t4300-merge-tree.sh
> > +++ b/t/t4300-merge-tree.sh
> > @@ -298,4 +298,21 @@ test_expect_success 'turn tree to file' '
> > test_cmp expect actual
> > '
> >
> > +test_expect_success 'add identical files to subdir' '
> > + cat >expected <<\EXPECTED &&
> > +added in both
> > + our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE
> > + their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE
> > +EXPECTED
> > +
> > + git reset --hard initial &&
> > + mkdir sub &&
> > + test_commit "sub-initial" "sub/initial" "initial" &&
> > + test_commit "sub-add-a-b-same-A" "sub/ONE" "AAA" &&
> > + git reset --hard sub-initial &&
> > + test_commit "sub-add-a-b-same-B" "sub/ONE" "AAA" &&
> > + git merge-tree sub-initial sub-add-a-b-same-A sub-add-a-b-same-B >actual &&
> > + test_cmp expected actual
> > +'
> > +
> > test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html