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 remotes/origin/sb/test-cherry-pick-submodule-getting-in-a-way) > 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 :/