"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

Mark Jamsek <mark@jamsek.com>
Re: abort commit if merged log messages are left as-is
Omar Polo <op@omarpolo.com>, gameoftrees@openbsd.org
Tue, 7 Feb 2023 16:56:42 +1100

Download raw body.

On 23-02-06 11:25PM, Stefan Sperling wrote:
> On Mon, Feb 06, 2023 at 09:41:18AM +0100, Omar Polo wrote:
> > maybe for the commit/cherrypick case we could count the initial
> > "comment" line as part of the commit message?  it'd still be possible
> > to abort the operation and the message preserved if really desired to.
> > just wondering.
> You are suggesting that we treat the log message as modified as soon as
> the user deletes one or more # comments that we put into the initial
> template? This sounds like a reasonable idea.
> For the 'commit' case, if someone is editing only # comments, then
> the commit should still error out because of an empty commit message,
> once the comments get stripped.
> And it is true that reusing a log message as-is will be common practice
> in the cherrpick case in particular. Not so sure about backout. But the
> rules should be the same for both commands, otherwise it gets too confusing.
> Ideally, we'd have one set of rules that works for commit/cherrypick/etc.
> And it seems like what you are suggesting should work.

The below diff achieves this with a minimal change by comparing the
edited log message len with the initial content len, and allowing the
commit if the former is less than the latter; therefore, as soon as the
user removes one of the commented lines, the message is viable even
though the message is the same as the initial (stripped) content.

We can't just proceed if logmsg_len != initial_content_len because, as
you mentioned, the user may add chars to the commented text, which will
be stripped and thus shouldn't count as a valid message edit.

Is this behaviour what you had in mind?

diff /home/mark/src/got
commit - c9003c52fc7f70dd988402560adcd22709e74800
path + /home/mark/src/got
blob - a32137a402ce476d4300983fe5e9e17d680c8ac0
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -486,7 +486,7 @@ edit_logmsg(char **logmsg, const char *editor, const c
 		    "commit message cannot be empty, aborting");
 		goto done;
-	if (require_modification &&
+	if (require_modification && logmsg_len >= initial_content_len &&
 	    strcmp(*logmsg, initial_content_stripped) == 0)
 		err = got_error_msg(GOT_ERR_COMMIT_MSG_EMPTY,
 		    "no changes made to commit message, aborting");

Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68