Re: [PATCH] merge --abort: don't delete autostash before reset succeeds

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> I'm afraid I don't think this is the right solution. We only want to 
> save the stash if there are conflicts when we apply it - that is why 
> MERGE_AUTOSTASH is deleted before we do the reset - we want to prevent 
> remove_branch_state() from saving it. If the stash applies cleanly then 
> we should not save it. If the reset fails then we should keep 
> MERGE_AUTOSTASH along with the other merge state files rather than 
> saving the stash (which is actually what happens after this patch 
> because cmd_reset() dies before it calls remove_branch_state()).

Thanks for pointing it out that reset calls remove_branch_state(),
which in turn calls remove_merge_branch_state(), which in turn calls
save_autostash_ref().  We end up (when cmd_reset() is successful)
applying the autostash (which is good) but also saving a new stash.

> I think the solution is probably to stop calling 
> builtin/reset.c:cmd_reset() and instead ...

Great.  In general, it is a bad pattern we should find and fix for
cmd_A() to call cmd_B() in its implementation as a subroutine.  To
clean any such instance is a great thing to do.

> ...
> So we only delete MERGE_AUTOSTASH after a successful reset and we only 
> save the stash if it applies with conflicts. That's all a bit more 
> involved than the patch here - please do give me a shout if you want 
> some more information.
>
> Thanks
>
> Phillip

Thanks.