Re: [PATCH 1/7] pack-objects: pass --objects with --path-walk

[Derrick Stolee <stolee@gmail.com>]

On 5/3/2026 8:49 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> When 'git pack-objects' has the --path-walk option enabled, it uses a
>> different set of revision walk parameters than normal. For once,
> 
> "once" -> "one" (or "instance")?

Yes, "one". Sorry for the typo.

>> --objects was previously assumed by the path-walk API and was not needed
>> to be added. We also needed --boundary to allow discovering
>> UNINTERESTING objects to use as delta bases.
>>
>> We will be updating the path-walk API soon to work with some filter
>> options. However, the revision machinery will trigger a fatal error:
>>
>>   fatal: object filtering requires --objects
>>
>> The fix is easy: add the --objects option as an argument. This has no
>> effect on the path-walk API but does simplify the revision option
>> parsing for the objects filter.
>>
>> We can remove the comment about "removing" the options because they were
>> never removed and instead not added. We still need to disable using
>> bitmaps.
> 
> In the old code, there was a valid reason why bitmaps were not used
> (i.e., "--objects" not enabled), but that no longer holds (i.e., now
> we add "--objects" ourselves).  Do we need to give an updated
> rationale to keep bitmap disabled?

>>  	if (path_walk) {
>>  		strvec_push(&rp, "--boundary");
>> -		 /*
>> -		  * We must disable the bitmaps because we are removing
>> -		  * the --objects / --objects-edge[-aggressive] options.
>> -		  */
>> +		strvec_push(&rp, "--objects");
>>  		use_bitmap_index = 0;
>>  	} else if (thin) {
This old comment is perhaps confusing things. The important thing here
is to disable bitmaps with 'use_bitmap_index = 0;' (though perhaps not
for long [1]).

[1] https://lore.kernel.org/git/f50f8df01a9f216d5b4388b2fe4ff58077b574f3.1777853408.git.me@ttaylorr.com/

The path-walk API itself disables the objects walk for the revision
machinery in walk_objects_by_path():

	info->revs->blob_objects = info->revs->tree_objects = 0;

This allows the path-walk API to rely on the revision walk for a
_commits only_ walk and then have the path-walk API handle the trees
and blobs.

The reason we need to add "--objects" now is to allow for parsing the
"--filter" option without the revision logic complaining.

Thanks,
-Stolee