Re: [PATCH 21/48] merge-recursive: Fix recursive case with D/F conflict via add/add conflict

[Junio C Hamano <gitster@pobox.com>]

Elijah Newren <newren@gmail.com> writes:

> When a D/F conflict is introduced via an add/add conflict, when
> o->call_depth > 0 we need to ensure that the higher stage entry from the
> base stage is removed.


>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-recursive.c                 |    2 ++
>  t/t6036-recursive-corner-cases.sh |    4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 7da6aa0..4771fb4 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1480,6 +1480,8 @@ static int process_df_entry(struct merge_options *o,
>  			       "Adding %s as %s",
>  			       conf, path, other_branch, path, new_path);
>  			update_file(o, 0, sha, mode, new_path);
> +			if (o->call_depth)
> +				remove_file_from_cache(path);

Puzzling... This codepath is inside

	if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
        	...

which _is_ all about the working tree. Why should the code even looking at
the working tree if it can be called for recursive virtual ancestor merge?
I'll keep reading the series, hoping that this is something that will be
fixed in a later patch (ah, 23/48 does that).

At the outermost merge, you would keep the made-up name "new_path" as well
as the original name "path"?

When you added "d/f" (file in a directory) and I added "d" (file), merging
your work to my tree with this code would result in

	d at stage #2 (no 0/1/3);
	d~madeupsuffix in the working tree;
        d/f at stage #3 (no 0/1/2); and
        d/f in the working tree.

right? I am not sure if you also have d-madeupsuffix in the index.

As I said in my review of an earlier patch for test suite, having "d" in
the index would be helpful to the user ("git diff" cannot be used to see
what's different) so we _might_ be better off leaving this instead:

	d-madeupsuffix at stage #2 (no 0/1/3);
	d~madeupsuffix in the working tree;
        d/f at stage #3 (no 0/1/2); and
        d/f in the working tree.

and removing "d" from the index even for the outermost merge.

Thanks.