[v3,6/7] RFC: refs: reflog entries aren't written based on reflog existence.

Commit Message

Han-Wen Nienhuys Sept. 7, 2021, 1:36 p.m. UTC

From: Han-Wen Nienhuys <[e-mail]>

Before, if we aren't supposed to update reflogs (eg.
core.logallrefupdates=NONE), we would still write reflog entries if the
reflog file (.git/logs/REFNAME) existed.

The reftable storage backend cannot distinguish between a non-existing
reflog, and an empty one. Therefore it cannot mimick this functionality.

With this feature, it is possible to mark only specific branches as subject to
reflog updates. When introduced, it presumably served as a cheap substitute for
introducing branch.$NAME.logRefUpdate configuration setting.

Reflogs are small and don't impact the runtime of normal operations, so this
flexibility is not very useful. Since it incurs complexity for alternate ref
backends, we remove it.

Further background to this change is in
<CAFQ2z_Ps3YxycA+NJ9VKt_PEXb+m83JdNB7ujzWw1fTPKyZ=[e-mail]>.

Signed-off-by: Han-Wen Nienhuys <[e-mail]>
---
 refs/files-backend.c  | 53 ++++++++++++++-----------------------------
 t/t1400-update-ref.sh |  5 ++--
 2 files changed, 19 insertions(+), 39 deletions(-)

Patch

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 04d73b89c9c..4aa4d2bbba1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1551,51 +1551,32 @@  static int log_ref_setup(struct files_ref_store *refs,
 	struct strbuf logfile_sb = STRBUF_INIT;
 	char *logfile;
 
+	*logfd = -1;
+	if (!force_create && !should_autocreate_reflog(refname))
+		return 0;
+
 	files_reflog_path(refs, &logfile_sb, refname);
 	logfile = strbuf_detach(&logfile_sb, NULL);
 
-	if (force_create || should_autocreate_reflog(refname)) {
-		if (raceproof_create_file(logfile, open_or_create_logfile, logfd)) {
-			if (errno == ENOENT)
-				strbuf_addf(err, "unable to create directory for '%s': "
-					    "%s", logfile, strerror(errno));
-			else if (errno == EISDIR)
-				strbuf_addf(err, "there are still logs under '%s'",
-					    logfile);
-			else
-				strbuf_addf(err, "unable to append to '%s': %s",
-					    logfile, strerror(errno));
-
-			goto error;
-		}
-	} else {
-		*logfd = open(logfile, O_APPEND | O_WRONLY, 0666);
-		if (*logfd < 0) {
-			if (errno == ENOENT || errno == EISDIR) {
-				/*
-				 * The logfile doesn't already exist,
-				 * but that is not an error; it only
-				 * means that we won't write log
-				 * entries to it.
-				 */
-				;
-			} else {
-				strbuf_addf(err, "unable to append to '%s': %s",
-					    logfile, strerror(errno));
-				goto error;
-			}
-		}
+	if (raceproof_create_file(logfile, open_or_create_logfile, logfd)) {
+		if (errno == ENOENT)
+			strbuf_addf(err,
+				    "unable to create directory for '%s': "
+				    "%s",
+				    logfile, strerror(errno));
+		else if (errno == EISDIR)
+			strbuf_addf(err, "there are still logs under '%s'",
+				    logfile);
+		else
+			strbuf_addf(err, "unable to append to '%s': %s",
+				    logfile, strerror(errno));
 	}
 
 	if (*logfd >= 0)
 		adjust_shared_perm(logfile);
 
 	free(logfile);
-	return 0;
-
-error:
-	free(logfile);
-	return -1;
+	return (*logfd < 0) ? -1 : 0;
 }
 
 static int files_create_reflog(struct ref_store *ref_store, const char *refname,
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 8ced98e0fe8..446b568cef3 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -270,7 +270,7 @@  test_expect_success "(not) changed .git/$m" '
 '
 
 rm -f .git/logs/refs/heads/main
-test_expect_success "create $m (logged by touch)" '
+test_expect_success "create $m" '
 	test_config core.logAllRefUpdates false &&
 	GIT_COMMITTER_DATE="2005-05-26 23:30" \
 	git update-ref --create-reflog HEAD $A -m "Initial Creation" &&
@@ -318,9 +318,8 @@  test_expect_success 'symref empty directory removal' '
 
 cat >expect <<EOF
 $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	Initial Creation
-$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000	Switch
-$B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150860 +0000$TAB
 EOF
+
 test_expect_success "verifying $m's log (logged by touch)" '
 	test_when_finished "git update-ref -d $m && rm -rf .git/logs actual expect" &&
 	test-tool ref-store main for-each-reflog-ent $m > actual &&