Web lists-archives.com

Re: [PATCH 02/30] merge-recursive: Fix logic ordering issue

On Mon, Nov 13, 2017 at 3:39 PM, Elijah Newren <newren@xxxxxxxxx> wrote:
> On Mon, Nov 13, 2017 at 2:12 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>> I wanted to debug a very similar issue today just after reviewing this
>> series, see
>> https://public-inbox.org/git/743acc29-85bb-3773-b6a0-68d4a0b8fd63@xxxxxxxxx/
> Oh, bleh.  That's not a D/F conflict at all, it's the code assuming
> there's a D/F conflict because the entry it is processing ("sub") is a
> submodule rather than a file, and it panics when it sees "a directory
> in the way" -- a directory that just so happens to be named "sub" and
> which is in fact the desired submodule, meaning that the working
> directory is already good and needs no changes.

yup, I came to find the same snippet of code to be the offender,
I just haven't figured out how to fix this bug.

Thanks for taking a look!

As you have a lot of fresh knowledge in the merge-recursive case
currently, how would we approach the fix here?

(there is a test available at

> In this case, the relevant code from merge-recursive.c is the following:
>         /* Case B: Added in one. */
>         /* [nothing|directory] -> ([nothing|directory], file) */
> <snip>
>         if (dir_in_way(path, !o->call_depth,
>                    S_ISGITLINK(a_mode))) {
>             char *new_path = unique_path(o, path, add_branch);
>             clean_merge = 0;
>             output(o, 1, _("CONFLICT (%s): There is a directory with
> name %s in %s. "
>                    "Adding %s as %s"),
>                    conf, path, other_branch, path, new_path);
> Note that the comment even explicitly assumes the newly added entry is
> a file.  We should expect there to be a directory present (the
> submodule being added), but the code doesn't have a check for that.
> The S_ISGITLINK(a_mode) makes you think it has special handling for
> the submodule case, but that's for the reverse situation (the
> submodule isn't yet present in the working copy, it came from the
> other side of history, but there is an empty directory present).

oh :/