# Minor regression in 2.35 with reflog expire 3 messages from 2022-03-10 to 2022-03-11. Participants: Johannes Sixt, Ævar Arnfjörð Bjarmason. Thread: https://gitlist.dev/t/57532 ## Johannes Sixt, 2022-03-10 19:08 Subject: Minor regression in 2.35 with reflog expire Message-ID: <571c0796-66d4-e8c7-c5a5-2e7a28132aa9@kdbg.org> URL: https://gitlist.dev/e/571c0796-66d4-e8c7-c5a5-2e7a28132aa9%40kdbg.org ``` Since daf1d8285eeb ("reflog expire: don't use lookup_commit_reference_gently()", 2021-12-22) we see $ git reflog expire --all error: Could not read 0000000000000000000000000000000000000000 that was not there before. The problem seem to be empty reflog files. I assume they can be created when reflogs expire completely. I have a handful of them in repository that was quiet for a long while. To reproduce: git init mkdir -p .git/logs/refs/heads touch .git/logs/refs/heads/foo git reflog expire --all The "scary" aspect of the regression is that it happens as part of git gc and alarms users, see for example https://stackoverflow.com/questions/71364717/git-gc-error-could-not-read-0000000000000000000000000000000000000000 -- Hannes ``` ## Ævar Arnfjörð Bjarmason, 2022-03-10 22:56 Subject: [PATCH] reflog: don't be noisy on empty reflogs Message-ID: URL: https://gitlist.dev/e/de5e2b0e290791d0a4f58a893d8571b5fc8c4f1a.1646952843.git.avarab%40gmail.com In-Reply-To: <571c0796-66d4-e8c7-c5a5-2e7a28132aa9@kdbg.org> ``` Fix a regression in my daf1d8285ee (reflog expire: don't use lookup_commit_reference_gently(), 2021-12-22), in changing from lookup_commit_reference_gently() to lookup_commit() we stopped trying to call deref_tag() and parse_object() on the provided OID, but we also started returning non-NULL for the null_oid(). As a result we'd emit an error() via mark_reachable() later in this function as we tried to invoke parse_commit() on it. Reported-by: Johannes Sixt Signed-off-by: Ævar Arnfjörð Bjarmason --- On Thu, Mar 10 2022, Johannes Sixt wrote: > Since daf1d8285eeb ("reflog expire: don't use > lookup_commit_reference_gently()", 2021-12-22) we see > > $ git reflog expire --all > error: Could not read 0000000000000000000000000000000000000000 > > that was not there before. > > The problem seem to be empty reflog files. I assume they can be created > when reflogs expire completely. I have a handful of them in repository > that was quiet for a long while. > > To reproduce: > > git init > mkdir -p .git/logs/refs/heads > touch .git/logs/refs/heads/foo > git reflog expire --all Thanks, and sorry about that. I believe this should fix it. builtin/reflog.c | 2 ++ t/t1410-reflog.sh | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/reflog.c b/builtin/reflog.c index 016466852f1..3fdf926759c 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -374,6 +374,8 @@ static void reflog_expiry_prepare(const char *refname, cb->unreachable_expire_kind = UE_HEAD; } else { commit = lookup_commit(the_repository, oid); + if (commit && is_null_oid(&commit->object.oid)) + commit = NULL; cb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS; } diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index 68f69bb5431..ea8e6ac2a02 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -423,4 +423,13 @@ test_expect_success 'expire with multiple worktrees' ' ) ' +test_expect_success REFFILES 'empty reflog' ' + test_when_finished "rm -rf empty" && + git init empty && + test_commit -C empty A && + >empty/.git/logs/refs/heads/foo && + git -C empty reflog expire --all 2>err && + test_must_be_empty err +' + test_done -- 2.35.GIT-dev ``` ## Johannes Sixt, 2022-03-11 07:37 Subject: Re: [PATCH] reflog: don't be noisy on empty reflogs Message-ID: <83d1c3cc-e281-4455-60d4-ead53d578bb8@kdbg.org> URL: https://gitlist.dev/e/83d1c3cc-e281-4455-60d4-ead53d578bb8%40kdbg.org In-Reply-To: ``` Am 10.03.22 um 23:56 schrieb Ævar Arnfjörð Bjarmason: > Fix a regression in my daf1d8285ee (reflog expire: don't use > lookup_commit_reference_gently(), 2021-12-22), in changing from > lookup_commit_reference_gently() to lookup_commit() we stopped trying > to call deref_tag() and parse_object() on the provided OID, but we > also started returning non-NULL for the null_oid(). > > As a result we'd emit an error() via mark_reachable() later in this > function as we tried to invoke parse_commit() on it. > > Reported-by: Johannes Sixt > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > > On Thu, Mar 10 2022, Johannes Sixt wrote: > >> Since daf1d8285eeb ("reflog expire: don't use >> lookup_commit_reference_gently()", 2021-12-22) we see >> >> $ git reflog expire --all >> error: Could not read 0000000000000000000000000000000000000000 >> >> that was not there before. >> >> The problem seem to be empty reflog files. I assume they can be created >> when reflogs expire completely. I have a handful of them in repository >> that was quiet for a long while. >> >> To reproduce: >> >> git init >> mkdir -p .git/logs/refs/heads >> touch .git/logs/refs/heads/foo >> git reflog expire --all > > Thanks, and sorry about that. I believe this should fix it. > > builtin/reflog.c | 2 ++ > t/t1410-reflog.sh | 9 +++++++++ > 2 files changed, 11 insertions(+) > > diff --git a/builtin/reflog.c b/builtin/reflog.c > index 016466852f1..3fdf926759c 100644 > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -374,6 +374,8 @@ static void reflog_expiry_prepare(const char *refname, > cb->unreachable_expire_kind = UE_HEAD; > } else { > commit = lookup_commit(the_repository, oid); > + if (commit && is_null_oid(&commit->object.oid)) > + commit = NULL; > cb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS; > } > > diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh > index 68f69bb5431..ea8e6ac2a02 100755 > --- a/t/t1410-reflog.sh > +++ b/t/t1410-reflog.sh > @@ -423,4 +423,13 @@ test_expect_success 'expire with multiple worktrees' ' > ) > ' > > +test_expect_success REFFILES 'empty reflog' ' > + test_when_finished "rm -rf empty" && > + git init empty && > + test_commit -C empty A && > + >empty/.git/logs/refs/heads/foo && > + git -C empty reflog expire --all 2>err && > + test_must_be_empty err > +' > + > test_done Thank you for the quick turnaround. This does indeed remove the error message. Tested-by: Johannes Sixt -- Hannes ```