Re: [PATCH v1 5/8] sequencer: don't die in print_commit_summary()

From: Phillip Wood <phillip.wood@talktalk.net>
Date: Nov 10, 2017, 14:53 UTC
Message-ID: <072b0edb-7a30-77f3-2ac5-893c28c0a695@talktalk.net>
In-Reply-To: <xmqq7ev2dsv2.fsf@gitster.mtv.corp.google.com>

On 07/11/17 15:13, Junio C Hamano wrote:

Show 10 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> And this step is going in the right direction, but I am not sure if
>> this made the function safe enough to be called repeatedly from the
>> rebase machinery and we are ready to unleash this to the end users
>> and tell them it is safe to use it.
> 
> Another possibility perhaps is that the function is safe to reuse
> already even without this patch, of course ;-).
>

Hmm, maybe it is. Looking at pick_commits() and do_pick_commit() if the sequencer dies in print_commit_summary() (which can only happen when cherry-picking or reverting) then neither the todo list or the abort safety file are updated to reflect the commit that was just made.

As I understand it print_commit_summary() dies because: (i) it cannot resolve HEAD either because some other process is updating it (which is bad news in the middle of a cherry-pick); (ii) because something went wrong HEAD is corrupt; or (iii) log_tree_commit() cannot read some objects. In all those cases dying will leave the sequencer in a sane state for aborting - 'git cherry-pick --abort' will rewind HEAD to the last successful commit before there was a problem with HEAD or the object database. If the user somehow fixes the problem and runs 'git cherry-pick --continue' then the sequencer will try and pick the same commit again which may or may not be what the user wants depending on what caused print_commit_summary() to die.