Re: [PATCH 1/3] pack-objects: fix handling of multiple --filter options

[Jeff King <peff@peff.net>]

On Sun, Nov 13, 2022 at 12:01:51AM -0500, Taylor Blau wrote:

> > Yikes. I wondered how we managed to miss this in the tests. Surely we
> > have some examples that use multiple filters?
> >
> > I think the answer is that we do in t5616, but in practice they didn't
> > hit this bug because of the way filter-specs are treated server-sid3.
> > Even if we say "clone --filter=foo --filter=bar" on the client, the
> > server upload-pack coalesces that into a single "combine:foo+bar"
> > string, and that's what its pack-objects sees.
> 
> ...Or in other words, clients aren't broken here because we always send
> "combine" filters when multiple `--filter` arguments are passed to `git
> clone`, `git fetch`, etc.

Right.

> Is that always the case? I *think* so, but it would be nice to know if
> there was a way that clients would not send the `combine` filter with >1
> filter.

I think it's always the case at least with Git's code, because the
client-side send_filter() sends from list_objects_filter_spec(), which
takes the single-string spec of the top-level combine filter. It would
have to explicitly iterate over the sub-filters to come up with anything
else, and I don't think we even have a function for that (we do when
evaluating them, of course, but not for generically iterating or
producing a multi-string set of specs).

And on the server side, even if a client did send multiple "filter"
lines, I think we'd hit list_objects_filter_die_if_populated(), so we'd
complain.

I don't know of any other code paths that could hit this (can you filter
on an outgoing push?). But in the end I'm not sure it even matters. We
should be fixing the bug anyway, and this is all just understanding how
far-reaching its effects are (and the answer seems to be: not very).

-Peff