On 10/11/17 18:05, Junio C Hamano wrote: > Phillip Wood <phillip.wood@xxxxxxxxxxxx> 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.
