Re: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling
Date: Thu, 1 Nov 2018 00:01:44 -0700	[thread overview]
Message-ID: <CABPp-BFZugCFDS_A7FrWdRhp7WO0m8jhsqbSnNCSUWnAVz7mtg@mail.gmail.com> (raw)
In-Reply-To: <d23b4e26-4df4-273d-a75c-a0af915a6e5e@gmail.com>

On Wed, Oct 31, 2018 at 8:08 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 10/19/2018 3:31 PM, Elijah Newren wrote:
> > [snip]
> >
> > +                     char *new_path = NULL;
> > +                     if (dir_in_way(b->path, !o->call_depth, 0)) {
> > +                             new_path = unique_path(o, b->path, ci->branch2);
> > +                             output(o, 1, _("%s is a directory in %s adding "
> > +                                            "as %s instead"),
> > +                                    b->path, ci->branch1, new_path);
>
> I tried really hard, but failed to get a test to cover the block below.
> I was able to
> find that the "check handling of differently renamed file with D/F
> conflicts" test
> in t6022-merge-rename.sh covers the block above. Trying to tweak the
> example using
> untracked files seems to hit an error message from unpack-trees.c instead.
>
> > +                     } else if (would_lose_untracked(b->path)) {
> > +                             new_path = unique_path(o, b->path, ci->branch2);
> > +                             output(o, 1, _("Refusing to lose untracked file"
> > +                                            " at %s; adding as %s instead"),
> > +                                    b->path, new_path);
>
> It could also be that I failed because I'm less familiar with this part
> of the
> codebase. Elijah, do you think it is possible to hit this block?

Yeah, this one's going to be a little harder; the upper block would be
done with a D/F, but I think for this block you'd need a directory
rename so that unpack-trees.c can't tell that the untracked file in
the way is actually in the way of anything.  But since this is in the
rename/rename(1to2) area, I think I had rules around avoiding doing
directory renames if the other side renamed the file to avoid getting
into rename/rename(1to3) situations and other weirdness.  So, it might
require a transitive rename (i.e. file renamed on both sides, and on
one side it's renamed into a directory that the other side renamed
away).

I'll try to take a look at it tomorrow, with everything else.  We'll
see how much I can get done.

Thanks for digging in to all these and bringing them up.