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

From: SZEDER Gábor <[email-masked]>
Date: Tue, 17 Sep 2019 00:15:01 +0200

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...

> 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)?
> Also, my proposed solution would work in the specific use case outlined
> in the script above, but can anyone think offhand of a case that it
> would make worse?