Re: [PATCH] reflog: don't be noisy on empty reflogs

[Johannes Sixt <j6t@kdbg.org>]

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 <j6t@kdbg.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 
> 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 <j6t@kdbg.org>

-- Hannes