Re: [PATCH v2 00/10] pack-objects: integrate --path-walk and some --filter options

[Derrick Stolee <stolee@gmail.com>]

On 5/5/2026 3:01 PM, Taylor Blau wrote:
> On Tue, May 05, 2026 at 12:18:28PM -0400, Derrick Stolee wrote:
>> One thing I discovered when testing Taylor's series is that this series
>> introduces new test failures when run with GIT_TEST_PACK_PATH_WALK=1.
>> It's probably due to new cases that are fragile to the difference
>> between delta compression algorithms, but are now exposed after the
>> filters are no longer disabling --path-walk even with that test var.
>>
>> I'll make sure these are fixed in the next version.
> 
> Thanks for looking into it.
> 
> It looks like this bisects (at least in t5310) to "path-walk: support
> blobless filter", which is 03/10 in this series. I suspect that there
> are other failures that are indeed due to delta selection sensitivity as
> you note, but in this case it looks like we are actually not sending the
> right set of objects:
> 
>     + git clone --no-local --bare --filter=blob:none . partial-clone.git
>     Cloning into bare repository 'partial-clone.git'...
>     [...]
>     fatal: bad object 782f60206c837dcd3d441e106549ad6f58de55b5
>     fatal: remote did not send all necessary objects
>     error: last command exited with $?=128
>     not ok 26 - partial clone from bitmapped repository
> 
> I think this is a consequence of us not sending directly-referenced
> blobs with `--filter=blob:none` when running the filters through
> `--path-walk`. Something like:
> 
> --- 8< ---
> diff --git a/path-walk.c b/path-walk.c
> index a4dd197c37e..dbad01287e2 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -159,8 +159,8 @@ static int add_tree_entries(struct path_walk_context *ctx,
>  		if (S_ISGITLINK(entry.mode))
>  			continue;
> 
> -		/* If the caller doesn't want blobs, then don't bother. */
> -		if (!ctx->info->blobs && type == OBJ_BLOB)
> +		if ((!ctx->info->blobs || ctx->info->prune_tree_blobs) &&
> +		    type == OBJ_BLOB)
>  			continue;
> 
>  		if (type == OBJ_TREE) {
> @@ -495,7 +495,7 @@ static int prepare_filters(struct path_walk_info *info,
> 
>  	case LOFC_BLOB_NONE:
>  		if (info) {
> -			info->blobs = 0;
> +			info->prune_tree_blobs = 1;
>  			list_objects_filter_release(options);
>  		}
>  		return 1;
> --- >8 ---

Thanks for this suggestion. I got pulled away from my investigation, so
wasn't to this point yet.
> fixes t5310 for me. I haven't looked into any of the other failures yet
> since you mentioned that you're looking into them, but let me know if
> you want to tag-team any of these.
> 
> (As a related side-note, I noticed that GIT_TEST_PACK_PATH_WALK=1 is not
> currently in the TEST-vars CI build.  I'm not sure if there are
> historical reasons for leaving it out, but if not I think it would be
> worthwhile to add it.)
I think the initial idea was that the feature was too niche to add it to
the CI builds right away. Your series is going to make it a lot more
important, so adding this to CI builds may be valuable.

Thanks,
-Stolee