Re: [PATCH 4/7] backfill: die on incompatible filter options

[Derrick Stolee <stolee@gmail.com>]

On 5/3/2026 6:59 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> The 'git backfill' command uses the path-walk API in a critical way: it
>> uses the objects output from the command to find the batches of missing
>> objects that should be requested from the server. Unlike 'git
>> pack-objects', we cannot fall back to another mechanism.
>>
>> The previous change added the path_walk_filter_compatible() method that
>> we can reuse here. Use it during argument validation in cmd_backfill().
>>
>> Signed-off-by: Derrick Stolee <stolee@gmail.com>
>> ---
>>  builtin/backfill.c  | 2 ++
>>  t/t5620-backfill.sh | 8 ++++++++
>>  2 files changed, 10 insertions(+)
> 
> Another topic adds a helper function to check for many incompatible
> options and calls it from here.  When I merged this topic, I made an
> semi-evil merge to move this call to that function (with necessary
> adjustment to the parameter).  Please sanity check the resolution I
> made in 'seen'.  Thanks.
Thanks for alerting me about this. I do think there is an error in
your merge. Hopefully the tests I wrote in this series caught the
mistake.

Here are the last lines in your copy of
reject_unsupported_rev_list_options():

	if (revs->filter.choice)
		die(_("'%s' cannot be used with 'git backfill'"),
		    "--filter");
	if (!path_walk_filter_compatible(&revs->filter))
		die(_("cannot backfill with these filter options"));
	if (revs->filter.blob_limit_value)
		die(_("cannot backfill with blob size limits"));

The last two options are correct, but they can't do anything
because the first one causes the command to fail immediately.

This should have caused failures in t5620-backfill.sh, specifically
the test 'backfill rejects incompatible filter options'. Indeed, I
get this error output when running on that commit:

+ test_grep cannot backfill with these filter options err
+ eval last_arg=${2}
+ last_arg=err
+ test -f err
+ test 2 -lt 2
+ test x! = xcannot backfill with these filter options
+ test x! = xcannot backfill with these filter options
+ grep cannot backfill with these filter options err
+ echo error: 'grep cannot backfill with these filter options err' didn't find a match in:
error: 'grep cannot backfill with these filter options err' didn't find a match in:
+ test -s err
+ cat err
fatal: '--filter' cannot be used with 'git backfill'
+ return 1

This does make it clear that I should add a new test in t5620 that
tests the 'sparse:<oid>' filter now that it is compatible, which I
missed in v1.

Thanks,
-Stolee