Subject: Re: [PATCH 3/5] backfill: accept revision arguments

From: Derrick Stolee <[email-masked]>
Date: Sun, 22 Mar 2026 20:35:09 -0400

On 3/19/26 5:54 AM, Patrick Steinhardt wrote:
> On Tue, Mar 17, 2026 at 12:29:19AM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <[email-masked]>
>>
>> The existing implementation of 'git backfill' only includes downloading
>> missing blobs reachable from HEAD. Advanced uses may desire more general
>> commit limiting options, such as '--all' for all references, specifying a
>> commit range via negative references, or specifying a recency of use such as
>> with '--since=<date>'.
>>
>> All of these options are available if we use setup_revisions() to parse the
>> unknown arguments with the revision machinery. This opens up a large number
>> of possibilities, only a small set of which are tested here.
>>
>> For documentation, we avoid duplicating the option documentation and instead
>> link to the documentation of 'git rev-list'.
>>
>> Note that these arguments currently allow specifying a pathspec, which
>> modifies the commit history checks but does not limit the paths used in the
>> backfill logic. This will be updated in a future change.
> 
> Makes me wonder whether reversing the order would have avoided this
> slight awkwardness. But let's just stick with the current order, the end
> result would be the same anyway.
True, we could have added the pathspec logic first, but we wouldn't be able
to test it right away because the parsing comes through the rev-list.

>> Signed-off-by: Derrick Stolee <[email-masked]>
>> ---
>>   Documentation/git-backfill.adoc |   3 +
>>   builtin/backfill.c              |  19 ++--
>>   t/t5620-backfill.sh             | 156 ++++++++++++++++++++++++++++++++
>>   3 files changed, 172 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/backfill.c b/builtin/backfill.c
>> index e80fc1b694..1b5595b27c 100644
>> --- a/builtin/backfill.c
>> +++ b/builtin/backfill.c
>> @@ -92,13 +92,14 @@ static int do_backfill(struct backfill_context *ctx)
>>   		}
>>   	}
>>   
>> -	repo_init_revisions(ctx->repo, &revs, "");
>> -	handle_revision_arg("HEAD", &revs, 0, 0);
>> +	/* Walk from HEAD if otherwise unspecified. */
>> +	if (!ctx->revs.pending.nr)
>> +		handle_revision_arg("HEAD", &ctx->revs, 0, 0);
> 
> Can we use `add_head_to_pending(&ctx->revs)` instead?

Nice. We absolutely can and should.

Thanks,
-Stolee