Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> The return value of do_recursive_merge() may be positive (indicating merge
> conflicts), or 0 (indicating success). It also may be negative, indicating
> a fatal error that requires us to abort.
>
> Now, if the return value indicates that there are merge conflicts, we
> should not try to commit those changes, of course.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> sequencer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index cbc3742..9ffc090 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -787,7 +787,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> res = allow;
> goto leave;
> }
> - if (!opts->no_commit)
> + if (!res && !opts->no_commit)
> res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
> opts, allow, opts->edit, 0, 0);
This by itself looks more like a bugfix than preparation for later
steps. The only reason why it is not a bugfix is because there is
nothing in this function that makes res a non-zero value and reach
this if statement at this step. We would have been caught by an
"if (res) { ... rerere(); goto leave; }" or
"if (allow < 0) { res = allow; goto leave; }"
that appear before this part of the code.
So while it is not wrong per-se, I think this should be part of an
actual change that makes it possible for the control flow to reach
here with non-zero res.