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

[Elijah Newren <newren@gmail.com>]

On Mon, Sep 16, 2019 at 5:09 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> Jonathan Tan wrote:
>
> > This was raised by a coworker at $DAYJOB. I run the following script:
> [reproduction recipe from (*) snipped]
> > 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.
>
> Gábor covered the "what happened", so let me say a little more about
> the motivation.
>
> The "foo" symlink is being replaced by a "foo" directory containing a
> "bar" file.  We're pretty far along now: we want to write actual files
> to disk.  Using the index we know where we were going from and to, but
> not everything in the world is tracked in the index: there could be
> build outputs under "foo/bar" blocking the operation from moving
> forward.
>
> So we check whether there's a directory there.  Once we are writing
> things out, there won't be, but the symlink confuses us.  Nice find.

Yep.

>
> [...]
> > Is this use case something that Git should be able to handle, and if
> > yes, is the correct solution to teach dir_in_way() that dirs reachable
> > from symlinks are not really in the way (presumably the implementation
> > would climb directories until we reach the root or we reach a filesystem
> > boundary, similar to what we do when we search for the .git directory)?
>
> The crucial detail here is that "foo" is going to be removed before we
> write "foo/bar".  We should be able to notice that and skip the
> dir_in_way check.

I know what you're getting at from a high level view, but this view is
incompatible with the machinery's internals.  In particular,
merge-recursive's design provides no way to "notice" that something is
"*going* to be removed" (every path is updated on the fly as it
processes it); the dir_in_way() check is there precisely to determine
if it's safe to write to the given path -- which basically means if
there is no directory in the way (a "foo/bar/" directory, in this
case).  So we definitely do not want to skip the dir_in_way() check,
we want to modify it to be aware that a leading path being a symlink
doesn't count as in the way (much as a the existence of a file on disk
corresponding to one of our leading paths doesn't count as in the way
for our purposes).