git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Kyle J. McKay" <mackyle@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Git mailing list <git@vger.kernel.org>
Subject: [PATCH 2/2] refs.c: avoid creating extra unwanted reflog entries
Date: Sat, 30 Jan 2021 03:19:09 -0700	[thread overview]
Message-ID: <1e8c8e3d23d3c2ddf031e3d7719241c@72481c9465c8b2c4aaff8b77ab5e23c> (raw)
In-Reply-To: <7c7e8679f2da7e1475606d698b2da8c@72481c9465c8b2c4aaff8b77ab5e23c>

Since commit 523fa69c36744ae6 ("reflog: cleanse messages in the refs.c
layer", 2020-07-10, v2.29.0), ref log messages are now being "cleansed"
to make sure they do not end up breaking the ref log files.  A laudable
endeavor.

Unfortunately, that commit had an unintended side effect that causes
the `git symbolic-ref <refname1> <refname2>` command to suddenly start
adding new entries to the ref log for <refname1> whenever it's run.

These new entries have a completely empty message and do not provide
any useful information.  In fact, there was no mention that the change
to "cleanse" ref log messages was intended to add these new ref log
entries at all.

What happened is that when the change to "cleanse" the incoming ref
log message was made, the code started inadvertently transforming
a NULL ref log message pointer into an empty string "".

This created the observed effect that using the `symbolic-ref` command
suddenly started causing ref log entries to be added.

The original code that predated the "cleanse" commit called the
`xstrdup_or_null` function to retain the original NULL pointer and
avoid introducing unwanted extra ref log entries.

After the "cleanse" commit, ref log messages are now funnelled through
a new static function named `normalize_reflog_message`.

Eliminate the unwanted extra blank ref log entries by returning a NULL
pointer when NULL is passed into `normalize_reflog_message` rather
than returning a pointer to an empty string ("").

To reflect this new behavior, rename the function to
`normalize_reflog_message_or_null` in the same spirit as the name
of the `xstrdup_or_null` function that was called pre-"cleanse".

Flip the `test_expect_failure` tests to `test_expect_success`
as they now pass again.

Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---
 refs.c                   | 16 +++++++++-------
 t/t1417-reflog-symref.sh |  6 +++---
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/refs.c b/refs.c
index 03968ad7..790b1ff0 100644
--- a/refs.c
+++ b/refs.c
@@ -835,11 +835,13 @@ static void copy_reflog_msg(struct strbuf *sb, const char *msg)
 	strbuf_rtrim(sb);
 }
 
-static char *normalize_reflog_message(const char *msg)
+static char *normalize_reflog_message_or_null(const char *msg)
 {
 	struct strbuf sb = STRBUF_INIT;
 
-	if (msg && *msg)
+	if (!msg)
+		return NULL;
+	if (*msg)
 		copy_reflog_msg(&sb, msg);
 	return strbuf_detach(&sb, NULL);
 }
@@ -1067,7 +1069,7 @@ struct ref_update *ref_transaction_add_update(
 		oidcpy(&update->new_oid, new_oid);
 	if (flags & REF_HAVE_OLD)
 		oidcpy(&update->old_oid, old_oid);
-	update->msg = normalize_reflog_message(msg);
+	update->msg = normalize_reflog_message_or_null(msg);
 	return update;
 }
 
@@ -1951,7 +1953,7 @@ int refs_create_symref(struct ref_store *refs,
 	char *msg;
 	int retval;
 
-	msg = normalize_reflog_message(logmsg);
+	msg = normalize_reflog_message_or_null(logmsg);
 	retval = refs->be->create_symref(refs, ref_target, refs_heads_master,
 					 msg);
 	free(msg);
@@ -2339,7 +2341,7 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,
 	char *msg;
 	int retval;
 
-	msg = normalize_reflog_message(logmsg);
+	msg = normalize_reflog_message_or_null(logmsg);
 	retval = refs->be->delete_refs(refs, msg, refnames, flags);
 	free(msg);
 	return retval;
@@ -2357,7 +2359,7 @@ int refs_rename_ref(struct ref_store *refs, const char *oldref,
 	char *msg;
 	int retval;
 
-	msg = normalize_reflog_message(logmsg);
+	msg = normalize_reflog_message_or_null(logmsg);
 	retval = refs->be->rename_ref(refs, oldref, newref, msg);
 	free(msg);
 	return retval;
@@ -2374,7 +2376,7 @@ int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
 	char *msg;
 	int retval;
 
-	msg = normalize_reflog_message(logmsg);
+	msg = normalize_reflog_message_or_null(logmsg);
 	retval = refs->be->copy_ref(refs, oldref, newref, msg);
 	free(msg);
 	return retval;
diff --git a/t/t1417-reflog-symref.sh b/t/t1417-reflog-symref.sh
index 6149531f..3687b058 100755
--- a/t/t1417-reflog-symref.sh
+++ b/t/t1417-reflog-symref.sh
@@ -53,7 +53,7 @@ test_expect_success setup '
 	test $hcnt -ne $kcnt
 '
 
-test_expect_failure 'HEAD reflog symbolic-ref' '
+test_expect_success 'HEAD reflog symbolic-ref' '
 	hcnt1=$(git reflog show HEAD | wc -l) &&
 	git symbolic-ref HEAD refs/heads/unu &&
 	git symbolic-ref HEAD refs/heads/du &&
@@ -62,7 +62,7 @@ test_expect_failure 'HEAD reflog symbolic-ref' '
 	test $hcnt1 = $hcnt2
 '
 
-test_expect_failure 'refs/heads/KVAR reflog symbolic-ref' '
+test_expect_success 'refs/heads/KVAR reflog symbolic-ref' '
 	kcnt1=$(git reflog show refs/heads/KVAR | wc -l) &&
 	git symbolic-ref refs/heads/KVAR refs/heads/tri &&
 	git symbolic-ref refs/heads/KVAR refs/heads/du &&
@@ -71,7 +71,7 @@ test_expect_failure 'refs/heads/KVAR reflog symbolic-ref' '
 	test $kcnt1 = $kcnt2
 '
 
-test_expect_failure 'double symref reflog symbolic-ref' '
+test_expect_success 'double symref reflog symbolic-ref' '
 	hcnt1=$(git reflog show HEAD | wc -l) &&
 	kcnt1=$(git reflog show refs/heads/KVAR | wc -l) &&
 	git symbolic-ref HEAD refs/heads/KVAR &&
-- 

      parent reply	other threads:[~2021-01-30 10:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-30 10:19 [PATCH 0/2] Eliminate extraneous ref log entries Kyle J. McKay
2021-01-30 10:19 ` [PATCH 1/2] t/t1417: test symbolic-ref effects on ref logs Kyle J. McKay
2021-01-30 18:56   ` Junio C Hamano
2021-01-30 23:02     ` Kyle J. McKay
2021-01-30 23:48       ` Junio C Hamano
2021-02-01 11:09         ` Han-Wen Nienhuys
2021-01-30 23:26     ` Kyle J. McKay
2021-01-31  0:11       ` Junio C Hamano
2021-01-30 10:19 ` Kyle J. McKay [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1e8c8e3d23d3c2ddf031e3d7719241c@72481c9465c8b2c4aaff8b77ab5e23c \
    --to=mackyle@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).