Re: [PATCH 1/3] merge-recursive.c: conflict using sparse should update file

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Apr 07, 2017 at 12:23:55PM -0700, Kevin Willford wrote:
> Update the file when there is a conflict with a modify/delete scenario
> when using the sparse-checkout feature since the file might not be on disk
> because the skip-worktree bit is on and the user will need the file and
> content to determine how to resolve the conflict.

I'm a bit uncertain about this, but it's mostly because I'm not
familiar with merge-recursive.c. I think the general principle is, if
a file is conflicted it must NOT have skip-worktree bit on and the
worktree version must be restored at the same time the bit is removed.

I think we should do that (restore the worktree version) when the
skip-worktree bit is removed. What I'm not sure is when the bit is
removed in this code.

I'm guessing that the bit is removed by unpack_trees() with the
threeway merge. Maybe we should restore the worktree version there at
that time. It does not matter what conflict type it is, just restore
the file (which should be what merge-recursive.c expects in normal
case) then merge-recursive.c can proceed to update, delete or whatever
to present the conflict to the user.

>
> Signed-off-by: Kevin Willford <kewillf@microsoft.com>
> ---
>  merge-recursive.c                |  8 ++++++++
>  t/t7614-merge-sparse-checkout.sh | 27 +++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>  create mode 100755 t/t7614-merge-sparse-checkout.sh
> 
> diff --git a/merge-recursive.c b/merge-recursive.c
> index b7ff1ada3c..54fce93dae 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1103,6 +1103,14 @@ static int handle_change_delete(struct merge_options *o,
>  			       "and %s in %s. Version %s of %s left in tree."),
>  			       change, path, o->branch2, change_past,
>  			       o->branch1, o->branch1, path);
> +			/*
> +			 * In a sparse checkout the file may not exist. Sadly,
> +			 * the CE_SKIP_WORKTREE flag is not preserved in the
> +			 * case of conflicts, therefore we do the next best
> +			 * thing: verify that the file exists.
> +			 */
> +			if (!file_exists(path))
> +				ret = update_file(o, 0, a_oid, a_mode, path);
>  		} else {
>  			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
>  			       "and %s in %s. Version %s of %s left in tree at %s."),
> diff --git a/t/t7614-merge-sparse-checkout.sh b/t/t7614-merge-sparse-checkout.sh
> new file mode 100755
> index 0000000000..6922f0244f
> --- /dev/null
> +++ b/t/t7614-merge-sparse-checkout.sh
> @@ -0,0 +1,27 @@
> +#!/bin/sh
> +
> +test_description='merge can handle sparse-checkout'
> +
> +. ./test-lib.sh
> +
> +# merges with conflicts
> +
> +test_expect_success 'setup' '
> +	test_commit a &&
> +	test_commit file &&
> +	git checkout -b delete-file &&
> +	git rm file.t &&
> +	test_tick &&
> +	git commit -m "remove file" &&
> +	git checkout master &&
> +	test_commit modify file.t changed
> +'
> +
> +test_expect_success 'merge conflict deleted file and modified' '
> +	echo "/a" >.git/info/sparse-checkout &&
> +	test_config core.sparsecheckout true &&
> +	test_must_fail git merge delete-file &&
> +	test_path_is_file file.t
> +'
> +
> +test_done
> -- 
> 2.12.2.windows.2.1.g7df5db8d31
>