Re: merge-recursive thinks symlink's child dirs are "real"

From: Elijah Newren <newren@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Jonathan Tan <jonathantanmy@google.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: merge-recursive thinks symlink's child dirs are "real"
Date: Tue, 17 Sep 2019 08:54:52 -0700	[thread overview]
Message-ID: <CABPp-BEjsfPb8BYjdDGwwJE2GEbRDsujV2xD=c-0zygJCv4F0g@mail.gmail.com> (raw)
In-Reply-To: <20190916221501.GD6190@szeder.dev>

On Mon, Sep 16, 2019 at 3:15 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Mon, Sep 16, 2019 at 02:47:07PM -0700, Jonathan Tan wrote:
> > This was raised by a coworker at $DAYJOB. I run the following script:
> >
> >   $GIT init test && cd test
> >   ln -s . foo
> >   mkdir bar && touch bar/file
> >   $GIT add foo bar/file
> >   $GIT commit -m "foo symlink"
> >
> >   $GIT checkout -b branch1
> >   $GIT commit --allow-empty -m "empty commit"
> >
> >   $GIT checkout master
> >   $GIT rm foo
> >   mkdir foo
> >   (cd foo; ln -s ../bar bar)
> >   $GIT add foo/bar
> >   $GIT commit -m "replace foo symlink with real foo dir and foo/bar symlink"
> >
> >   $GIT checkout branch1
> >   $GIT cherry-pick master
> >
> > The cherry-pick must be manually resolved, when I would expect it to
> > happen without needing user intervention.
> >
> > You can see that at the point of the cherry-pick, in the working
> > directory, ./foo is a symlink and ./foo/bar is a directory. I traced the
> > code that ran during the cherry-pick to process_entry() in
> > merge-recursive.c. When processing "foo/bar", control flow correctly
> > reaches "Case B: Added in one", but the dir_in_way() invocation returns
> > true, since lstat() indeed reveals that "foo/bar" is a directory. If I
> > hardcode dir_in_way() to return false, then the cherry-pick happens
> > without needing user intervention. I checked with "ls-tree -r" and the
> > resulting tree is as I expect (foo is a real dir, foo/bar is a symlink).
> >
> > Is this use case something that Git should be able to handle,
>
> FWIW, Git used to handle this case, but it broke with edd2faf52e
> (merge-recursive: Consolidate process_entry() and process_df_entry(),
> 2011-08-11).
>
> Cc-ing Elijah for insights...

Thanks for the heads up.

Yeah, git used to get _extremely_ confused with D/F conflicts, even
worse than here.  Interesting that it took 8 years to find this case.
Anyway, the problem here is that a symlink involved in a D/F conflict
could be a symlink to a directory instead of to a file, and then make
it look like we have some weird nested D/F conflicts in cases when we
don't (i.e. make it look like not only "foo" exists as both a file
(symlink) and directory, but that "foo/bar" does too).

Anyway, being more careful to treat symlinks in D/F conflicts more
like how we treat files in such conflicts, using the way Jonathan Tan
suggested, should fix this issue up.  I put more details in my email
to him.