diff mbox series

[v2,2/2] refs: allow @{n} to work with n-sized reflog

Message ID 18a35506b87356c1ae844d2686b2be6bd04567b2.1609923643.git.liu.denton@gmail.com (mailing list archive)
State Superseded
Headers show
Series refs: allow @{n} to work with n-sized reflog | expand

Commit Message

Denton Liu Jan. 6, 2021, 9:01 a.m. UTC
This sequence works

	$ git checkout -b newbranch
	$ git commit --allow-empty -m one
	$ git show -s newbranch@{1}

and shows the state that was immediately after the newbranch was
created.

But then if you do

	$ git reflog expire --expire=now refs/heads/newbranch
	$ git commit --allow=empty -m two
	$ git show -s newbranch@{1}

you'd be scolded with

	fatal: log for 'newbranch' only has 1 entries

While it is true that it has only 1 entry, we have enough
information in that single entry that records the transition between
the state in which the tip of the branch was pointing at commit
'one' to the new commit 'two' built on it, so we should be able to
answer "what object newbranch was pointing at?". But we refuse to
do so.

Make @{0} the special case where we use the new side to look up that
entry. Otherwise, look up @{n} using the old side of the (n-1)th entry
of the reflog.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 refs.c                      | 42 ++++++++++++++++++++++++-------------
 t/t1503-rev-parse-verify.sh |  4 ++--
 t/t1508-at-combinations.sh  | 16 ++++++++++++++
 3 files changed, 46 insertions(+), 16 deletions(-)

Comments

SZEDER Gábor Jan. 6, 2021, 9:59 a.m. UTC | #1
On Wed, Jan 06, 2021 at 01:01:54AM -0800, Denton Liu wrote:
> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
> index 4a9964e9dc..15aac6e77a 100755
> --- a/t/t1508-at-combinations.sh
> +++ b/t/t1508-at-combinations.sh
> @@ -99,4 +99,20 @@ test_expect_success 'create path with @' '
>  check "@:normal" blob content
>  check "@:fun@ny" blob content
>  
> +test_expect_success '@{1} works with only one reflog entry' '
> +	git checkout -B newbranch master &&
> +	git reflog expire --expire=now refs/heads/newbranch &&
> +	git commit --allow-empty -m "first after expiration" &&
> +	git rev-parse newbranch~ >expect &&
> +	git rev-parse newbranch@{1} >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '@{0} works with empty reflog' '
> +	git checkout -B newbranch master &&
> +	git reflog expire --expire=now refs/heads/newbranch &&
> +	git rev-parse newbranch >expect &&
> +	git rev-parse newbranch@{0} >actual &&
> +	test_cmp expect actual

You could use 'test_cmp_rev' in these two tests to spare a few lines
and to get a bit friendlier error message on failure.
diff mbox series

Patch

diff --git a/refs.c b/refs.c
index bfdd04aefd..9eb26d456d 100644
--- a/refs.c
+++ b/refs.c
@@ -900,40 +900,49 @@  static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
 		const char *message, void *cb_data)
 {
 	struct read_ref_at_cb *cb = cb_data;
+	int at_indexed_ent;
 
 	cb->tz = tz;
 	cb->date = timestamp;
 
-	if (timestamp <= cb->at_time || cb->cnt == 0) {
+	if (cb->cnt > 0)
+		cb->cnt--;
+	at_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);
+	if (timestamp <= cb->at_time || at_indexed_ent) {
 		set_read_ref_cutoffs(cb, timestamp, tz, message);
 		/*
 		 * we have not yet updated cb->[n|o]oid so they still
 		 * hold the values for the previous record.
 		 */
-		if (!is_null_oid(&cb->ooid)) {
-			oidcpy(cb->oid, noid);
-			if (!oideq(&cb->ooid, noid))
-				warning(_("log for ref %s has gap after %s"),
+		if (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))
+			warning(_("log for ref %s has gap after %s"),
 					cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));
-		}
-		else if (cb->date == cb->at_time)
+		if (at_indexed_ent)
+			oidcpy(cb->oid, ooid);
+		else if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)
 			oidcpy(cb->oid, noid);
 		else if (!oideq(noid, cb->oid))
 			warning(_("log for ref %s unexpectedly ended on %s"),
 				cb->refname, show_date(cb->date, cb->tz,
 						       DATE_MODE(RFC2822)));
-		cb->reccnt++;
-		oidcpy(&cb->ooid, ooid);
-		oidcpy(&cb->noid, noid);
 		cb->found_it = 1;
-		return 1;
 	}
 	cb->reccnt++;
 	oidcpy(&cb->ooid, ooid);
 	oidcpy(&cb->noid, noid);
-	if (cb->cnt > 0)
-		cb->cnt--;
-	return 0;
+	return cb->found_it;
+}
+
+static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid,
+				  const char *email, timestamp_t timestamp,
+				  int tz, const char *message, void *cb_data)
+{
+	struct read_ref_at_cb *cb = cb_data;
+
+	set_read_ref_cutoffs(cb, timestamp, tz, message);
+	oidcpy(cb->oid, noid);
+	/* We just want the first entry */
+	return 1;
 }
 
 static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,
@@ -967,6 +976,11 @@  int read_ref_at(struct ref_store *refs, const char *refname,
 	cb.cutoff_cnt = cutoff_cnt;
 	cb.oid = oid;
 
+	if (cb.cnt == 0) {
+		refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);
+		return 0;
+	}
+
 	refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);
 
 	if (!cb.reccnt) {
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index dc9fe3cbf1..a7e9b4863d 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -86,8 +86,8 @@  test_expect_success 'fails silently when using -q' '
 test_expect_success 'fails silently when using -q with deleted reflogs' '
 	ref=$(git rev-parse HEAD) &&
 	git update-ref --create-reflog -m "message for refs/test" refs/test "$ref" &&
-	git reflog delete --updateref --rewrite refs/test@{0} &&
-	test_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&
+	git reflog delete --updateref --rewrite refs/test@{1} &&
+	test_must_fail git rev-parse -q --verify refs/test@{1} >error 2>&1 &&
 	test_must_be_empty error
 '
 
diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
index 4a9964e9dc..15aac6e77a 100755
--- a/t/t1508-at-combinations.sh
+++ b/t/t1508-at-combinations.sh
@@ -99,4 +99,20 @@  test_expect_success 'create path with @' '
 check "@:normal" blob content
 check "@:fun@ny" blob content
 
+test_expect_success '@{1} works with only one reflog entry' '
+	git checkout -B newbranch master &&
+	git reflog expire --expire=now refs/heads/newbranch &&
+	git commit --allow-empty -m "first after expiration" &&
+	git rev-parse newbranch~ >expect &&
+	git rev-parse newbranch@{1} >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '@{0} works with empty reflog' '
+	git checkout -B newbranch master &&
+	git reflog expire --expire=now refs/heads/newbranch &&
+	git rev-parse newbranch >expect &&
+	git rev-parse newbranch@{0} >actual &&
+	test_cmp expect actual
+'
 test_done