Re: [PATCH v2 0/8] rev-parse: implement object type filter

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 9, 2021, 11:14 UTC
Message-ID: <YHA3EkH3HDe45EKA@ncase>
In-Reply-To: <YGyjtCWqdeCj3S3U@coredump.intra.peff.net>

On Tue, Apr 06, 2021 at 02:08:52PM -0400, Jeff King wrote:

Show 36 quoted lines

> On Sat, Mar 20, 2021 at 02:10:41PM -0700, Junio C Hamano wrote:
> 
> > Patrick Steinhardt <ps@pks.im> writes:
> > 
> > > this is the second version of my patch series which implements a new
> > > `object:type` filter for git-rev-parse(1) and git-upload-pack(1) and
> > > extends support for bitmap indices to work with combined filters.
> > > ...
> > > Please see the attached range-diff for more details.
> > 
> > Any comment from stakeholders?
> 
> Sorry, this languished on my to-review list for a while.
> 
> I took a careful look. I found a few small nits, but the code overall
> looks pretty good.
> 
> I do still find the use of the filter code here a _little_ bit
> off-putting. It makes perfect sense in some ways: we are asking rev-list
> to filter the output, and it keeps our implementation nice and simple.
> It took me a while to figure out what I think makes it weird, but I
> think it's:
> 
>   - the partial-clone feature exposes the filter mechanism in a very
>     transparent way. So while it's not _wrong_ to be able to ask for a
>     partial clone of only trees, it's an odd thing that nobody would
>     really use in practice. And so it's a bit funny that it gets
>     documented alongside blob:limit, etc.
> 
>   - for the same reason, it's very rigid. We have no way to say "this
>     filter OR that filter", and are unlikely to grow them (because this
>     is all part of the network protocol). Whereas it's perfectly
>     reasonable for somebody to ask for "trees and blobs" via rev-list.
> 
> I dunno. Those aren't objections exactly. Just trying to put my finger
> on why my initial reaction was "huh, why --filter?".

Yeah, I do kind of share these concerns. Ideally, we'd provide a nicer only-user-facing interface to query the repository for various objects. git-cat-file(1) would be the obvious thing that first gets into my mind, where it would be nice to have it filter stuff. But then on the other hand, it's really rather a simple "Give me what I tell you to" binary, which is probably a good thing. Other than that I don't think there's any executable that'd be a good fit -- we could do this via a new git-list-objects(1), but then again git-rev-list(1) already does most of what git-list-objects(1) would do, so why bother.

It kind of feels like git-checkout(1) to me: it does many things, and if you know how to wield it it works perfectly fine. But the user interface is lacking, which is why it was split up into git-switch(1) and git-restore(1). It's telling already that the summary of git-rev-list(1) is "Lists commit objects in reverse chronological order". I mean yes, that's what it does in many cases. But there's just as many cases where it doesn't.

Patrick