{"thread":{"id":"54917","subject":"[PATCH] refs: allow @{n} to work with n-sized reflog","startedAt":"2021-01-02T01:37:53Z","lastAt":"2021-01-12T06:28:52Z","messageCount":21,"participants":["Denton Liu","Martin Ågren","SZEDER Gábor","Junio C Hamano","Simon Ruderich"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"413308","messageId":"0c6885f15f5ce0be28142d9c69724362e72481a9.1609551262.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":null,"subject":"[PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-02T01:36:06Z","receivedAt":"2021-01-02T01:37:53Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This sequence works\n\n\t$ git checkout -b newbranch\n\t$ git commit --allow-empty -m one\n\t$ git show -s newbranch@{1}\n\nand shows the state that was immediately after the newbranch was\ncreated.\n\nBut then if you do\n\n\t$ git reflog expire --expire=now refs/heads/newbranch\n\t$ git commit --allow=empty -m two\n\t$ git show -s newbranch@{1}\n\nyou'd be scolded with\n\n\tfatal: log for 'newbranch' only has 1 entries\n\nWhile it is true that it has only 1 entry, we have enough\ninformation in that single entry that records the transition between\nthe state in which the tip of the branch was pointing at commit\n'one' to the new commit 'two' built on it, so we should be able to\nanswer \"what object newbranch was pointing at?\". But we refuse to\ndo so.\n\nMake @{0} the special case where we use the new side to look up that\nentry. Otherwise, look up @{n} using the old side of the (n-1)th entry\nof the reflog.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Denton Liu \n---\n refs.c | 48 +++++++++++++++++++++++++++----------\n t/t1503-rev-parse-verify.sh | 17 +++++++++++--\n 2 files changed, 50 insertions(+), 15 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 13dc2c3291..c35c61a009 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -887,12 +887,16 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *message, void *cb_data)\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n+\tint at_indexed_ent;\n \n \tcb->reccnt++;\n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n-\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n+\tif (cb->cnt > 0)\n+\t\tcb->cnt--;\n+\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n+\tif (timestamp <= cb->at_time || at_indexed_ent) {\n \t\tif (cb->msg)\n \t\t\t*cb->msg = xstrdup(message);\n \t\tif (cb->cutoff_time)\n@@ -905,28 +909,41 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\t * we have not yet updated cb->[n|o]oid so they still\n \t\t * hold the values for the previous record.\n \t\t */\n-\t\tif (!is_null_oid(&cb->ooid)) {\n-\t\t\toidcpy(cb->oid, noid);\n-\t\t\tif (!oideq(&cb->ooid, noid))\n-\t\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n+\t\tif (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))\n+\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n \t\t\t\t\tcb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));\n-\t\t}\n-\t\telse if (cb->date == cb->at_time)\n+\t\tif (at_indexed_ent)\n+\t\t\toidcpy(cb->oid, ooid);\n+\t\telse if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)\n \t\t\toidcpy(cb->oid, noid);\n \t\telse if (!oideq(noid, cb->oid))\n \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n \t\t\t\t\t\t DATE_MODE(RFC2822)));\n-\t\toidcpy(&cb->ooid, ooid);\n-\t\toidcpy(&cb->noid, noid);\n \t\tcb->found_it = 1;\n-\t\treturn 1;\n \t}\n \toidcpy(&cb->ooid, ooid);\n \toidcpy(&cb->noid, noid);\n-\tif (cb->cnt > 0)\n-\t\tcb->cnt--;\n-\treturn 0;\n+\treturn cb->found_it;\n+}\n+\n+static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid,\n+\t\t\t\t const char *email, timestamp_t timestamp,\n+\t\t\t\t int tz, const char *message, void *cb_data)\n+{\n+\tstruct read_ref_at_cb *cb = cb_data;\n+\n+\tif (cb->msg)\n+\t\t*cb->msg = xstrdup(message);\n+\tif (cb->cutoff_time)\n+\t\t*cb->cutoff_time = timestamp;\n+\tif (cb->cutoff_tz)\n+\t\t*cb->cutoff_tz = tz;\n+\tif (cb->cutoff_cnt)\n+\t\t*cb->cutoff_cnt = cb->reccnt;\n+\toidcpy(cb->oid, noid);\n+\t/* We just want the first entry */\n+\treturn 1;\n }\n \n static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,\n@@ -967,6 +984,11 @@ int read_ref_at(struct ref_store *refs, const char *refname,\n \tcb.cutoff_cnt = cutoff_cnt;\n \tcb.oid = oid;\n \n+\tif (cb.cnt == 0) {\n+\t\trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);\n+\t\treturn 0;\n+\t}\n+\n \trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);\n \n \tif (!cb.reccnt) {\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex dc9fe3cbf1..ed4a366e85 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -86,8 +86,8 @@ test_expect_success 'fails silently when using -q' '\n test_expect_success 'fails silently when using -q with deleted reflogs' '\n \tref=$(git rev-parse HEAD) &&\n \tgit update-ref --create-reflog -m \"message for refs/test\" refs/test \"$ref\" &&\n-\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n-\ttest_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&\n+\tgit reflog delete --updateref --rewrite refs/test@{1} &&\n+\ttest_must_fail git rev-parse -q --verify refs/test@{1} >error 2>&1 &&\n \ttest_must_be_empty error\n '\n \n@@ -139,6 +139,19 @@ test_expect_success 'master@{n} for various n' '\n \ttest_must_fail git rev-parse --verify master@{$Np1}\n '\n \n+test_expect_success '@{1} works with only one reflog entry' '\n+\tgit checkout -B newbranch &&\n+\tgit reflog expire --expire=now refs/heads/newbranch &&\n+\tgit commit --allow-empty -mexpired &&\n+\tgit rev-parse --verify newbranch@{1}\n+'\n+\n+test_expect_success '@{0} works with empty reflog' '\n+\tgit checkout -B newbranch &&\n+\tgit reflog expire --expire=now refs/heads/newbranch &&\n+\tgit rev-parse --verify newbranch@{0}\n+'\n+\n test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '\n \tln -s does-not-exist .git/refs/heads/broken &&\n \ttest_must_fail git rev-parse --verify broken\n-- \n2.30.0\n\n"},{"id":"413323","messageId":"CAN0heSqotLJp3T5XbmeibVcg0RSyqH=69c1tV8-k153-_=CTxg@mail.gmail.com","threadId":"54917","inReplyTo":"0c6885f15f5ce0be28142d9c69724362e72481a9.1609551262.git.liu.denton@gmail.com","subject":"Re: [PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-01-02T22:30:26Z","receivedAt":"2021-01-02T22:31:26Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Sat, 2 Jan 2021 at 02:41, Denton Liu wrote:\n>\n> But then if you do\n>\n> $ git reflog expire --expire=now refs/heads/newbranch\n> $ git commit --allow=empty -m two\n> $ git show -s newbranch@{1}\n>\n> you'd be scolded with\n>\n> fatal: log for 'newbranch' only has 1 entries\n>\n> While it is true that it has only 1 entry, we have enough\n> information in that single entry that records the transition between\n> the state in which the tip of the branch was pointing at commit\n> 'one' to the new commit 'two' built on it, so we should be able to\n> answer \"what object newbranch was pointing at?\". But we refuse to\n> do so.\n\nThe basic idea seems to make sense to me...\n\n> Make @{0} the special case where we use the new side to look up that\n> entry. Otherwise, look up @{n} using the old side of the (n-1)th entry\n> of the reflog.\n\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -887,12 +887,16 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> const char *message, void *cb_data)\n> {\n> struct read_ref_at_cb *cb = cb_data;\n> + int at_indexed_ent;\n>\n> cb->reccnt++;\n> cb->tz = tz;\n> cb->date = timestamp;\n>\n> - if (timestamp <= cb->at_time || cb->cnt == 0) {\n> + if (cb->cnt > 0)\n> + cb->cnt--;\n> + at_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n> + if (timestamp <= cb->at_time || at_indexed_ent) {\n\n... but I can't really say anything about the implementation.\n\n> +test_expect_success '@{1} works with only one reflog entry' '\n> + git checkout -B newbranch &&\n> + git reflog expire --expire=now refs/heads/newbranch &&\n> + git commit --allow-empty -mexpired &&\n\nMinor nit: not sure about \"expired\" -- maybe \"first after expiration\".\n\n> + git rev-parse --verify newbranch@{1}\n> +'\n\nShould this capture the output and compare it to, e.g., `git rev-parse\nnewbranch^`?\n\n> +test_expect_success '@{0} works with empty reflog' '\n> + git checkout -B newbranch &&\n> + git reflog expire --expire=now refs/heads/newbranch &&\n> + git rev-parse --verify newbranch@{0}\n> +'\n\nSame here, but comparing to `git rev-parse newbranch`? Both of these\nchecks seem worthwhile to make sure that we don't just answer\n*something*, but that we actually get the right answer, as per your\n\"redefinition\".\n\nSpeaking of redefinition, does this warrant an update of the\ndocumentation? That's a genuine question -- having browsed git-reflog(1)\nand gitrevisions(7) a bit, I'm not sure.\n\nMartin\n"},{"id":"413327","messageId":"X/EcucyFs3Ecuc2K@generichostname","threadId":"54917","inReplyTo":"0c6885f15f5ce0be28142d9c69724362e72481a9.1609551262.git.liu.denton@gmail.com","subject":"Re: [PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-03T01:24:09Z","receivedAt":"2021-01-03T01:25:15Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Fri, Jan 01, 2021 at 05:36:06PM -0800, Denton Liu wrote:\n> This sequence works\n> \n> \t$ git checkout -b newbranch\n> \t$ git commit --allow-empty -m one\n> \t$ git show -s newbranch@{1}\n> \n> and shows the state that was immediately after the newbranch was\n> created.\n> \n> But then if you do\n> \n> \t$ git reflog expire --expire=now refs/heads/newbranch\n> \t$ git commit --allow=empty -m two\n> \t$ git show -s newbranch@{1}\n> \n> you'd be scolded with\n> \n> \tfatal: log for 'newbranch' only has 1 entries\n> \n> While it is true that it has only 1 entry, we have enough\n> information in that single entry that records the transition between\n> the state in which the tip of the branch was pointing at commit\n> 'one' to the new commit 'two' built on it, so we should be able to\n> answer \"what object newbranch was pointing at?\". But we refuse to\n> do so.\n> \n> Make @{0} the special case where we use the new side to look up that\n> entry. Otherwise, look up @{n} using the old side of the (n-1)th entry\n> of the reflog.\n> \n> Suggested-by: Junio C Hamano \n> Signed-off-by: Denton Liu \n\nI forgot to mention that the original thread that spawned this idea is\nhere:\n\n\thttps://lore.kernel.org/git/xmqqzh8zgcfp.fsf@gitster.c.googlers.com/\n"},{"id":"413438","messageId":"20210105085221.GM8396@szeder.dev","threadId":"54917","inReplyTo":"0c6885f15f5ce0be28142d9c69724362e72481a9.1609551262.git.liu.denton@gmail.com","subject":"Re: [PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-05T08:52:21Z","receivedAt":"2021-01-05T08:53:07Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Jan 01, 2021 at 05:36:06PM -0800, Denton Liu wrote:\n> This sequence works\n> \n> \t$ git checkout -b newbranch\n> \t$ git commit --allow-empty -m one\n> \t$ git show -s newbranch@{1}\n> \n> and shows the state that was immediately after the newbranch was\n> created.\n> \n> But then if you do\n> \n> \t$ git reflog expire --expire=now refs/heads/newbranch\n> \t$ git commit --allow=empty -m two\n> \t$ git show -s newbranch@{1}\n> \n> you'd be scolded with\n> \n> \tfatal: log for 'newbranch' only has 1 entries\n> \n> While it is true that it has only 1 entry, we have enough\n> information in that single entry that records the transition between\n> the state in which the tip of the branch was pointing at commit\n> 'one' to the new commit 'two' built on it, so we should be able to\n> answer \"what object newbranch was pointing at?\". But we refuse to\n> do so.\n\nGreat! I've run into this issue quite a while ago with regular 'git\ngc' expiring too old reflog entries, and wondered while the @{N}\nnotation errored out while the information was clearly still there in\nthe reflog.\n\nhttps://public-inbox.org/git/20130619125059.GD20052@goldbirke/T/#u\n\n> @@ -139,6 +139,19 @@ test_expect_success 'master@{n} for various n' '\n> \ttest_must_fail git rev-parse --verify master@{$Np1}\n> '\n> \n> +test_expect_success '@{1} works with only one reflog entry' '\n> +\tgit checkout -B newbranch &&\n> +\tgit reflog expire --expire=now refs/heads/newbranch &&\n> +\tgit commit --allow-empty -mexpired &&\n> +\tgit rev-parse --verify newbranch@{1}\n> +'\n> +\n> +test_expect_success '@{0} works with empty reflog' '\n> +\tgit checkout -B newbranch &&\n> +\tgit reflog expire --expire=now refs/heads/newbranch &&\n> +\tgit rev-parse --verify newbranch@{0}\n> +'\n\nI agree with Martin about these tests: not failing is one thing, but\nwe should make sure that the right value is printed.\n\n> test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '\n> \tln -s does-not-exist .git/refs/heads/broken &&\n> \ttest_must_fail git rev-parse --verify broken\n> -- \n> 2.30.0\n> \n"},{"id":"413532","messageId":"xmqqk0sqvcby.fsf@gitster.c.googlers.com","threadId":"54917","inReplyTo":"0c6885f15f5ce0be28142d9c69724362e72481a9.1609551262.git.liu.denton@gmail.com","subject":"Re: [PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-06T05:55:29Z","receivedAt":"2021-01-06T05:56:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> This sequence works\n>\n> \t$ git checkout -b newbranch\n> \t$ git commit --allow-empty -m one\n> \t$ git show -s newbranch@{1}\n>\n> and shows the state that was immediately after the newbranch was\n> created.\n>\n> But then if you do\n>\n> \t$ git reflog expire --expire=now refs/heads/newbranch\n> \t$ git commit --allow=empty -m two\n> \t$ git show -s newbranch@{1}\n>\n> you'd be scolded with\n>\n> \tfatal: log for 'newbranch' only has 1 entries\n>\n> While it is true that it has only 1 entry, we have enough\n> information in that single entry that records the transition between\n> the state in which the tip of the branch was pointing at commit\n> 'one' to the new commit 'two' built on it, so we should be able to\n> answer \"what object newbranch was pointing at?\". But we refuse to\n> do so.\n\nYeah, I am often hit and irritated by this behaviour.\n\n> Make @{0} the special case where we use the new side to look up that\n> entry. Otherwise, look up @{n} using the old side of the (n-1)th entry\n> of the reflog.\n\nOK.\n\n> diff --git a/refs.c b/refs.c\n> index 13dc2c3291..c35c61a009 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -887,12 +887,16 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> \t\tconst char *message, void *cb_data)\n> {\n> \tstruct read_ref_at_cb *cb = cb_data;\n> +\tint at_indexed_ent;\n> \n> \tcb->reccnt++;\n> \tcb->tz = tz;\n> \tcb->date = timestamp;\n> \n> -\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n> +\tif (cb->cnt > 0)\n> +\t\tcb->cnt--;\n> +\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n\nThe code treats two cases identically (i.e. the case where cb->cnt\nwas originally zero, and one). Is that intended?\n\nI thought the code was to special case only @{0}, but with this\nconditional decrement, cb->cnt==0 would not be usable by the rest\nof the code as the \"we must read the new side instead\" signal. Is\nthat why null-ness of ooid is also tested here? It is hard to tell\nthe intention because \"at_indexed_ent\" does not quite tell me what\nthe code wants to use the variable for.\n\n> +\tif (timestamp <= cb->at_time || at_indexed_ent) {\n> \t\tif (cb->msg)\n> \t\t\t*cb->msg = xstrdup(message);\n> \t\tif (cb->cutoff_time)\n> @@ -905,28 +909,41 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> \t\t * we have not yet updated cb->[n|o]oid so they still\n> \t\t * hold the values for the previous record.\n> \t\t */\n> -\t\tif (!is_null_oid(&cb->ooid)) {\n> -\t\t\toidcpy(cb->oid, noid);\n> -\t\t\tif (!oideq(&cb->ooid, noid))\n> -\t\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n> +\t\tif (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))\n> +\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n> \t\t\t\t\tcb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));\n> -\t\t}\n> -\t\telse if (cb->date == cb->at_time)\n> +\t\tif (at_indexed_ent)\n> +\t\t\toidcpy(cb->oid, ooid);\n> +\t\telse if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)\n> \t\t\toidcpy(cb->oid, noid);\n> \t\telse if (!oideq(noid, cb->oid))\n> \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n> \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n> \t\t\t\t\t\t DATE_MODE(RFC2822)));\n> -\t\toidcpy(&cb->ooid, ooid);\n> -\t\toidcpy(&cb->noid, noid);\n> \t\tcb->found_it = 1;\n> -\t\treturn 1;\n> \t}\n> \toidcpy(&cb->ooid, ooid);\n> \toidcpy(&cb->noid, noid);\n> -\tif (cb->cnt > 0)\n> -\t\tcb->cnt--;\n> -\treturn 0;\n> +\treturn cb->found_it;\n> +}\n> +\n> +static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid,\n> +\t\t\t\t const char *email, timestamp_t timestamp,\n> +\t\t\t\t int tz, const char *message, void *cb_data)\n> +{\n> +\tstruct read_ref_at_cb *cb = cb_data;\n> +\n> +\tif (cb->msg)\n> +\t\t*cb->msg = xstrdup(message);\n> +\tif (cb->cutoff_time)\n> +\t\t*cb->cutoff_time = timestamp;\n> +\tif (cb->cutoff_tz)\n> +\t\t*cb->cutoff_tz = tz;\n> +\tif (cb->cutoff_cnt)\n> +\t\t*cb->cutoff_cnt = cb->reccnt;\n> +\toidcpy(cb->oid, noid);\n> +\t/* We just want the first entry */\n> +\treturn 1;\n> }\n\nThe similarity of this to read_ref_at_ent_oldest is somehow\nstriking. Do we really need to invent a new callback?\n\n> static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,\n> @@ -967,6 +984,11 @@ int read_ref_at(struct ref_store *refs, const char *refname,\n> \tcb.cutoff_cnt = cutoff_cnt;\n> \tcb.oid = oid;\n> \n> +\tif (cb.cnt == 0) {\n> +\t\trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);\n> +\t\treturn 0;\n> +\t}\n> +\n> \trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);\n> \n> \tif (!cb.reccnt) {\n> diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\n> index dc9fe3cbf1..ed4a366e85 100755\n> --- a/t/t1503-rev-parse-verify.sh\n> +++ b/t/t1503-rev-parse-verify.sh\n> @@ -86,8 +86,8 @@ test_expect_success 'fails silently when using -q' '\n> test_expect_success 'fails silently when using -q with deleted reflogs' '\n> \tref=$(git rev-parse HEAD) &&\n> \tgit update-ref --create-reflog -m \"message for refs/test\" refs/test \"$ref\" &&\n> -\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n> -\ttest_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&\n> +\tgit reflog delete --updateref --rewrite refs/test@{1} &&\n> +\ttest_must_fail git rev-parse -q --verify refs/test@{1} >error 2>&1 &&\n> \ttest_must_be_empty error\n> '\n> \n> @@ -139,6 +139,19 @@ test_expect_success 'master@{n} for various n' '\n> \ttest_must_fail git rev-parse --verify master@{$Np1}\n> '\n> \n> +test_expect_success '@{1} works with only one reflog entry' '\n> +\tgit checkout -B newbranch &&\n> +\tgit reflog expire --expire=now refs/heads/newbranch &&\n> +\tgit commit --allow-empty -mexpired &&\n> +\tgit rev-parse --verify newbranch@{1}\n> +'\n> +\n> +test_expect_success '@{0} works with empty reflog' '\n> +\tgit checkout -B newbranch &&\n> +\tgit reflog expire --expire=now refs/heads/newbranch &&\n> +\tgit rev-parse --verify newbranch@{0}\n> +'\n> +\n> test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '\n> \tln -s does-not-exist .git/refs/heads/broken &&\n> \ttest_must_fail git rev-parse --verify broken\n"},{"id":"413542","messageId":"X/V0DU+CD6mS36dK@generichostname","threadId":"54917","inReplyTo":"xmqqk0sqvcby.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-06T08:25:49Z","receivedAt":"2021-01-06T08:26:37Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Junio,\n\nOn Tue, Jan 05, 2021 at 09:55:29PM -0800, Junio C Hamano wrote:\n> Denton Liu writes:\n> \n> > This sequence works\n> >\n> > \t$ git checkout -b newbranch\n> > \t$ git commit --allow-empty -m one\n> > \t$ git show -s newbranch@{1}\n> >\n> > and shows the state that was immediately after the newbranch was\n> > created.\n> >\n> > But then if you do\n> >\n> > \t$ git reflog expire --expire=now refs/heads/newbranch\n> > \t$ git commit --allow=empty -m two\n> > \t$ git show -s newbranch@{1}\n> >\n> > you'd be scolded with\n> >\n> > \tfatal: log for 'newbranch' only has 1 entries\n> >\n> > While it is true that it has only 1 entry, we have enough\n> > information in that single entry that records the transition between\n> > the state in which the tip of the branch was pointing at commit\n> > 'one' to the new commit 'two' built on it, so we should be able to\n> > answer \"what object newbranch was pointing at?\". But we refuse to\n> > do so.\n> \n> Yeah, I am often hit and irritated by this behaviour.\n\nYep, this was inspired by one of your emails ;)\n\n> > diff --git a/refs.c b/refs.c\n> > index 13dc2c3291..c35c61a009 100644\n> > --- a/refs.c\n> > +++ b/refs.c\n> > @@ -887,12 +887,16 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> > \t\tconst char *message, void *cb_data)\n> > {\n> > \tstruct read_ref_at_cb *cb = cb_data;\n> > +\tint at_indexed_ent;\n> > \n> > \tcb->reccnt++;\n> > \tcb->tz = tz;\n> > \tcb->date = timestamp;\n> > \n> > -\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n> > +\tif (cb->cnt > 0)\n> > +\t\tcb->cnt--;\n> > +\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n> \n> The code treats two cases identically (i.e. the case where cb->cnt\n> was originally zero, and one). Is that intended?\n\nIt shouldn't be possible for cb->cnt == 0 on the first iteration\nbecause there's a special-case check at [0]. As a result, it can only be\n-1 or >= 1 on the first iteration.\n\nThe -1 case happens when we're doing date-based lookup and that's what\nthis if is intended to handle.\n\nIn the case where it's >= 1, we will always enter the if and it will\nalways pre-decrement. This essentially gets us the n-1 behaviour.\n\n> I thought the code was to special case only @{0}, but with this\n> conditional decrement, cb->cnt==0 would not be usable by the rest\n> of the code as the \"we must read the new side instead\" signal. Is\n> that why null-ness of ooid is also tested here? It is hard to tell\n> the intention because \"at_indexed_ent\" does not quite tell me what\n> the code wants to use the variable for.\n\nThe null-ness of the ooid is needed because on the last entry of the\nreflog, ooid will be null so we should skip that.\n\n\"at_indexed_ent\" is meant to signal when we are indexing the reflog\nnumerically (as opposed to by date), we have arrived at the correct\nentry. If you have a more fitting name, I'm open to suggestions.\n\n> > +\tif (timestamp <= cb->at_time || at_indexed_ent) {\n> > \t\tif (cb->msg)\n> > \t\t\t*cb->msg = xstrdup(message);\n> > \t\tif (cb->cutoff_time)\n> > @@ -905,28 +909,41 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> > \t\t * we have not yet updated cb->[n|o]oid so they still\n> > \t\t * hold the values for the previous record.\n> > \t\t */\n> > -\t\tif (!is_null_oid(&cb->ooid)) {\n> > -\t\t\toidcpy(cb->oid, noid);\n> > -\t\t\tif (!oideq(&cb->ooid, noid))\n> > -\t\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n> > +\t\tif (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))\n> > +\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n> > \t\t\t\t\tcb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));\n> > -\t\t}\n> > -\t\telse if (cb->date == cb->at_time)\n> > +\t\tif (at_indexed_ent)\n> > +\t\t\toidcpy(cb->oid, ooid);\n> > +\t\telse if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)\n> > \t\t\toidcpy(cb->oid, noid);\n> > \t\telse if (!oideq(noid, cb->oid))\n> > \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n> > \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n> > \t\t\t\t\t\t DATE_MODE(RFC2822)));\n> > -\t\toidcpy(&cb->ooid, ooid);\n> > -\t\toidcpy(&cb->noid, noid);\n> > \t\tcb->found_it = 1;\n> > -\t\treturn 1;\n> > \t}\n> > \toidcpy(&cb->ooid, ooid);\n> > \toidcpy(&cb->noid, noid);\n> > -\tif (cb->cnt > 0)\n> > -\t\tcb->cnt--;\n> > -\treturn 0;\n> > +\treturn cb->found_it;\n> > +}\n> > +\n> > +static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid,\n> > +\t\t\t\t const char *email, timestamp_t timestamp,\n> > +\t\t\t\t int tz, const char *message, void *cb_data)\n> > +{\n> > +\tstruct read_ref_at_cb *cb = cb_data;\n> > +\n> > +\tif (cb->msg)\n> > +\t\t*cb->msg = xstrdup(message);\n> > +\tif (cb->cutoff_time)\n> > +\t\t*cb->cutoff_time = timestamp;\n> > +\tif (cb->cutoff_tz)\n> > +\t\t*cb->cutoff_tz = tz;\n> > +\tif (cb->cutoff_cnt)\n> > +\t\t*cb->cutoff_cnt = cb->reccnt;\n> > +\toidcpy(cb->oid, noid);\n> > +\t/* We just want the first entry */\n> > +\treturn 1;\n> > }\n> \n> The similarity of this to read_ref_at_ent_oldest is somehow\n> striking. Do we really need to invent a new callback?\n\nUnfortunately, yes. The alternative is to add a flag into\n`struct read_ref_at_cb` and we could conditionally choose whether or not\nto copy noid or ooid but this seems like the lesser of two evils.\n\nThe duplicated part,\n\n\tif (cb->msg)\n\t\t*cb->msg = xstrdup(message);\n\tif (cb->cutoff_time)\n\t\t*cb->cutoff_time = timestamp;\n\tif (cb->cutoff_tz)\n\t\t*cb->cutoff_tz = tz;\n\tif (cb->cutoff_cnt)\n\t\t*cb->cutoff_cnt = cb->reccnt;\n\nis actually repeated three times -- once in each of the callbacks. I\nconsidered extracting factoring it out into a function but I was on the\nfence because the function would still have some duplication since it'd\nstill require cb, message, timestamp and tz to all be passed in.\n\n> > static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,\n> > @@ -967,6 +984,11 @@ int read_ref_at(struct ref_store *refs, const char *refname,\n> > \tcb.cutoff_cnt = cutoff_cnt;\n> > \tcb.oid = oid;\n> > \n> > +\tif (cb.cnt == 0) {\n> > +\t\trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);\n> > +\t\treturn 0;\n> > +\t}\n> > +\n\n[0]\n\n> > \trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);\n> > \n> > \tif (!cb.reccnt) {\n\nThanks,\nDenton\n"},{"id":"413548","messageId":"8f14ec39970b6cbf9b6615485316063306706e6a.1609923643.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"cover.1609923643.git.liu.denton@gmail.com","subject":"[PATCH v2 1/2] refs: factor out set_read_ref_cutoffs()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-06T09:01:53Z","receivedAt":"2021-01-06T09:03:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This block of code is duplicated twice. In a future commit, it will be\nduplicated for a third time. Factor out the common functionality into\nset_read_ref_cutoffs().\n\nIn the case of read_ref_at_ent(), we are incrementing `cb->reccnt` at the\nbeginning of the function. Move these to right before the return so that\nthe `cb->reccnt - 1` is changed to `cb->reccnt` and it can be cleanly\nfactored out into set_read_ref_cutoffs(). The duplication of the\nincrement statements will be removed in a future patch.\n\nSigned-off-by: Denton Liu \n---\n refs.c | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 13dc2c3291..bfdd04aefd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -882,25 +882,30 @@ struct read_ref_at_cb {\n \tint *cutoff_cnt;\n };\n \n+static void set_read_ref_cutoffs(struct read_ref_at_cb *cb,\n+\t\ttimestamp_t timestamp, int tz, const char *message)\n+{\n+\tif (cb->msg)\n+\t\t*cb->msg = xstrdup(message);\n+\tif (cb->cutoff_time)\n+\t\t*cb->cutoff_time = timestamp;\n+\tif (cb->cutoff_tz)\n+\t\t*cb->cutoff_tz = tz;\n+\tif (cb->cutoff_cnt)\n+\t\t*cb->cutoff_cnt = cb->reccnt;\n+}\n+\n static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *email, timestamp_t timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n \n-\tcb->reccnt++;\n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n \tif (timestamp <= cb->at_time || cb->cnt == 0) {\n-\t\tif (cb->msg)\n-\t\t\t*cb->msg = xstrdup(message);\n-\t\tif (cb->cutoff_time)\n-\t\t\t*cb->cutoff_time = timestamp;\n-\t\tif (cb->cutoff_tz)\n-\t\t\t*cb->cutoff_tz = tz;\n-\t\tif (cb->cutoff_cnt)\n-\t\t\t*cb->cutoff_cnt = cb->reccnt - 1;\n+\t\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \t\t/*\n \t\t * we have not yet updated cb->[n|o]oid so they still\n \t\t * hold the values for the previous record.\n@@ -917,11 +922,13 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n \t\t\t\t\t\t DATE_MODE(RFC2822)));\n+\t\tcb->reccnt++;\n \t\toidcpy(&cb->ooid, ooid);\n \t\toidcpy(&cb->noid, noid);\n \t\tcb->found_it = 1;\n \t\treturn 1;\n \t}\n+\tcb->reccnt++;\n \toidcpy(&cb->ooid, ooid);\n \toidcpy(&cb->noid, noid);\n \tif (cb->cnt > 0)\n@@ -935,14 +942,7 @@ static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n \n-\tif (cb->msg)\n-\t\t*cb->msg = xstrdup(message);\n-\tif (cb->cutoff_time)\n-\t\t*cb->cutoff_time = timestamp;\n-\tif (cb->cutoff_tz)\n-\t\t*cb->cutoff_tz = tz;\n-\tif (cb->cutoff_cnt)\n-\t\t*cb->cutoff_cnt = cb->reccnt;\n+\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \toidcpy(cb->oid, ooid);\n \tif (is_null_oid(cb->oid))\n \t\toidcpy(cb->oid, noid);\n-- \n2.30.0\n\n"},{"id":"413549","messageId":"cover.1609923643.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"0c6885f15f5ce0be28142d9c69724362e72481a9.1609551262.git.liu.denton@gmail.com","subject":"[PATCH v2 0/2] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-06T09:01:52Z","receivedAt":"2021-01-06T09:03:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When there is only one reflog entry (perhaps caused by expiring the\nreflog and then making a single commit) @{1} errors out even though\nthere is technically enough information to do the lookup. Look at the\nold side of the reflog instead of the new side so that this does not\nfail. This is explained in more detail in the commit of the last patch.\n\nThis idea was given by Junio at [0].\n\n[0]: https://lore.kernel.org/git/xmqqzh8zgcfp.fsf@gitster.c.googlers.com/\n\nChanges since v1:\n\n* Factor out set_read_ref_cutoffs()\n\n* Check the output of rev-parse to ensure that the intended commit is\n returned\n\nDenton Liu (2):\n refs: factor out set_read_ref_cutoffs()\n refs: allow @{n} to work with n-sized reflog\n\n refs.c | 118 ++++++++++++++++++++----------------\n t/t1503-rev-parse-verify.sh | 4 +-\n t/t1508-at-combinations.sh | 16 +++++\n 3 files changed, 84 insertions(+), 54 deletions(-)\n\nRange-diff against v1:\n-: ---------- > 1: 8f14ec3997 refs: factor out set_read_ref_cutoffs()\n1: 0c6885f15f ! 2: 18a35506b8 refs: allow @{n} to work with n-sized reflog\n @@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noi\n \tstruct read_ref_at_cb *cb = cb_data;\n +\tint at_indexed_ent;\n \n - \tcb->reccnt++;\n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n @@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noi\n +\t\tcb->cnt--;\n +\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n +\tif (timestamp <= cb->at_time || at_indexed_ent) {\n - \t\tif (cb->msg)\n - \t\t\t*cb->msg = xstrdup(message);\n - \t\tif (cb->cutoff_time)\n -@@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n + \t\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n + \t\t/*\n \t\t * we have not yet updated cb->[n|o]oid so they still\n \t\t * hold the values for the previous record.\n \t\t */\n @@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noi\n \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n \t\t\t\t\t\t DATE_MODE(RFC2822)));\n +-\t\tcb->reccnt++;\n -\t\toidcpy(&cb->ooid, ooid);\n -\t\toidcpy(&cb->noid, noid);\n \t\tcb->found_it = 1;\n -\t\treturn 1;\n \t}\n + \tcb->reccnt++;\n \toidcpy(&cb->ooid, ooid);\n \toidcpy(&cb->noid, noid);\n -\tif (cb->cnt > 0)\n @@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noi\n +{\n +\tstruct read_ref_at_cb *cb = cb_data;\n +\n -+\tif (cb->msg)\n -+\t\t*cb->msg = xstrdup(message);\n -+\tif (cb->cutoff_time)\n -+\t\t*cb->cutoff_time = timestamp;\n -+\tif (cb->cutoff_tz)\n -+\t\t*cb->cutoff_tz = tz;\n -+\tif (cb->cutoff_cnt)\n -+\t\t*cb->cutoff_cnt = cb->reccnt;\n ++\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n +\toidcpy(cb->oid, noid);\n +\t/* We just want the first entry */\n +\treturn 1;\n @@ t/t1503-rev-parse-verify.sh: test_expect_success 'fails silently when using -q'\n \ttest_must_be_empty error\n '\n \n -@@ t/t1503-rev-parse-verify.sh: test_expect_success 'master@{n} for various n' '\n - \ttest_must_fail git rev-parse --verify master@{$Np1}\n - '\n +\n + ## t/t1508-at-combinations.sh ##\n +@@ t/t1508-at-combinations.sh: test_expect_success 'create path with @' '\n + check \"@:normal\" blob content\n + check \"@:fun@ny\" blob content\n \n +test_expect_success '@{1} works with only one reflog entry' '\n -+\tgit checkout -B newbranch &&\n ++\tgit checkout -B newbranch master &&\n +\tgit reflog expire --expire=now refs/heads/newbranch &&\n -+\tgit commit --allow-empty -mexpired &&\n -+\tgit rev-parse --verify newbranch@{1}\n ++\tgit commit --allow-empty -m \"first after expiration\" &&\n ++\tgit rev-parse newbranch~ >expect &&\n ++\tgit rev-parse newbranch@{1} >actual &&\n ++\ttest_cmp expect actual\n +'\n +\n +test_expect_success '@{0} works with empty reflog' '\n -+\tgit checkout -B newbranch &&\n ++\tgit checkout -B newbranch master &&\n +\tgit reflog expire --expire=now refs/heads/newbranch &&\n -+\tgit rev-parse --verify newbranch@{0}\n ++\tgit rev-parse newbranch >expect &&\n ++\tgit rev-parse newbranch@{0} >actual &&\n ++\ttest_cmp expect actual\n +'\n -+\n - test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '\n - \tln -s does-not-exist .git/refs/heads/broken &&\n - \ttest_must_fail git rev-parse --verify broken\n + test_done\n-- \n2.30.0\n\n"},{"id":"413550","messageId":"18a35506b87356c1ae844d2686b2be6bd04567b2.1609923643.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"cover.1609923643.git.liu.denton@gmail.com","subject":"[PATCH v2 2/2] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-06T09:01:54Z","receivedAt":"2021-01-06T09:03:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This sequence works\n\n\t$ git checkout -b newbranch\n\t$ git commit --allow-empty -m one\n\t$ git show -s newbranch@{1}\n\nand shows the state that was immediately after the newbranch was\ncreated.\n\nBut then if you do\n\n\t$ git reflog expire --expire=now refs/heads/newbranch\n\t$ git commit --allow=empty -m two\n\t$ git show -s newbranch@{1}\n\nyou'd be scolded with\n\n\tfatal: log for 'newbranch' only has 1 entries\n\nWhile it is true that it has only 1 entry, we have enough\ninformation in that single entry that records the transition between\nthe state in which the tip of the branch was pointing at commit\n'one' to the new commit 'two' built on it, so we should be able to\nanswer \"what object newbranch was pointing at?\". But we refuse to\ndo so.\n\nMake @{0} the special case where we use the new side to look up that\nentry. Otherwise, look up @{n} using the old side of the (n-1)th entry\nof the reflog.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Denton Liu \n---\n refs.c | 42 ++++++++++++++++++++++++-------------\n t/t1503-rev-parse-verify.sh | 4 ++--\n t/t1508-at-combinations.sh | 16 ++++++++++++++\n 3 files changed, 46 insertions(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex bfdd04aefd..9eb26d456d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -900,40 +900,49 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *message, void *cb_data)\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n+\tint at_indexed_ent;\n \n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n-\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n+\tif (cb->cnt > 0)\n+\t\tcb->cnt--;\n+\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n+\tif (timestamp <= cb->at_time || at_indexed_ent) {\n \t\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \t\t/*\n \t\t * we have not yet updated cb->[n|o]oid so they still\n \t\t * hold the values for the previous record.\n \t\t */\n-\t\tif (!is_null_oid(&cb->ooid)) {\n-\t\t\toidcpy(cb->oid, noid);\n-\t\t\tif (!oideq(&cb->ooid, noid))\n-\t\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n+\t\tif (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))\n+\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n \t\t\t\t\tcb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));\n-\t\t}\n-\t\telse if (cb->date == cb->at_time)\n+\t\tif (at_indexed_ent)\n+\t\t\toidcpy(cb->oid, ooid);\n+\t\telse if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)\n \t\t\toidcpy(cb->oid, noid);\n \t\telse if (!oideq(noid, cb->oid))\n \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n \t\t\t\t\t\t DATE_MODE(RFC2822)));\n-\t\tcb->reccnt++;\n-\t\toidcpy(&cb->ooid, ooid);\n-\t\toidcpy(&cb->noid, noid);\n \t\tcb->found_it = 1;\n-\t\treturn 1;\n \t}\n \tcb->reccnt++;\n \toidcpy(&cb->ooid, ooid);\n \toidcpy(&cb->noid, noid);\n-\tif (cb->cnt > 0)\n-\t\tcb->cnt--;\n-\treturn 0;\n+\treturn cb->found_it;\n+}\n+\n+static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid,\n+\t\t\t\t const char *email, timestamp_t timestamp,\n+\t\t\t\t int tz, const char *message, void *cb_data)\n+{\n+\tstruct read_ref_at_cb *cb = cb_data;\n+\n+\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n+\toidcpy(cb->oid, noid);\n+\t/* We just want the first entry */\n+\treturn 1;\n }\n \n static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,\n@@ -967,6 +976,11 @@ int read_ref_at(struct ref_store *refs, const char *refname,\n \tcb.cutoff_cnt = cutoff_cnt;\n \tcb.oid = oid;\n \n+\tif (cb.cnt == 0) {\n+\t\trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);\n+\t\treturn 0;\n+\t}\n+\n \trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);\n \n \tif (!cb.reccnt) {\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex dc9fe3cbf1..a7e9b4863d 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -86,8 +86,8 @@ test_expect_success 'fails silently when using -q' '\n test_expect_success 'fails silently when using -q with deleted reflogs' '\n \tref=$(git rev-parse HEAD) &&\n \tgit update-ref --create-reflog -m \"message for refs/test\" refs/test \"$ref\" &&\n-\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n-\ttest_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&\n+\tgit reflog delete --updateref --rewrite refs/test@{1} &&\n+\ttest_must_fail git rev-parse -q --verify refs/test@{1} >error 2>&1 &&\n \ttest_must_be_empty error\n '\n \ndiff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\nindex 4a9964e9dc..15aac6e77a 100755\n--- a/t/t1508-at-combinations.sh\n+++ b/t/t1508-at-combinations.sh\n@@ -99,4 +99,20 @@ test_expect_success 'create path with @' '\n check \"@:normal\" blob content\n check \"@:fun@ny\" blob content\n \n+test_expect_success '@{1} works with only one reflog entry' '\n+\tgit checkout -B newbranch master &&\n+\tgit reflog expire --expire=now refs/heads/newbranch &&\n+\tgit commit --allow-empty -m \"first after expiration\" &&\n+\tgit rev-parse newbranch~ >expect &&\n+\tgit rev-parse newbranch@{1} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '@{0} works with empty reflog' '\n+\tgit checkout -B newbranch master &&\n+\tgit reflog expire --expire=now refs/heads/newbranch &&\n+\tgit rev-parse newbranch >expect &&\n+\tgit rev-parse newbranch@{0} >actual &&\n+\ttest_cmp expect actual\n+'\n test_done\n-- \n2.30.0\n\n"},{"id":"413554","messageId":"20210106095947.GP8396@szeder.dev","threadId":"54917","inReplyTo":"18a35506b87356c1ae844d2686b2be6bd04567b2.1609923643.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 2/2] refs: allow @{n} to work with n-sized reflog","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-06T09:59:47Z","receivedAt":"2021-01-06T10:00:48Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Jan 06, 2021 at 01:01:54AM -0800, Denton Liu wrote:\n> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\n> index 4a9964e9dc..15aac6e77a 100755\n> --- a/t/t1508-at-combinations.sh\n> +++ b/t/t1508-at-combinations.sh\n> @@ -99,4 +99,20 @@ test_expect_success 'create path with @' '\n> check \"@:normal\" blob content\n> check \"@:fun@ny\" blob content\n> \n> +test_expect_success '@{1} works with only one reflog entry' '\n> +\tgit checkout -B newbranch master &&\n> +\tgit reflog expire --expire=now refs/heads/newbranch &&\n> +\tgit commit --allow-empty -m \"first after expiration\" &&\n> +\tgit rev-parse newbranch~ >expect &&\n> +\tgit rev-parse newbranch@{1} >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '@{0} works with empty reflog' '\n> +\tgit checkout -B newbranch master &&\n> +\tgit reflog expire --expire=now refs/heads/newbranch &&\n> +\tgit rev-parse newbranch >expect &&\n> +\tgit rev-parse newbranch@{0} >actual &&\n> +\ttest_cmp expect actual\n\nYou could use 'test_cmp_rev' in these two tests to spare a few lines\nand to get a bit friendlier error message on failure.\n\n"},{"id":"413605","messageId":"xmqqft3drd82.fsf@gitster.c.googlers.com","threadId":"54917","inReplyTo":"X/V0DU+CD6mS36dK@generichostname","subject":"Re: [PATCH] refs: allow @{n} to work with n-sized reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-06T21:02:05Z","receivedAt":"2021-01-06T21:02:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n>> > -\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n>> > +\tif (cb->cnt > 0)\n>> > +\t\tcb->cnt--;\n>> > +\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n>> \n>> The code treats two cases identically (i.e. the case where cb->cnt\n>> was originally zero, and one). Is that intended?\n>\n> It shouldn't be possible for cb->cnt == 0 on the first iteration\n> because there's a special-case check at [0]. As a result, it can only be\n> -1 or >= 1 on the first iteration.\n>\n> The -1 case happens when we're doing date-based lookup and that's what\n> this if is intended to handle.\n\nI knew about -1; it wasn't apparent that the caller won't call us\nwith cnt==0. Perhaps it deserves a mention in an in-code comment.\n\n> \"at_indexed_ent\" is meant to signal when we are indexing the reflog\n> numerically (as opposed to by date), we have arrived at the correct\n> entry. If you have a more fitting name, I'm open to suggestions.\n\nWhen querying for @{24}, all the entries are indexed\nnumerically (counted), not just the 24th one, and that contributed\nto my puzzlement.\n\nI offhand do not think of a \"name\", but \"at target\", \"found\",\n\"reached count\", are phrases that come to my mind as starting\npoints.\n\nThanks.\n"},{"id":"413676","messageId":"cover.1610015769.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"cover.1609923643.git.liu.denton@gmail.com","subject":"[PATCH v3 0/2] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-07T10:36:57Z","receivedAt":"2021-01-07T10:37:54Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When there is only one reflog entry (perhaps caused by expiring the\nreflog and then making a single commit) @{1} errors out even though\nthere is technically enough information to do the lookup. Look at the\nold side of the reflog instead of the new side so that this does not\nfail. This is explained in more detail in the commit of the last patch.\n\nThis idea was given by Junio at [0].\n\n[0]: https://lore.kernel.org/git/xmqqzh8zgcfp.fsf@gitster.c.googlers.com/\n\nChanges since v1:\n\n* Factor out set_read_ref_cutoffs()\n\n* Check the output of rev-parse to ensure that the intended commit is\n returned\n\nChanges since v2:\n\n* Rename at_indexed_ent -> reached_count\n\n* Add an in-code comment to document that cb->cnt can't be 0 in the first\n iteration of read_ref_at_ent()\n\n* Make test cases use test_cmp_rev() for brevity and better errors\n\nDenton Liu (2):\n refs: factor out set_read_ref_cutoffs()\n refs: allow @{n} to work with n-sized reflog\n\n refs.c | 122 +++++++++++++++++++++---------------\n t/t1503-rev-parse-verify.sh | 4 +-\n t/t1508-at-combinations.sh | 12 ++++\n 3 files changed, 84 insertions(+), 54 deletions(-)\n\nRange-diff against v2:\n1: 8f14ec3997 = 1: 8f14ec3997 refs: factor out set_read_ref_cutoffs()\n2: 18a35506b8 ! 2: c88c997eab refs: allow @{n} to work with n-sized reflog\n @@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noi\n \t\tconst char *message, void *cb_data)\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n -+\tint at_indexed_ent;\n ++\tint reached_count;\n \n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n -\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n ++\t/*\n ++\t * It is not possible for cb->cnt == 0 on the first itertion because\n ++\t * that special case is handled in read_ref_at().\n ++\t */\n +\tif (cb->cnt > 0)\n +\t\tcb->cnt--;\n -+\tat_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid);\n -+\tif (timestamp <= cb->at_time || at_indexed_ent) {\n ++\treached_count = cb->cnt == 0 && !is_null_oid(ooid);\n ++\tif (timestamp <= cb->at_time || reached_count) {\n \t\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \t\t/*\n \t\t * we have not yet updated cb->[n|o]oid so they still\n @@ refs.c: static int read_ref_at_ent(struct object_id *ooid, struct object_id *noi\n \t\t\t\t\tcb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));\n -\t\t}\n -\t\telse if (cb->date == cb->at_time)\n -+\t\tif (at_indexed_ent)\n ++\t\tif (reached_count)\n +\t\t\toidcpy(cb->oid, ooid);\n +\t\telse if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)\n \t\t\toidcpy(cb->oid, noid);\n @@ t/t1508-at-combinations.sh: test_expect_success 'create path with @' '\n +\tgit checkout -B newbranch master &&\n +\tgit reflog expire --expire=now refs/heads/newbranch &&\n +\tgit commit --allow-empty -m \"first after expiration\" &&\n -+\tgit rev-parse newbranch~ >expect &&\n -+\tgit rev-parse newbranch@{1} >actual &&\n -+\ttest_cmp expect actual\n ++\ttest_cmp_rev newbranch~ newbranch@{1}\n +'\n +\n +test_expect_success '@{0} works with empty reflog' '\n +\tgit checkout -B newbranch master &&\n +\tgit reflog expire --expire=now refs/heads/newbranch &&\n -+\tgit rev-parse newbranch >expect &&\n -+\tgit rev-parse newbranch@{0} >actual &&\n -+\ttest_cmp expect actual\n ++\ttest_cmp_rev newbranch newbranch@{0}\n +'\n test_done\n-- \n2.30.0\n\n"},{"id":"413677","messageId":"8f14ec39970b6cbf9b6615485316063306706e6a.1610015769.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"cover.1610015769.git.liu.denton@gmail.com","subject":"[PATCH v3 1/2] refs: factor out set_read_ref_cutoffs()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-07T10:36:58Z","receivedAt":"2021-01-07T10:38:31Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This block of code is duplicated twice. In a future commit, it will be\nduplicated for a third time. Factor out the common functionality into\nset_read_ref_cutoffs().\n\nIn the case of read_ref_at_ent(), we are incrementing `cb->reccnt` at the\nbeginning of the function. Move these to right before the return so that\nthe `cb->reccnt - 1` is changed to `cb->reccnt` and it can be cleanly\nfactored out into set_read_ref_cutoffs(). The duplication of the\nincrement statements will be removed in a future patch.\n\nSigned-off-by: Denton Liu \n---\n refs.c | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 13dc2c3291..bfdd04aefd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -882,25 +882,30 @@ struct read_ref_at_cb {\n \tint *cutoff_cnt;\n };\n \n+static void set_read_ref_cutoffs(struct read_ref_at_cb *cb,\n+\t\ttimestamp_t timestamp, int tz, const char *message)\n+{\n+\tif (cb->msg)\n+\t\t*cb->msg = xstrdup(message);\n+\tif (cb->cutoff_time)\n+\t\t*cb->cutoff_time = timestamp;\n+\tif (cb->cutoff_tz)\n+\t\t*cb->cutoff_tz = tz;\n+\tif (cb->cutoff_cnt)\n+\t\t*cb->cutoff_cnt = cb->reccnt;\n+}\n+\n static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *email, timestamp_t timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n \n-\tcb->reccnt++;\n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n \tif (timestamp <= cb->at_time || cb->cnt == 0) {\n-\t\tif (cb->msg)\n-\t\t\t*cb->msg = xstrdup(message);\n-\t\tif (cb->cutoff_time)\n-\t\t\t*cb->cutoff_time = timestamp;\n-\t\tif (cb->cutoff_tz)\n-\t\t\t*cb->cutoff_tz = tz;\n-\t\tif (cb->cutoff_cnt)\n-\t\t\t*cb->cutoff_cnt = cb->reccnt - 1;\n+\t\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \t\t/*\n \t\t * we have not yet updated cb->[n|o]oid so they still\n \t\t * hold the values for the previous record.\n@@ -917,11 +922,13 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n \t\t\t\t\t\t DATE_MODE(RFC2822)));\n+\t\tcb->reccnt++;\n \t\toidcpy(&cb->ooid, ooid);\n \t\toidcpy(&cb->noid, noid);\n \t\tcb->found_it = 1;\n \t\treturn 1;\n \t}\n+\tcb->reccnt++;\n \toidcpy(&cb->ooid, ooid);\n \toidcpy(&cb->noid, noid);\n \tif (cb->cnt > 0)\n@@ -935,14 +942,7 @@ static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n \n-\tif (cb->msg)\n-\t\t*cb->msg = xstrdup(message);\n-\tif (cb->cutoff_time)\n-\t\t*cb->cutoff_time = timestamp;\n-\tif (cb->cutoff_tz)\n-\t\t*cb->cutoff_tz = tz;\n-\tif (cb->cutoff_cnt)\n-\t\t*cb->cutoff_cnt = cb->reccnt;\n+\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \toidcpy(cb->oid, ooid);\n \tif (is_null_oid(cb->oid))\n \t\toidcpy(cb->oid, noid);\n-- \n2.30.0\n\n"},{"id":"413678","messageId":"c88c997eab2b3405cce20f719bf07d5bba7aa18e.1610015769.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"cover.1610015769.git.liu.denton@gmail.com","subject":"[PATCH v3 2/2] refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-07T10:36:59Z","receivedAt":"2021-01-07T10:38:31Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This sequence works\n\n\t$ git checkout -b newbranch\n\t$ git commit --allow-empty -m one\n\t$ git show -s newbranch@{1}\n\nand shows the state that was immediately after the newbranch was\ncreated.\n\nBut then if you do\n\n\t$ git reflog expire --expire=now refs/heads/newbranch\n\t$ git commit --allow=empty -m two\n\t$ git show -s newbranch@{1}\n\nyou'd be scolded with\n\n\tfatal: log for 'newbranch' only has 1 entries\n\nWhile it is true that it has only 1 entry, we have enough\ninformation in that single entry that records the transition between\nthe state in which the tip of the branch was pointing at commit\n'one' to the new commit 'two' built on it, so we should be able to\nanswer \"what object newbranch was pointing at?\". But we refuse to\ndo so.\n\nMake @{0} the special case where we use the new side to look up that\nentry. Otherwise, look up @{n} using the old side of the (n-1)th entry\nof the reflog.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Denton Liu \n---\n refs.c | 46 ++++++++++++++++++++++++++-----------\n t/t1503-rev-parse-verify.sh | 4 ++--\n t/t1508-at-combinations.sh | 12 ++++++++++\n 3 files changed, 46 insertions(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex bfdd04aefd..e70dcd33f7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -900,40 +900,53 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *message, void *cb_data)\n {\n \tstruct read_ref_at_cb *cb = cb_data;\n+\tint reached_count;\n \n \tcb->tz = tz;\n \tcb->date = timestamp;\n \n-\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n+\t/*\n+\t * It is not possible for cb->cnt == 0 on the first itertion because\n+\t * that special case is handled in read_ref_at().\n+\t */\n+\tif (cb->cnt > 0)\n+\t\tcb->cnt--;\n+\treached_count = cb->cnt == 0 && !is_null_oid(ooid);\n+\tif (timestamp <= cb->at_time || reached_count) {\n \t\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n \t\t/*\n \t\t * we have not yet updated cb->[n|o]oid so they still\n \t\t * hold the values for the previous record.\n \t\t */\n-\t\tif (!is_null_oid(&cb->ooid)) {\n-\t\t\toidcpy(cb->oid, noid);\n-\t\t\tif (!oideq(&cb->ooid, noid))\n-\t\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n+\t\tif (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))\n+\t\t\twarning(_(\"log for ref %s has gap after %s\"),\n \t\t\t\t\tcb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));\n-\t\t}\n-\t\telse if (cb->date == cb->at_time)\n+\t\tif (reached_count)\n+\t\t\toidcpy(cb->oid, ooid);\n+\t\telse if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)\n \t\t\toidcpy(cb->oid, noid);\n \t\telse if (!oideq(noid, cb->oid))\n \t\t\twarning(_(\"log for ref %s unexpectedly ended on %s\"),\n \t\t\t\tcb->refname, show_date(cb->date, cb->tz,\n \t\t\t\t\t\t DATE_MODE(RFC2822)));\n-\t\tcb->reccnt++;\n-\t\toidcpy(&cb->ooid, ooid);\n-\t\toidcpy(&cb->noid, noid);\n \t\tcb->found_it = 1;\n-\t\treturn 1;\n \t}\n \tcb->reccnt++;\n \toidcpy(&cb->ooid, ooid);\n \toidcpy(&cb->noid, noid);\n-\tif (cb->cnt > 0)\n-\t\tcb->cnt--;\n-\treturn 0;\n+\treturn cb->found_it;\n+}\n+\n+static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid,\n+\t\t\t\t const char *email, timestamp_t timestamp,\n+\t\t\t\t int tz, const char *message, void *cb_data)\n+{\n+\tstruct read_ref_at_cb *cb = cb_data;\n+\n+\tset_read_ref_cutoffs(cb, timestamp, tz, message);\n+\toidcpy(cb->oid, noid);\n+\t/* We just want the first entry */\n+\treturn 1;\n }\n \n static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,\n@@ -967,6 +980,11 @@ int read_ref_at(struct ref_store *refs, const char *refname,\n \tcb.cutoff_cnt = cutoff_cnt;\n \tcb.oid = oid;\n \n+\tif (cb.cnt == 0) {\n+\t\trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);\n+\t\treturn 0;\n+\t}\n+\n \trefs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);\n \n \tif (!cb.reccnt) {\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex dc9fe3cbf1..a7e9b4863d 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -86,8 +86,8 @@ test_expect_success 'fails silently when using -q' '\n test_expect_success 'fails silently when using -q with deleted reflogs' '\n \tref=$(git rev-parse HEAD) &&\n \tgit update-ref --create-reflog -m \"message for refs/test\" refs/test \"$ref\" &&\n-\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n-\ttest_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&\n+\tgit reflog delete --updateref --rewrite refs/test@{1} &&\n+\ttest_must_fail git rev-parse -q --verify refs/test@{1} >error 2>&1 &&\n \ttest_must_be_empty error\n '\n \ndiff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\nindex 4a9964e9dc..528a77287c 100755\n--- a/t/t1508-at-combinations.sh\n+++ b/t/t1508-at-combinations.sh\n@@ -99,4 +99,16 @@ test_expect_success 'create path with @' '\n check \"@:normal\" blob content\n check \"@:fun@ny\" blob content\n \n+test_expect_success '@{1} works with only one reflog entry' '\n+\tgit checkout -B newbranch master &&\n+\tgit reflog expire --expire=now refs/heads/newbranch &&\n+\tgit commit --allow-empty -m \"first after expiration\" &&\n+\ttest_cmp_rev newbranch~ newbranch@{1}\n+'\n+\n+test_expect_success '@{0} works with empty reflog' '\n+\tgit checkout -B newbranch master &&\n+\tgit reflog expire --expire=now refs/heads/newbranch &&\n+\ttest_cmp_rev newbranch newbranch@{0}\n+'\n test_done\n-- \n2.30.0\n\n"},{"id":"413679","messageId":"8ce0071bbf8373ed00cccc797f83c4d30d72ae6e.1610016122.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"cover.1609923643.git.liu.denton@gmail.com","subject":"[PATCH v3 3/2] fixup! refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-07T10:43:15Z","receivedAt":"2021-01-07T10:44:14Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n t/t1508-at-combinations.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\nindex 528a77287c..e4521b7b97 100755\n--- a/t/t1508-at-combinations.sh\n+++ b/t/t1508-at-combinations.sh\n@@ -111,4 +111,5 @@ test_expect_success '@{0} works with empty reflog' '\n \tgit reflog expire --expire=now refs/heads/newbranch &&\n \ttest_cmp_rev newbranch newbranch@{0}\n '\n+\n test_done\n-- \n2.30.0\n\n"},{"id":"413973","messageId":"20210110144423.GU8396@szeder.dev","threadId":"54917","inReplyTo":"cover.1610015769.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 0/2] refs: allow @{n} to work with n-sized reflog","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-10T14:44:23Z","receivedAt":"2021-01-10T14:45:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Junio,\n\nOn Thu, Jan 07, 2021 at 02:36:57AM -0800, Denton Liu wrote:\n> When there is only one reflog entry (perhaps caused by expiring the\n> reflog and then making a single commit) @{1} errors out even though\n> there is technically enough information to do the lookup. Look at the\n> old side of the reflog instead of the new side so that this does not\n> fail. This is explained in more detail in the commit of the last patch.\n\n> Denton Liu (2):\n> refs: factor out set_read_ref_cutoffs()\n> refs: allow @{n} to work with n-sized reflog\n\nTopic 'dl/reflog-with-single-entry', i.e. these two patches queued\ndirectly on top of v2.29.2, break the test case \"61 - valid ref of the\nform \"n\", n < N\" in 't3903-stash.sh'. Queueing them on top of\nsomething already containing commit 4f44c5659b (stash: simplify reflog\nemptiness check, 2020-10-24) fixes this issue.\n\n"},{"id":"413986","messageId":"xmqq4kjoa6bu.fsf@gitster.c.googlers.com","threadId":"54917","inReplyTo":"20210110144423.GU8396@szeder.dev","subject":"Re: [PATCH v3 0/2] refs: allow @{n} to work with n-sized reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-10T20:24:21Z","receivedAt":"2021-01-10T20:25:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> Junio,\n>\n> On Thu, Jan 07, 2021 at 02:36:57AM -0800, Denton Liu wrote:\n>> When there is only one reflog entry (perhaps caused by expiring the\n>> reflog and then making a single commit) @{1} errors out even though\n>> there is technically enough information to do the lookup. Look at the\n>> old side of the reflog instead of the new side so that this does not\n>> fail. This is explained in more detail in the commit of the last patch.\n>\n>> Denton Liu (2):\n>> refs: factor out set_read_ref_cutoffs()\n>> refs: allow @{n} to work with n-sized reflog\n>\n> Topic 'dl/reflog-with-single-entry', i.e. these two patches queued\n> directly on top of v2.29.2, break the test case \"61 - valid ref of the\n> form \"n\", n < N\" in 't3903-stash.sh'. Queueing them on top of\n> something already containing commit 4f44c5659b (stash: simplify reflog\n> emptiness check, 2020-10-24) fixes this issue.\n\nThanks for carefully watching ;-)\n\nThere is no reason why this fix needs to be backported down to 2.29\ntrack, I would think.\n"},{"id":"413988","messageId":"X/tkFuESnia/n35e@ruderich.org","threadId":"54917","inReplyTo":"c88c997eab2b3405cce20f719bf07d5bba7aa18e.1610015769.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 2/2] refs: allow @{n} to work with n-sized reflog","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2021-01-10T20:31:18Z","receivedAt":"2021-01-10T20:37:07Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Thu, Jan 07, 2021 at 02:36:59AM -0800, Denton Liu wrote:\n> diff --git a/refs.c b/refs.c\n> index bfdd04aefd..e70dcd33f7 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -900,40 +900,53 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> \t\tconst char *message, void *cb_data)\n> {\n> \tstruct read_ref_at_cb *cb = cb_data;\n> +\tint reached_count;\n>\n> \tcb->tz = tz;\n> \tcb->date = timestamp;\n>\n> -\tif (timestamp <= cb->at_time || cb->cnt == 0) {\n> +\t/*\n> +\t * It is not possible for cb->cnt == 0 on the first itertion because\n\ns/itertion/iteration/\n\nRegards\nSimon\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"414095","messageId":"d51c9ccca7c58524d5c735c596c142583029a36c.1610432037.git.liu.denton@gmail.com","threadId":"54917","inReplyTo":"X/tkFuESnia/n35e@ruderich.org","subject":"[PATCH v3] fixup! refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-12T06:14:14Z","receivedAt":"2021-01-12T06:15:14Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n refs.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex e70dcd33f7..03968ad787 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -906,7 +906,7 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n \tcb->date = timestamp;\n \n \t/*\n-\t * It is not possible for cb->cnt == 0 on the first itertion because\n+\t * It is not possible for cb->cnt == 0 on the first iteration because\n \t * that special case is handled in read_ref_at().\n \t */\n \tif (cb->cnt > 0)\n-- \n2.30.0.284.gd98b1dd5ea\n\n"},{"id":"414096","messageId":"X/0/Jo3NYlKG3hxE@generichostname","threadId":"54917","inReplyTo":"d51c9ccca7c58524d5c735c596c142583029a36c.1610432037.git.liu.denton@gmail.com","subject":"Re: [PATCH v3] fixup! refs: allow @{n} to work with n-sized reflog","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-01-12T06:18:14Z","receivedAt":"2021-01-12T06:18:59Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Please disregard, I see that the fixup has already been included.\n\n-Denton\n\nOn Mon, Jan 11, 2021 at 10:14:14PM -0800, Denton Liu wrote:\n> Signed-off-by: Denton Liu \n> ---\n> refs.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/refs.c b/refs.c\n> index e70dcd33f7..03968ad787 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -906,7 +906,7 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,\n> \tcb->date = timestamp;\n> \n> \t/*\n> -\t * It is not possible for cb->cnt == 0 on the first itertion because\n> +\t * It is not possible for cb->cnt == 0 on the first iteration because\n> \t * that special case is handled in read_ref_at().\n> \t */\n> \tif (cb->cnt > 0)\n> -- \n> 2.30.0.284.gd98b1dd5ea\n> \n"},{"id":"414097","messageId":"xmqq35z6655c.fsf@gitster.c.googlers.com","threadId":"54917","inReplyTo":"X/0/Jo3NYlKG3hxE@generichostname","subject":"Re: [PATCH v3] fixup! refs: allow @{n} to work with n-sized reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T06:27:59Z","receivedAt":"2021-01-12T06:28:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Please disregard, I see that the fixup has already been included.\n\nBeing careful never hurts; it may make duplicated work from time to\ntime, but that is better than fixes falling thru the cracks.\n\nThanks.\n"}]}