Re: [PATCH v2 0/3] pack-objects: fix and simplify --filter handling

[Jeff King <peff@peff.net>]

On Sun, Nov 20, 2022 at 11:03:35AM +0100, René Scharfe wrote:

> Fix a regression that prevents using multiple --filter options, simplify
> the option parsing code and avoid relying on undefined behavior in it.
> 
> Patch 3 conflicts with cc/filtered-repack in seen, but not semantically.
> 
> Changes since v1:
> - Added patch 1 to fix an issue with existing tests.
> - Separate patch 2 for new tests.
> - Test using blob size filters, only, which is a bit simpler.
> - Test both combinations to also catch not just the current
>   last-one-wins regression, but also a possible future first-one-wins
>   issue.
> - Actually revert 5cb28270a1 (pack-objects: lazily set up
>   "struct rev_info", don't leak, 2022-03-28) instead of having a
>   minimal fix and then adding some kind of middle ground by using a
>   separate struct list_objects_filter_options.

Thanks, this looks good to me. The new test is a little funny in that
it's somewhat-nonsensical input, but I find it unlikely that we'd ever
add code to treat two instances of the same filter type specially.
Compared to something that produces two distinct effects (like a blob
and tree-depth filter combined), it also requires the "test in both
directions" trick to cover everything.

But I do like that it's easier to reason about what the output _should_
be. So I don't feel too strongly about it either way.

-Peff