{"thread":{"id":"57532","subject":"Minor regression in 2.35 with reflog expire","startedAt":"2022-03-10T19:08:37Z","lastAt":"2022-03-11T07:38:01Z","messageCount":3,"participants":["Johannes Sixt","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"451067","messageId":"571c0796-66d4-e8c7-c5a5-2e7a28132aa9@kdbg.org","threadId":"57532","inReplyTo":null,"subject":"Minor regression in 2.35 with reflog expire","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2022-03-10T19:08:32Z","receivedAt":"2022-03-10T19:08:37Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Since daf1d8285eeb (\"reflog expire: don't use\nlookup_commit_reference_gently()\", 2021-12-22) we see\n\n$ git reflog expire --all\nerror: Could not read 0000000000000000000000000000000000000000\n\nthat was not there before.\n\nThe problem seem to be empty reflog files. I assume they can be created\nwhen reflogs expire completely. I have a handful of them in repository\nthat was quiet for a long while.\n\nTo reproduce:\n\ngit init\nmkdir -p .git/logs/refs/heads\ntouch .git/logs/refs/heads/foo\ngit reflog expire --all\n\nThe \"scary\" aspect of the regression is that it happens as part of git\ngc and alarms users, see for example\nhttps://stackoverflow.com/questions/71364717/git-gc-error-could-not-read-0000000000000000000000000000000000000000\n\n-- Hannes\n"},{"id":"451098","messageId":"de5e2b0e290791d0a4f58a893d8571b5fc8c4f1a.1646952843.git.avarab@gmail.com","threadId":"57532","inReplyTo":"571c0796-66d4-e8c7-c5a5-2e7a28132aa9@kdbg.org","subject":"[PATCH] reflog: don't be noisy on empty reflogs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-10T22:56:11Z","receivedAt":"2022-03-10T22:56:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in my daf1d8285ee (reflog expire: don't use\nlookup_commit_reference_gently(), 2021-12-22), in changing from\nlookup_commit_reference_gently() to lookup_commit() we stopped trying\nto call deref_tag() and parse_object() on the provided OID, but we\nalso started returning non-NULL for the null_oid().\n\nAs a result we'd emit an error() via mark_reachable() later in this\nfunction as we tried to invoke parse_commit() on it.\n\nReported-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nOn Thu, Mar 10 2022, Johannes Sixt wrote:\n\n> Since daf1d8285eeb (\"reflog expire: don't use\n> lookup_commit_reference_gently()\", 2021-12-22) we see\n>\n> $ git reflog expire --all\n> error: Could not read 0000000000000000000000000000000000000000\n>\n> that was not there before.\n>\n> The problem seem to be empty reflog files. I assume they can be created\n> when reflogs expire completely. I have a handful of them in repository\n> that was quiet for a long while.\n>\n> To reproduce:\n>\n> git init\n> mkdir -p .git/logs/refs/heads\n> touch .git/logs/refs/heads/foo\n> git reflog expire --all\n\nThanks, and sorry about that. I believe this should fix it.\n\n builtin/reflog.c | 2 ++\n t/t1410-reflog.sh | 9 +++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 016466852f1..3fdf926759c 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -374,6 +374,8 @@ static void reflog_expiry_prepare(const char *refname,\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n \t\tcommit = lookup_commit(the_repository, oid);\n+\t\tif (commit && is_null_oid(&commit->object.oid))\n+\t\t\tcommit = NULL;\n \t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \ndiff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\nindex 68f69bb5431..ea8e6ac2a02 100755\n--- a/t/t1410-reflog.sh\n+++ b/t/t1410-reflog.sh\n@@ -423,4 +423,13 @@ test_expect_success 'expire with multiple worktrees' '\n \t)\n '\n \n+test_expect_success REFFILES 'empty reflog' '\n+\ttest_when_finished \"rm -rf empty\" &&\n+\tgit init empty &&\n+\ttest_commit -C empty A &&\n+\t>empty/.git/logs/refs/heads/foo &&\n+\tgit -C empty reflog expire --all 2>err &&\n+\ttest_must_be_empty err\n+'\n+\n test_done\n-- \n2.35.GIT-dev\n\n"},{"id":"451113","messageId":"83d1c3cc-e281-4455-60d4-ead53d578bb8@kdbg.org","threadId":"57532","inReplyTo":"de5e2b0e290791d0a4f58a893d8571b5fc8c4f1a.1646952843.git.avarab@gmail.com","subject":"Re: [PATCH] reflog: don't be noisy on empty reflogs","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2022-03-11T07:37:49Z","receivedAt":"2022-03-11T07:38:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.03.22 um 23:56 schrieb Ævar Arnfjörð Bjarmason:\n> Fix a regression in my daf1d8285ee (reflog expire: don't use\n> lookup_commit_reference_gently(), 2021-12-22), in changing from\n> lookup_commit_reference_gently() to lookup_commit() we stopped trying\n> to call deref_tag() and parse_object() on the provided OID, but we\n> also started returning non-NULL for the null_oid().\n> \n> As a result we'd emit an error() via mark_reachable() later in this\n> function as we tried to invoke parse_commit() on it.\n> \n> Reported-by: Johannes Sixt \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> \n> On Thu, Mar 10 2022, Johannes Sixt wrote:\n> \n>> Since daf1d8285eeb (\"reflog expire: don't use\n>> lookup_commit_reference_gently()\", 2021-12-22) we see\n>>\n>> $ git reflog expire --all\n>> error: Could not read 0000000000000000000000000000000000000000\n>>\n>> that was not there before.\n>>\n>> The problem seem to be empty reflog files. I assume they can be created\n>> when reflogs expire completely. I have a handful of them in repository\n>> that was quiet for a long while.\n>>\n>> To reproduce:\n>>\n>> git init\n>> mkdir -p .git/logs/refs/heads\n>> touch .git/logs/refs/heads/foo\n>> git reflog expire --all\n> \n> Thanks, and sorry about that. I believe this should fix it.\n> \n> builtin/reflog.c | 2 ++\n> t/t1410-reflog.sh | 9 +++++++++\n> 2 files changed, 11 insertions(+)\n> \n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index 016466852f1..3fdf926759c 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -374,6 +374,8 @@ static void reflog_expiry_prepare(const char *refname,\n> \t\tcb->unreachable_expire_kind = UE_HEAD;\n> \t} else {\n> \t\tcommit = lookup_commit(the_repository, oid);\n> +\t\tif (commit && is_null_oid(&commit->object.oid))\n> +\t\t\tcommit = NULL;\n> \t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n> \t}\n> \n> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\n> index 68f69bb5431..ea8e6ac2a02 100755\n> --- a/t/t1410-reflog.sh\n> +++ b/t/t1410-reflog.sh\n> @@ -423,4 +423,13 @@ test_expect_success 'expire with multiple worktrees' '\n> \t)\n> '\n> \n> +test_expect_success REFFILES 'empty reflog' '\n> +\ttest_when_finished \"rm -rf empty\" &&\n> +\tgit init empty &&\n> +\ttest_commit -C empty A &&\n> +\t>empty/.git/logs/refs/heads/foo &&\n> +\tgit -C empty reflog expire --all 2>err &&\n> +\ttest_must_be_empty err\n> +'\n> +\n> test_done\n\nThank you for the quick turnaround. This does indeed remove the error\nmessage.\n\nTested-by: Johannes Sixt \n\n-- Hannes\n"}]}