Re: [PATCH 2/2] merge: make restore_state() do as its name says

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

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 00de224a2da..ae3ee3a996b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -377,11 +377,11 @@ static void restore_state(const struct object_id *head,
>  {
>  	const char *args[] = { "stash", "apply", NULL, NULL };
>  
> -	if (is_null_oid(stash))
> -		return;
> -
>  	reset_hard(head, 1);

when there is only one strategy to be tried, save_state() will never
be called.  Removing the above safety means the hard-reset is
discarding a local change that is not saved anywhere.  The reason
why the merge stopped may be because such a local change has crashed
with the change the merge wanted to bring in, no?

> +	if (is_null_oid(stash))
> +		goto refresh_cache;
> +
> +test_description="Test that merge state is as expected after failed merge"
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +. ./test-lib.sh
> +
> +test_expect_success 'set up custom strategy' '
> +	test_commit --no-tag "Initial" base base &&
> +git show-ref &&
> +
> +	for b in branch1 branch2 branch3
> +	do
> +		git checkout -b $b main &&
> +		test_commit --no-tag "Change on $b" base $b
> +	done &&
> +
> +	git checkout branch1 &&

Here, perhaps we can make two additional test cases, that try with
local change that (1) overlaps with the changes branch2 and branch3
bring in and that (2) does not overlap.  I am worried about the case
(2) losing the local change due to the call to reset_hard().

> +	test_must_fail git merge branch2 branch3 &&
> +	git diff --exit-code --name-status &&
> +	test_path_is_missing .git/MERGE_HEAD
> +'
> +
> +test_done