git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/4] refs: trim newline from reflog message

From
Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com>
Date
Nov 22, 2021, 14:20 UTC
Message-ID
<dfb639373234a6b8d5f9110380a66ffccbe0b1d6.1637590855.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1145.git.git.1637590855.gitgitgadget@gmail.com>
From: Han-Wen Nienhuys <hanwen@google.com>

Commit 523fa69c ("reflog: cleanse messages in the refs.c layer") standardizes how write entries into the reflog. This commit standardizes how we get messages out of the reflog. Before, the files backend implicitly added '\n' to the end of reflog message on reading, which creates a subtle incompatibility with alternate ref storage backends, such as reftable.

We address this by stripping LF from the message before we pass it to the user-provided callback.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 builtin/show-branch.c          |  5 -----
 reflog-walk.c                  |  6 ++----
 refs/files-backend.c           | 30 +++++++++++++++---------------
 t/t1405-main-ref-store.sh      |  5 ++---
 t/t1406-submodule-ref-store.sh |  4 ++--
 5 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f1e8318592c..016ccdafd0f 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -761,7 +761,6 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			char *logmsg;
 			char *nth_desc;
 			const char *msg;
-			char *end;
 			timestamp_t timestamp;
 			int tz;
 
@@ -772,10 +771,6 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 				break;
 			}
 
-			end = strchr(logmsg, '\n');
-			if (end)
-				*end = '\0';
-
 			msg = (*logmsg == '\0') ? "(none)" : logmsg;
 			reflog_msg[i] = xstrfmt("(%s) %s",
 						show_date(timestamp, tz,
diff --git a/reflog-walk.c b/reflog-walk.c
index 8ac4b284b6b..3ee59a98d2f 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -244,8 +244,6 @@ void get_reflog_message(struct strbuf *sb,
 
 	info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
 	len = strlen(info->message);
-	if (len > 0)
-		len--; /* strip away trailing newline */
 	strbuf_add(sb, info->message, len);
 }
 
@@ -284,10 +282,10 @@ void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
 		get_reflog_selector(&selector, reflog_info, dmode, force_date, 0);
 		if (oneline) {
-			printf("%s: %s", selector.buf, info->message);
+			printf("%s: %s\n", selector.buf, info->message);
 		}
 		else {
-			printf("Reflog: %s (%s)\nReflog message: %s",
+			printf("Reflog: %s (%s)\nReflog message: %s\n",
 			       selector.buf, info->email, info->message);
 		}
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 151b0056fe5..583bbc5f8b9 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1936,17 +1936,15 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 	int tz;
 	const char *p = sb->buf;
 
-	/* old SP new SP name <email> SP time TAB msg LF */
-	if (!sb->len || sb->buf[sb->len - 1] != '\n' ||
-	    parse_oid_hex(p, &ooid, &p) || *p++ != ' ' ||
+	/* old SP new SP name <email> SP time TAB msg */
+	if (!sb->len || parse_oid_hex(p, &ooid, &p) || *p++ != ' ' ||
 	    parse_oid_hex(p, &noid, &p) || *p++ != ' ' ||
-	    !(email_end = strchr(p, '>')) ||
-	    email_end[1] != ' ' ||
+	    !(email_end = strchr(p, '>')) || email_end[1] != ' ' ||
 	    !(timestamp = parse_timestamp(email_end + 2, &message, 10)) ||
 	    !message || message[0] != ' ' ||
-	    (message[1] != '+' && message[1] != '-') ||
-	    !isdigit(message[2]) || !isdigit(message[3]) ||
-	    !isdigit(message[4]) || !isdigit(message[5]))
+	    (message[1] != '+' && message[1] != '-') || !isdigit(message[2]) ||
+	    !isdigit(message[3]) || !isdigit(message[4]) ||
+	    !isdigit(message[5]))
 		return 0; /* corrupt? */
 	email_end[1] = '\0';
 	tz = strtol(message + 1, NULL, 10);
@@ -2038,6 +2036,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 				strbuf_splice(&sb, 0, 0, bp + 1, endp - (bp + 1));
 				scanp = bp;
 				endp = bp + 1;
+				strbuf_trim_trailing_newline(&sb);
 				ret = show_one_reflog_ent(&sb, fn, cb_data);
 				strbuf_reset(&sb);
 				if (ret)
@@ -2050,6 +2049,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 				 * Process it, and we can end the loop.
 				 */
 				strbuf_splice(&sb, 0, 0, buf, endp - buf);
+				strbuf_trim_trailing_newline(&sb);
 				ret = show_one_reflog_ent(&sb, fn, cb_data);
 				strbuf_reset(&sb);
 				break;
@@ -2099,7 +2099,7 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 	if (!logfp)
 		return -1;
 
-	while (!ret && !strbuf_getwholeline(&sb, logfp, '\n'))
+	while (!ret && !strbuf_getline(&sb, logfp))
 		ret = show_one_reflog_ent(&sb, fn, cb_data);
 	fclose(logfp);
 	strbuf_release(&sb);
@@ -3059,18 +3059,18 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	if ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,
 				   message, policy_cb)) {
 		if (!cb->newlog)
-			printf("would prune %s", message);
+			printf("would prune %s\n", message);
 		else if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
-			printf("prune %s", message);
+			printf("prune %s\n", message);
 	} else {
 		if (cb->newlog) {
-			fprintf(cb->newlog, "%s %s %s %"PRItime" %+05d\t%s",
-				oid_to_hex(ooid), oid_to_hex(noid),
-				email, timestamp, tz, message);
+			fprintf(cb->newlog, "%s %s %s %" PRItime " %+05d\t%s\n",
+				oid_to_hex(ooid), oid_to_hex(noid), email,
+				timestamp, tz, message);
 			oidcpy(&cb->last_kept_oid, noid);
 		}
 		if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
-			printf("keep %s", message);
+			printf("keep %s\n", message);
 	}
 	return 0;
 }
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 49718b7ea7f..a600bedf2cd 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -89,13 +89,12 @@ test_expect_success 'for_each_reflog()' '
 test_expect_success 'for_each_reflog_ent()' '
 	$RUN for-each-reflog-ent HEAD >actual &&
 	head -n1 actual | grep one &&
-	tail -n2 actual | head -n1 | grep recreate-main
+	tail -n1 actual | grep recreate-main
 '
 
 test_expect_success 'for_each_reflog_ent_reverse()' '
 	$RUN for-each-reflog-ent-reverse HEAD >actual &&
-	head -n1 actual | grep recreate-main &&
-	tail -n2 actual | head -n1 | grep one
+	tail -n1 actual | grep one
 '
 
 test_expect_success 'reflog_exists(HEAD)' '
diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index 0a87058971e..b0365c1fee0 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -74,13 +74,13 @@ test_expect_success 'for_each_reflog()' '
 test_expect_success 'for_each_reflog_ent()' '
 	$RUN for-each-reflog-ent HEAD >actual &&
 	head -n1 actual | grep first &&
-	tail -n2 actual | head -n1 | grep main.to.new
+	tail -n1 actual | grep main.to.new
 '
 
 test_expect_success 'for_each_reflog_ent_reverse()' '
 	$RUN for-each-reflog-ent-reverse HEAD >actual &&
 	head -n1 actual | grep main.to.new &&
-	tail -n2 actual | head -n1 | grep first
+	tail -n1 actual | grep first
 '
 
 test_expect_success 'reflog_exists(HEAD)' '

-- 
gitgitgadget
Previous: Elijah NewrenNext: Junio C Hamano
Message 6 of 60 in “Inspect reflog data programmatically in more tests”
  1. 0/4 Inspect reflog data programmatically in more testsHan-Wen Nienhuys via GitGitGadget, Nov 22, 2021
  2. 1/4 show-branch: show reflog messageHan-Wen Nienhuys via GitGitGadget, Nov 22, 2021
  3. Junio C HamanoNov 22, 2021
  4. Bagas SanjayaNov 23, 2021
  5. Elijah NewrenNov 23, 2021
  6. 2/4 refs: trim newline from reflog messageHan-Wen Nienhuys via GitGitGadget, Nov 22, 2021
  7. Junio C HamanoNov 22, 2021
  8. Han-Wen NienhuysNov 23, 2021
  9. Junio C HamanoNov 23, 2021
  10. Han-Wen NienhuysNov 23, 2021
  11. Junio C HamanoNov 23, 2021
  12. Han-Wen NienhuysNov 24, 2021
  13. Junio C HamanoNov 24, 2021
  14. Han-Wen NienhuysNov 24, 2021
  15. Junio C HamanoNov 24, 2021
  16. Han-Wen NienhuysNov 25, 2021
  17. Junio C HamanoNov 29, 2021
  18. Junio C HamanoNov 24, 2021
  19. Han-Wen NienhuysNov 24, 2021
  20. Re* [PATCH 2/4] refs: trim newline from reflog messageJunio C Hamano, Nov 26, 2021
  21. Ævar Arnfjörð BjarmasonNov 28, 2021
  22. Junio C HamanoNov 28, 2021
  23. Re* [PATCH 2/4] refs: trim newline from reflog messageJunio C Hamano, Nov 28, 2021
  24. Ævar Arnfjörð BjarmasonNov 29, 2021
  25. Ævar Arnfjörð BjarmasonNov 23, 2021
  26. Han-Wen NienhuysNov 23, 2021
  27. 3/4 test-ref-store: tweaks to for-each-reflog-ent formatHan-Wen Nienhuys via GitGitGadget, Nov 22, 2021
  28. Junio C HamanoNov 22, 2021
  29. Han-Wen NienhuysNov 23, 2021
  30. Junio C HamanoNov 23, 2021
  31. 4/4 t1400: use test-helper ref-store to inspect reflog contentsHan-Wen Nienhuys via GitGitGadget, Nov 22, 2021
  32. Ævar Arnfjörð BjarmasonNov 22, 2021
  33. Han-Wen NienhuysNov 22, 2021
  34. Junio C HamanoNov 22, 2021
  35. 0/5 Inspect reflog data programmatically in more testsHan-Wen Nienhuys via GitGitGadget, Nov 25, 2021
  36. 3/5 t1405: check for_each_reflog_ent_reverse() more thoroughlyHan-Wen Nienhuys via GitGitGadget, Nov 25, 2021
  37. 5/5 refs/debug: trim trailing LF from reflog messageHan-Wen Nienhuys via GitGitGadget, Nov 25, 2021
  38. Junio C HamanoNov 26, 2021
  39. Han-Wen NienhuysNov 29, 2021
  40. Junio C HamanoNov 29, 2021
  41. Junio C HamanoNov 29, 2021
  42. Han-Wen NienhuysDec 2, 2021
  43. Junio C HamanoDec 2, 2021
  44. Ævar Arnfjörð BjarmasonNov 29, 2021
  45. 1/5 show-branch: show reflog messageHan-Wen Nienhuys via GitGitGadget, Nov 25, 2021
  46. 2/5 test-ref-store: don't add newline to reflog messageHan-Wen Nienhuys via GitGitGadget, Nov 25, 2021
  47. Junio C HamanoNov 26, 2021
  48. 4/5 test-ref-store: tweaks to for-each-reflog-ent formatHan-Wen Nienhuys via GitGitGadget, Nov 25, 2021
  49. Junio C HamanoNov 26, 2021
  50. Ævar Arnfjörð BjarmasonNov 29, 2021
  51. Han-Wen NienhuysNov 29, 2021
  52. Junio C HamanoNov 29, 2021
  53. Ævar Arnfjörð BjarmasonNov 29, 2021
  54. Han-Wen NienhuysDec 2, 2021
  55. 0/5 Inspect reflog data programmatically in more testsHan-Wen Nienhuys via GitGitGadget, Dec 2, 2021
  56. 1/5 show-branch: show reflog messageHan-Wen Nienhuys via GitGitGadget, Dec 2, 2021
  57. 5/5 refs/debug: trim trailing LF from reflog messageHan-Wen Nienhuys via GitGitGadget, Dec 2, 2021
  58. 4/5 test-ref-store: tweaks to for-each-reflog-ent formatHan-Wen Nienhuys via GitGitGadget, Dec 2, 2021
  59. 3/5 t1405: check for_each_reflog_ent_reverse() more thoroughlyHan-Wen Nienhuys via GitGitGadget, Dec 2, 2021
  60. 2/5 test-ref-store: don't add newline to reflog messageHan-Wen Nienhuys via GitGitGadget, Dec 2, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.