On 10/11/17 18:05, Junio C Hamano wrote:
> Phillip Wood <phillip.w...@talktalk.net> writes:
>
>> On 07/11/17 15:13, Junio C Hamano wrote:
>> ...
>>> 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.
>
> The above is all good analysis---thanks for your diligence. Perhaps
> some if not all of it can go to the log message?
>
Thanks, that's a good idea. I see the above as a reason to drop this
commit as returning an error will try and update the abort safety file
which we don't want to do if there is a problem with HEAD so I'll add it
to the previous commit to explain why it is okay to die.
