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

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

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Return an error rather than dying so that the sequencer can exit
> cleanly once it starts committing without forking 'git commit'
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>...
> @@ -948,7 +951,9 @@ void print_commit_summary(const char *prefix, const struct object_id *oid,
>  		log_tree_commit(&rev, commit);
>  	}

Is that call to log_tree_commit() a big elephant in the room?

All of the die() we see above you are making into error() are rather
unlikely conditions (e.g. you created a commit, and try to look it
up immediately after that, and you somehow fail to find it);
log_tree_commit() makes tons more object accesses, any of which
would be equally likely to fail and die.

It definitely is a good thing to eventually make a direct in-process
call into the commit machinery, and we should aim for that endgame.

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.