git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>,
	ben.humphreys@atlassian.com,
	Ben Humphreys <behumphreys@atlassian.com>
Subject: Re: [PATCH v2] merge-recursive: restore accidentally dropped setting of path
Date: Tue, 4 Jun 2019 15:48:26 -0700	[thread overview]
Message-ID: <CABPp-BFjAfpzDtgN3m9YAgcS+97TUvfZPzpOo3s9Kpn8yuDQ1A@mail.gmail.com> (raw)
In-Reply-To: <20190604210754.GT951@szeder.dev>

On Tue, Jun 4, 2019 at 2:07 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Tue, Jun 04, 2019 at 01:27:50PM -0700, Elijah Newren wrote:
> > Changes since v1:
> >   * Minor tweaks suggested by SZEDER
>
> > diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
> > index 09dfa8bd92..3fe2cd91dc 100755
> > --- a/t/t6042-merge-rename-corner-cases.sh
> > +++ b/t/t6042-merge-rename-corner-cases.sh
> > @@ -411,6 +411,124 @@ test_expect_success 'disappearing dir in rename/directory conflict handled' '
> >       )
> >  '
> >
> > +# Test for basic rename/add-dest conflict, with rename needing content merge:
> > +#   Commit O: a
> > +#   Commit A: rename a->b, modifying b too
> > +#   Commit B: modify a, add different b
> > +
> > +test_expect_success 'setup rename-with-content-merge vs. add' '
> > +     test_create_repo rename-with-content-merge-and-add &&
> > +     (
> > +             cd rename-with-content-merge-and-add &&
> > +
> > +             test_seq 1 5 >a &&
> > +             git add a &&
> > +             git commit -m O &&
> > +             git tag O &&
> > +
> > +             git checkout -b A O &&
> > +             git mv a b &&
> > +             test_seq 0 5 >b &&
> > +             git add b &&
> > +             git commit -m A &&
> > +
> > +             git checkout -b B O &&
> > +             echo 6 >>a &&
> > +             echo hello world >b &&
> > +             git add a b &&
> > +             git commit -m B
> > +     )
> > +'
> > +
> > +test_expect_success 'handle rename-with-content-merge vs. add' '
> > +     (
> > +             cd rename-with-content-merge-and-add &&
> > +
> > +             git checkout A^0 &&
> > +
> > +             test_must_fail git merge -s recursive B^0 >out &&
> > +             test_i18ngrep "CONFLICT (rename/add)" out &&
> > +
> > +             git ls-files -s >out &&
> > +             test_line_count = 2 out &&
> > +             git ls-files -u >out &&
> > +             test_line_count = 2 out &&
> > +             # Also, make sure both unmerged entries are for "b"
> > +             git ls-files -u b >out &&
> > +             test_line_count = 2 out &&
> > +             git ls-files -o >out &&
> > +             test_line_count = 1 out &&
> > +
> > +             test_path_is_missing a &&
> > +             test_path_is_file b &&
> > +
> > +             test_seq 0 6 >tmp &&
> > +             git hash-object tmp >expect &&
> > +             git rev-parse B:b >>expect &&
> > +             git rev-parse >actual  \
> > +                     :2:b    :3:b   &&
> > +             test_cmp expect actual &&
> > +
> > +             # Test that the two-way merge in b is as expected
> > +             git cat-file -p :2:b >>ours &&
> > +             git cat-file -p :3:b >>theirs &&
> > +             >empty &&
> > +             test_must_fail git merge-file \
> > +                     -L "HEAD" \
> > +                     -L "" \
> > +                     -L "B^0" \
> > +                     ours empty theirs &&
> > +             test_cmp ours b
>
> This one is OK.
>
> > +     )
> > +'
> > +
> > +test_expect_success 'handle rename-with-content-merge vs. add, merge other way' '
> > +     (
> > +             cd rename-with-content-merge-and-add &&
> > +
> > +             git reset --hard &&
> > +             git clean -fdx &&
> > +
> > +             git checkout B^0 &&
> > +
> > +             test_must_fail git merge -s recursive A^0 >out &&
> > +             test_i18ngrep "CONFLICT (rename/add)" out &&
> > +
> > +             git ls-files -s >out &&
> > +             test_line_count = 2 out &&
> > +             git ls-files -u >out &&
> > +             test_line_count = 2 out &&
> > +             # Also, make sure both unmerged entries are for "b"
> > +             git ls-files -u b >out &&
> > +             test_line_count = 2 out &&
> > +             git ls-files -o >out &&
> > +             test_line_count = 1 out &&
> > +
> > +             test_path_is_missing a &&
> > +             test_path_is_file b &&
> > +
> > +             test_seq 0 6 >tmp &&
> > +             git rev-parse B:b >expect &&
> > +             git hash-object tmp >>expect &&
> > +             git rev-parse >actual  \
> > +                     :2:b    :3:b   &&
> > +             test_cmp expect actual &&
> > +
> > +             # Test that the two-way merge in b is as expected
> > +             git cat-file -p :2:b >>ours &&
> > +             git cat-file -p :3:b >>theirs &&
> > +             >empty &&
> > +             test_must_fail git merge-file \
> > +                     -L "HEAD" \
> > +                     -L "" \
> > +                     -L "A^0" \
> > +                     ours empty theirs &&
> > +             git hash-object b >actual &&
> > +             git hash-object ours >expect &&
> > +             test_cmp ours b
>
> Here, however, you only changed what test_cmp looks at, but still kept
> the two 'git hash-object' invocations, and their output will never be
> looked at.

Whoops, indeed.

Junio: it looks like you've already deleted the two lines in your pu
pushout with your 'SQUASH???' commit at the tip of
en/merge-directory-renames-fix; that's the exact same change I would
make.  Would you like me to resend this patch with the two lines
removed, or do you just want to squash in your commit that does the
same?

Thanks,
Elijah

  parent reply	other threads:[~2019-06-04 22:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-03 20:23 [ANNOUNCE] Git v2.22.0-rc3 Junio C Hamano
2019-06-04  1:32 ` Ben Humphreys
2019-06-04  2:30   ` SZEDER Gábor
2019-06-04  7:26     ` [PATCH] merge-recursive: restore accidentally dropped setting of path Elijah Newren
2019-06-04  8:33       ` Ben Humphreys
2019-06-04 13:14       ` SZEDER Gábor
2019-06-04 20:14         ` Elijah Newren
2019-06-04 20:22       ` Elijah Newren
2019-06-04 20:27       ` [PATCH v2] " Elijah Newren
2019-06-04 21:07         ` SZEDER Gábor
2019-06-04 21:33           ` Junio C Hamano
2019-06-04 22:48           ` Elijah Newren [this message]
2019-06-04  1:47 ` [ANNOUNCE] Git v2.22.0-rc3 Bhaskar Chowdhury
2019-06-04 14:45 ` Git for Windows v2.22.0-rc3, was " Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BFjAfpzDtgN3m9YAgcS+97TUvfZPzpOo3s9Kpn8yuDQ1A@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=behumphreys@atlassian.com \
    --cc=ben.humphreys@atlassian.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).