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

[PATCH 2/3] get_oid_basic(): special-case ref@{n} for oldest reflog entry

From
Jeff King <peff@peff.net>
Date
Feb 26, 2024, 10:04 UTC
Message-ID
<20240226100407.GB2685600@coredump.intra.peff.net>
In-Reply-To
<20240226100010.GA1214708@coredump.intra.peff.net>

The goal of 6436a20284 (refs: allow @{n} to work with n-sized reflog, 2021-01-07) was that if we have "n" entries in a reflog, we should still be able to resolve ref@{n} by looking at the "old" value of the oldest entry.

Commit 6436a20284 tried to put the logic into read_ref_at() by shifting its idea of "n" by one. But we reverted that in the previous commit, since it led to bugs in other callers which cared about the details of the reflog entry we found. Instead, let's put the special case into the caller that resolves @{n}, as it cares only about the oid.

read_ref_at() is even kind enough to return the "old" value from the final reflog; it just returns "1" to signal to us that we ran off the end of the reflog. But we can notice in the caller that we read just enough records for that "old" value to be the one we're looking for, and use it.

Note that read_ref_at() could notice this case, too, and just return 0. But we don't want to do that, because the caller must be made aware that we only found the oid, not an actual reflog entry (and the call sites in show-branch do care about this).

There is one complication, though. When read_ref_at() hits a truncated reflog, it will return the "old" value of the oldest entry only if it is not the null oid. Otherwise, it actually returns the "new" value from that entry! This bit of fudging is due to d1a4489a56 (avoid null SHA1 in oldest reflog, 2008-07-08), where asking for "ref@{20.years.ago}" for a ref created recently will produce the initial value as a convenience (even though technically it did not exist 20 years ago).

But this convenience is only useful for time-based cutoffs. For count-based cutoffs, get_oid_basic() has always simply complained about going too far back:

  $ git rev-parse HEAD@{20}
  fatal: log for 'HEAD' only has 16 entries

and we should continue to do so, rather than returning a nonsense value (there's even a test in t1508 already which covers this). So let's have the d1a4489a56 code kick in only when doing timestamp-based cutoffs.

Signed-off-by: Jeff King <peff@peff.net>
---
 object-name.c              | 9 +++++++++
 refs.c                     | 2 +-
 t/t1508-at-combinations.sh | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/object-name.c b/object-name.c
index 3a2ef5d680..511f09bc0f 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1034,6 +1034,15 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 						len, str,
 						show_date(co_time, co_tz, DATE_MODE(RFC2822)));
 				}
+			} else if (nth == co_cnt && !is_null_oid(oid)) {
+				/*
+				 * We were asked for the Nth reflog (counting
+				 * from 0), but there were only N entries.
+				 * read_ref_at() will have returned "1" to tell
+				 * us it did not find an entry, but it did
+				 * still fill in the oid with the "old" value,
+				 * which we can use.
+				 */
 			} else {
 				if (flags & GET_OID_QUIETLY) {
 					exit(128);
diff --git a/refs.c b/refs.c
index ba1a4db754..6b826b002e 100644
--- a/refs.c
+++ b/refs.c
@@ -1083,7 +1083,7 @@ static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid
 
 	set_read_ref_cutoffs(cb, timestamp, tz, message);
 	oidcpy(cb->oid, ooid);
-	if (is_null_oid(cb->oid))
+	if (cb->at_time && is_null_oid(cb->oid))
 		oidcpy(cb->oid, noid);
 	/* We just want the first entry */
 	return 1;
diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
index 3e5f32f604..370bf7137e 100755
--- a/t/t1508-at-combinations.sh
+++ b/t/t1508-at-combinations.sh
@@ -103,7 +103,7 @@ test_expect_success 'create path with @' '
 check "@:normal" blob content
 check "@:fun@ny" blob content
 
-test_expect_failure '@{1} works with only one reflog entry' '
+test_expect_success '@{1} works with only one reflog entry' '
 	git checkout -B newbranch main &&
 	git reflog expire --expire=now refs/heads/newbranch &&
 	git commit --allow-empty -m "first after expiration" &&
-- 
2.44.0.rc2.424.gbdbf4d014b
Previous: Jeff KingNext: Junio C Hamano
Message 11 of 27 in “Segfault: git show-branch --reflog refs/pullreqs/1”
  1. Yasushi SHOJIFeb 21, 2024
  2. Jeff KingFeb 21, 2024
  3. Patrick SteinhardtFeb 21, 2024
  4. Jeff KingFeb 21, 2024
  5. Junio C HamanoFeb 21, 2024
  6. Patrick SteinhardtFeb 22, 2024
  7. Junio C HamanoFeb 22, 2024
  8. Jeff KingFeb 22, 2024
  9. 0/3 show-branch --reflog fixesJeff King, Feb 26, 2024
  10. 1/3 Revert "refs: allow @{n} to work with n-sized reflog"Jeff King, Feb 26, 2024
  11. 2/3 get_oid_basic(): special-case ref@{n} for oldest reflog entryJeff King, Feb 26, 2024
  12. Junio C HamanoFeb 26, 2024
  13. 3/3 read_ref_at(): special-case ref@{0} for an empty reflogJeff King, Feb 26, 2024
  14. Jeff KingFeb 26, 2024
  15. Junio C HamanoFeb 26, 2024
  16. Jeff KingFeb 27, 2024
  17. Junio C HamanoFeb 26, 2024
  18. Jeff KingFeb 27, 2024
  19. Junio C HamanoFeb 27, 2024
  20. Patrick SteinhardtFeb 21, 2024
  21. 0/2 Detect empty or missing reflogs with `ref@{0}`Patrick Steinhardt, Feb 21, 2024
  22. 1/2 object-name: detect and report empty reflogsPatrick Steinhardt, Feb 21, 2024
  23. Kristoffer HaugsbakkFeb 21, 2024
  24. Eric SunshineFeb 21, 2024
  25. Jeff KingFeb 21, 2024
  26. 2/2 builtin/show-branch: detect empty reflogsPatrick Steinhardt, Feb 21, 2024
  27. Jeff KingFeb 21, 2024

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.