Re: [PATCH v4 0/8] rev-list: implement object type filter

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2825 bytes --]

On Tue, Apr 13, 2021 at 03:45:21AM -0400, Jeff King wrote:
> On Mon, Apr 12, 2021 at 03:37:14PM +0200, Patrick Steinhardt wrote:
> 
> > this is the fourth version of my patch series which implements a new
> > `object:type` filter for git-rev-list(1) and git-upload-pack(1) and
> > extends support for bitmap indices to work with combined filters.
> > 
> > Changes compared to v3:
> > 
> >     - Small style fix to not pass an empty string and `0`, but instead
> >       simply pass two `NULL` pointers to
> >       `list_objects_filter__filter_object, pointed out by Junio.
> > 
> >     - I've changed patch 7/8 as proposed by Peff: support of combined
> >       filters is now determined directly in `filter_bitmap()`, without
> >       having to mirror all filter types in the new `filter_supported()`
> >       function.
> > 
> >     - Renamed `--filter-provided-revisions` to
> >       `--filter-provided-objects` as proposed by Peff and addressed both
> >       commit message and tests as pointed out by Philip.
> 
> Thanks. I have no more nits to pick. The only question left for me is
> the big one of "do we like this with --filter, or should it be some kind
> of rev-list option", as discussed in:
> 
>   https://lore.kernel.org/git/YHB7R8hVRt+V+i2W@coredump.intra.peff.net/
> 
> As I said elsewhere, I'm OK going with this route. I just wanted to call
> attention to that earlier response in case you had any final thoughts
> (I'm guessing your final thoughts are "jeez, I already wrote it with
> --filter, so let's just go with that" which is perfectly reasonable to
> me. ;) ).

I don't think it would help usability to add new `--show-blobs` and
`--show-trees` options. The user interface to show this kind of
information exists already with `--objects`, and by adding another way
of asking a similar query would raise the question of how these two ways
interact with each other:

    - Does `--show-blobs` have effect if `--objects` is not set?

    - Is `--objects` redundant if we have `--show-blobs`, or would
      `--objects --show-blobs` list all objects regardless of whether
      they're blobs or not?

    - What would happen if the user says `--show-blobs --no-objects`?

    - Are these options mutually exclusive?

We avoid all these questions by just adding it as a filter.

Furthermore, the filter also allows future iterations which build on top
of this. If we had a combined OR filter in addition to the existing
combined AND filter, the user could say "Give me all blobs which aren't
bigger than a specific size PLUS all trees with a depth smaller than 5
PLUS all commits and tags". It's not like I'd know of a specific usecase
for this right now, but I think the potential of having such filters in
the future is a plus.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]