"Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> Merge strategies can fail -- not just have conflicts, but give up and
> say that they are unable to handle the current type of merge. However,
> they can also make changes to the index and working tree before giving
> up; merge-octopus does this, for example. Currently, we do not expect
> the individual strategies to clean up after themselves, but instead
> expect builtin/merge.c to do so. For it to be able to, it needs to save
> the state before trying the merge strategy so it can have something to
> restore to. Therefore, remove the shortcut bypassing the save_state()
> call.
>
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> builtin/merge.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 2dc56fab70b..aaee8f6a553 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1663,12 +1663,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> * tree in the index -- this means that the index must be in
> * sync with the head commit. The strategies are responsible
> * to ensure this.
> + *
> + * Stash away the local changes so that we can try more than one.
> */
The comment explains why we limited the save_state() to avoid wasted
cycles and SSD wear and tear by looking at the number of strategies.
But because we are removing the restriction (which I am not 100%
sure is a good idea), "so that we can try more than one" no longer
applies as the reason why we run save_state() here.
> - if (use_strategies_nr == 1 ||
> - /*
> - * Stash away the local changes so that we can try more than one.
> - */
> - save_state(&stash))
> + if (save_state(&stash))
> oidclr(&stash);
>
> for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
