Re: [PATCH v3 5/7] merge: make restore_state() restore staged state too

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 21, 2022, 16:16 UTC
Message-ID: <xmqqfsiul9vj.fsf@gitster.g>
In-Reply-To: <91c495c770e3f7f91ae655a503bdd1cd99935e40.1658391391.git.gitgitgadget@gmail.com>

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

Show 5 quoted lines

>   4) Merge strategies can make changes to the index and working tree,
>      and have no expectation to clean up after themselves, *even* if
>      they bail out and say they are not an appropriate handler for
>      the merge in question.  (The `octopus` merge strategy does this,
>      for example.)

I personally consider this is a bug in `octopus` (and I can do so without offending anybody, as `octopus` was what I did), but because the point of having pluggable merge strategies is to allow end users and third parties write their own. So save-and-restore dance would be a prudent approach to the issue than forbidding this "buggy" behaviour.

Show 7 quoted lines

> Unfortunately, if users had staged changes before calling `git merge`,
> builtin/merge.c could do the following:
>
>    * stash the changes, in order to clean up after the strategies
>    * try all the merge strategies in turn, each of which report they
>      cannot function due to the index not matching HEAD
>    * restore the changes via "git stash apply"

Show 7 quoted lines

>  	test_seq 0 10 >a &&
>  	git add a &&
> +	git rev-parse :a >expect &&
> ...
> +	# Changes to "a" should remain staged
> +	git rev-parse :a >actual &&
> +	test_cmp expect actual

Makes sense.

Thanks.