Re: [PATCH 5/5] rebase -m: fix --signoff with conflicts

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 18, 2024, 21:16 UTC
Message-ID: <xmqq7cguqsln.fsf@gitster.g>
In-Reply-To: <4c8f8843780f3ac23262f1e45a5000d183adca6b.1713445918.git.phillip.wood@dunelm.org.uk>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 15 quoted lines

> +	struct replay_ctx *ctx = opts->ctx;
> +
> +	/*
> +	 * Write the commit message to be used by "git rebase
> +	 * --continue". If a "fixup" or "squash" command has conflicts
> +	 * then we will have already written rebase_path_message() in
> +	 * error_failed_squash(). If an "edit" command was
> +	 * fast-forwarded then we don't have a message in ctx->message
> +	 * and rely on make_patch() to write rebase_path_message()
> +	 * instead.
> +	 */
> +	if (ctx->have_message && !file_exists(rebase_path_message()) &&
> +	    write_message(ctx->message.buf, ctx->message.len,
> +			  rebase_path_message(), 0))
> +		return error(_("could not write commit message file"));

Makes the readers wonder if there are cases where we have written to disc, but .have_message is true and the on-disc contents and in-core contents are different. If a codepath that writes to the file had the message in-core, and it needs to tweak the message (e.g., add a sign-off trailer) before writing it out to the file but forgot to do so, then later we have tweaked the message in-core, such a bug would result in the necessary "tweak" we have done not appear on disc. Optionally keeping an in-core copy of what we have on disc does not directly reduces the possibility of introducing such a bug, and I am wondering if we can do anything clever about it.

Thanks.