On Tue, Apr 06, 2021 at 02:08:52PM -0400, Jeff King wrote: > On Sat, Mar 20, 2021 at 02:10:41PM -0700, Junio C Hamano wrote: > > > Patrick Steinhardt <ps@xxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
