Re: [PATCH 1/2] replay: handle failure to create commits

[Patrick Steinhardt <ps@pks.im>]

On Fri, Sep 25, 2026 at 04:53:47PM -0400, Patrick Monette wrote:
> When pick_regular_commit() returns NULL, the caller relies on
> `result->clean` to figure out what happened. 1 means success, 0 means a
> conflict, and a negative value means an error.
> 
> Right now, if the commit creation fails, `result->clean` stays at 1. The
> caller doesn't expect the combination of NULL + clean == 1, so it breaks
> out of the loop, but the rest of the function treats this as a success.
> 
> The next commit will add a failure mode (signing) to the commit
> creation, so this needs to be handled correctly. Set `result->clean`
> to -1 when the commit creation fails.

True, this is something we should fix indeed.

> diff --git a/replay.c b/replay.c
> index f415103023..ad87863565 100644
> --- a/replay.c
> +++ b/replay.c
> @@ -361,7 +362,11 @@ static struct commit *pick_regular_commit(struct repository *repo,
>  		}
>  	}
>  
> -	return create_commit(repo, result->tree, pickme, replayed_base, mode);
> +	new_commit = create_commit(repo, result->tree, pickme, replayed_base,
> +				   mode);
> +	if (!new_commit)
> +		result->clean = -1;
> +	return new_commit;
>  }

Other error paths end up printing an error message. But we don't have to
because `create_commit()` already knows to do that for us.

Patrick