Re: [PATCH 2/2] builtin-merge: avoid run_command_v_opt() for recursive

On Mon, Aug 11, 2008 at 11:47:07AM -0700, Junio C Hamano [off-list ref] wrote:

Yes, it saves a fork, but is this really worth it in the bigger picture?

Doesn't the current code structure have benefit of allowing git-merge
itself do necessary clean-up action when merge-recursive calls any of the
die() it has in many places?

As far as I see in most cases merge-recursive does not call die().

Cases when it does are like:

- broken snprintf
- cache_tree_fully_valid() or cache_tree_update() fails
- diff_setup_done() fails
- flush_buffer() fails
- read_sha1_file() fails
- ll_merge() fails

etc.

In short, I think there are two cases when a die() would be problematic
inside merge-recursive when calling it from builtin-merge:

- merge-recursive can't handle a merge, but an other strategy could do.
  This is the case when doing an octopus merge but in that case
  merge_recursive() is not called at all.

- merge-recursive results in conflicts, but an other strategy could
  handle the merge without conflicts. In this case die() isn't used
  either, so this will not be a problem.

So I don't think there is a case when a die() inside merge-recursive
would occur, but an other strategy would handle the merge properly.

Unless I missed something. ;-)