Re: [GSoC PATCH v2] backfill: error out when HEAD cannot be parsed

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 2, 2026, 20:09 UTC
Message-ID: <xmqq8qb5p1jq.fsf@gitster.g>
In-Reply-To: <xmqqcy0hp25j.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 26 quoted lines

> Trieu Huynh <vikingtc4@gmail.com> writes:
>
>> From: Trieu Huynh <vikingtc4@gmail.com>
>>
>> handle_revision_arg() returns non-zero on failure, but do_backfill()
>> ignores the return value. On an empty repo with no commits, HEAD is
>> unborn and handle_revision_arg() fails, but backfill silently
>> continues with an empty revision walk and exists with a zero return
>> code.
>
> "exists" -> "exits", I think.
>
> But more importantly (with Devil's advocate hat on), what's the
> downside of the current behaviour?
>
> You tell the command to backfill, the machinery does not find
> anything necessary to fetch to backfill, and successfully, quickly,
> and quietly exits.  That sounds like a graceful exit to me.
>
> Is there anything wrong with that?
>
>> +test_expect_success 'backfill on empty repo fails gracefully' '
>> +	git init empty-repo &&
>> +	test_must_fail git -C empty-repo backfill 2>err &&
>> +	test_grep "unable to parse HEAD" err
>> +'

By the way, a more relevant thing to mention is that this change will probably become totally unnecessary in the presence of the ds/backfill-revs topic that is already in 'next'.

It does the usual "if you do not get revision range, fall back to HEAD", so

    git backfill<RET>

in an empty repository gracefully does nothing, while giving revision ranges explicitly, like,

    git backfill master..next
    git backfill HEAD

in such a repository will be greeted with a more explicit "bad revision" error.

Another lesson to pay closer attention to what others are doing in the same project. This would have been easily discoverable if you attempted trial merges to 'next' and to 'seen' after you tested your change standalone (well, that is how I recalled the other topic anyway).

Thanks.