commit 93436ccdaf0945f5fdb4b5cd2b90c9ac0bb14c83
from: Mark Jamsek <mark@jamsek.dev>
date: Fri Feb 10 12:04:49 2023 UTC

got: use timestamp and emptiness to validate log message

As suggested by naddy: consider commit log messages valid provided the temp
file time stamp has changed and the file is not empty. This heuristic
provides the desired behaviour (i.e., reusing cherrypicked/backed-out log
messages) that's currently provided but is simpler to grok.

Improved by and ok stsp@

commit - 4a1dd8cd4ffaff7d51710820c34321acef181042
commit + 93436ccdaf0945f5fdb4b5cd2b90c9ac0bb14c83
blob - 6909ba95d7617d65c7fcad655d3baa1f1543340a
blob + d3ce58fd68ab0a1ef9be75ee3b113fb0080db8db
--- got/got.1
+++ got/got.1
@@ -2040,10 +2040,10 @@ is committed with
 the log messages of relevant merged commits will then appear in the editor,
 where the messages should be further adjusted to convey the reasons for
 cherrypicking the changes.
-At the very least, comment lines must be removed.
-Otherwise
+Upon exiting the editor, if the time stamp of the log message file
+is unchanged or the log message is empty,
 .Cm got commit
-will fail with an unmodified log message error.
+will fail with an unmodified or empty log message error.
 .Pp
 If all the changes in all files touched by a given commit are discarded,
 e.g. with
@@ -2148,10 +2148,10 @@ is committed with
 the log messages of relevant reverse-merged commits will then appear in
 the editor, where the messages should be further adjusted to convey the
 reasons for backing out the changes.
-At the very least, comment lines must be removed.
-Otherwise
+Upon exiting the editor, if the time stamp of the log message file
+is unchanged or the log message is empty,
 .Cm got commit
-will fail with an unmodified log message error.
+will fail with an unmodified or empty log message error.
 .Pp
 If all the changes in all files touched by a given commit are discarded,
 e.g. with
blob - e4bb98750bf47f68c270846e694ff0d461408b05
blob + 4a0cd57f859bca5045a0003d40445f08adc26657
--- got/got.c
+++ got/got.c
@@ -442,11 +442,9 @@ edit_logmsg(char **logmsg, const char *editor, const c
     int require_modification)
 {
 	const struct got_error *err = NULL;
-	char *line = NULL;
 	struct stat st, st2;
 	FILE *fp = NULL;
-	size_t len, logmsg_len;
-	char *initial_content_stripped = NULL, *buf = NULL, *s;
+	size_t logmsg_len;
 
 	*logmsg = NULL;
 
@@ -459,64 +457,17 @@ edit_logmsg(char **logmsg, const char *editor, const c
 	if (stat(logmsg_path, &st2) == -1)
 		return got_error_from_errno("stat");
 
-	if (require_modification &&
-	    st.st_mtime == st2.st_mtime && st.st_size == st2.st_size)
+	if (require_modification && timespeccmp(&st.st_mtim, &st2.st_mtim, ==))
 		return got_error_msg(GOT_ERR_COMMIT_MSG_EMPTY,
 		    "no changes made to commit message, aborting");
-
-	/*
-	 * Set up a stripped version of the initial content without comments
-	 * and blank lines. We need this in order to check if the message
-	 * has in fact been edited.
-	 */
-	initial_content_stripped = malloc(initial_content_len + 1);
-	if (initial_content_stripped == NULL)
-		return got_error_from_errno("malloc");
-	initial_content_stripped[0] = '\0';
 
-	buf = strdup(initial_content);
-	if (buf == NULL) {
-		err = got_error_from_errno("strdup");
-		goto done;
-	}
-	s = buf;
-	len = 0;
-	while ((line = strsep(&s, "\n")) != NULL) {
-		if (len == 0 && line[0] == '\n')
-			continue; /* remove leading empty lines */
-		len = strlcat(initial_content_stripped, line,
-		    initial_content_len + 1);
-		if (len >= initial_content_len + 1) {
-			err = got_error(GOT_ERR_NO_SPACE);
-			goto done;
-		}
-	}
-	while (len > 0 && initial_content_stripped[len - 1] == '\n') {
-		initial_content_stripped[len - 1] = '\0';
-		len--;
-	}
-
 	fp = fopen(logmsg_path, "re");
 	if (fp == NULL) {
 		err = got_error_from_errno("fopen");
 		goto done;
 	}
 
-	/*
-	 * Check whether the log message was modified.
-	 * Editing or removal of comments does count as a modifcation to
-	 * support reuse of existing log messages during cherrypick/backout.
-	 */
-	err = read_logmsg(logmsg, &logmsg_len, fp, st2.st_size, 0);
-	if (err)
-		goto done;
-	if (require_modification &&
-	    strcmp(*logmsg, initial_content_stripped) == 0)
-		err = got_error_msg(GOT_ERR_COMMIT_MSG_EMPTY,
-		    "no changes made to commit message, aborting");
-
-	/* Read log message again, stripping comments this time around. */
-	free(*logmsg);
+	/* strip comments and leading/trailing newlines */
 	err = read_logmsg(logmsg, &logmsg_len, fp, st2.st_size, 1);
 	if (err)
 		goto done;
@@ -526,8 +477,6 @@ edit_logmsg(char **logmsg, const char *editor, const c
 		goto done;
 	}
 done:
-	free(initial_content_stripped);
-	free(buf);
 	if (fp && fclose(fp) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
 	if (err) {