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

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

From
Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 7, 2021, 13:36 UTC
Message-ID
<7a030cfd3e2521f08562ff7ecc00743b786c0e32.1631021808.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1067.v3.git.git.1631021808.gitgitgadget@gmail.com>
From: Han-Wen Nienhuys <hanwen@google.com>

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=fg@mail.gmail.com>.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 refs/files-backend.c  | 53 ++++++++++++++-----------------------------
 t/t1400-update-ref.sh |  5 ++--
 2 files changed, 19 insertions(+), 39 deletions(-)
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 &&
-- 
gitgitgadget
Previous: Han-Wen Nienhuys via GitGitGadgetNext: Han-Wen Nienhuys via GitGitGadget
Message 32 of 40 in “Gets rid of "if reflog exists, append to it regardless of config settings"”
  1. 0/4 Gets rid of "if reflog exists, append to it regardless of config settings"Han-Wen Nienhuys via GitGitGadget, Aug 30, 2021
  2. 1/4 test-ref-store: tweaks to for-each-reflog-ent formatHan-Wen Nienhuys via GitGitGadget, Aug 30, 2021
  3. Taylor BlauAug 30, 2021
  4. Taylor BlauAug 30, 2021
  5. Junio C HamanoAug 30, 2021
  6. Taylor BlauAug 30, 2021
  7. Junio C HamanoAug 30, 2021
  8. 2/4 t1400: use test-helper ref-store to inspect reflog contentsHan-Wen Nienhuys via GitGitGadget, Aug 30, 2021
  9. Junio C HamanoAug 30, 2021
  10. 3/4 refs: drop force_create argument of create_reflog APIHan-Wen Nienhuys via GitGitGadget, Aug 30, 2021
  11. Junio C HamanoAug 30, 2021
  12. 4/4 RFC: refs: reflog entries aren't written based on reflog existence.Han-Wen Nienhuys via GitGitGadget, Aug 30, 2021
  13. Taylor BlauAug 30, 2021
  14. Junio C HamanoAug 30, 2021
  15. 0/5 Gets rid of "if reflog exists, append to it regardless of config settings"Han-Wen Nienhuys via GitGitGadget, Sep 6, 2021
  16. 1/5 refs: trim newline from reflog messageHan-Wen Nienhuys via GitGitGadget, Sep 6, 2021
  17. Ævar Arnfjörð BjarmasonSep 6, 2021
  18. 2/5 test-ref-store: tweaks to for-each-reflog-ent formatHan-Wen Nienhuys via GitGitGadget, Sep 6, 2021
  19. Ævar Arnfjörð BjarmasonSep 6, 2021
  20. Han-Wen NienhuysSep 7, 2021
  21. Ævar Arnfjörð BjarmasonSep 7, 2021
  22. 3/5 t1400: use test-helper ref-store to inspect reflog contentsHan-Wen Nienhuys via GitGitGadget, Sep 6, 2021
  23. 4/5 refs: drop force_create argument of create_reflog APIHan-Wen Nienhuys via GitGitGadget, Sep 6, 2021
  24. Ævar Arnfjörð BjarmasonSep 6, 2021
  25. 5/5 RFC: refs: reflog entries aren't written based on reflog existence.Han-Wen Nienhuys via GitGitGadget, Sep 6, 2021
  26. Ævar Arnfjörð BjarmasonSep 6, 2021
  27. 0/7 Gets rid of "if reflog exists, append to it regardless of config settings"Han-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  28. 1/7 show-branch: show reflog messageHan-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  29. 5/7 refs: drop force_create argument of create_reflog APIHan-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  30. 2/7 refs: trim newline from reflog messageHan-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  31. 3/7 test-ref-store: tweaks to for-each-reflog-ent formatHan-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  32. 6/7 RFC: refs: reflog entries aren't written based on reflog existence.Han-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  33. 4/7 t1400: use test-helper ref-store to inspect reflog contentsHan-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  34. 7/7 refs: change log_ref_setup calling conventionHan-Wen Nienhuys via GitGitGadget, Sep 7, 2021
  35. Han-Wen NienhuysOct 5, 2021
  36. Junio C HamanoOct 6, 2021
  37. Junio C HamanoOct 6, 2021
  38. Junio C HamanoOct 7, 2021
  39. Han-Wen NienhuysNov 11, 2021
  40. Ævar Arnfjörð BjarmasonNov 11, 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.