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

From: Jeff King <peff@peff.net>
Date: Apr 9, 2021, 16:05 UTC
Message-ID: <YHB7R8hVRt+V+i2W@coredump.intra.peff.net>
In-Reply-To: <YHA3EkH3HDe45EKA@ncase>

On Fri, Apr 09, 2021 at 01:14:26PM +0200, Patrick Steinhardt wrote:

Show 12 quoted lines

> > 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.

I don't think cat-file does quite the same thing. An important part of rev-list is that it is traversing. So it is determining both reachability, but also eliminating excluded objects. For example, there is no cat-file equivalent (and can never be) of:

  git rev-list --objects --filter=object:type=blob $old..$new

Likewise for list-objects (which cat-file really _does_ cover, with --batch-all-objects). Obviously you can pair rev-list with cat-file to traverse and then filter, but the whole point of this series is to do so more efficiently.

So I think putting this into rev-list is the only sensible option. The question is just whether to use --filter, or if it should be:

  git rev-list --show-blobs --show-trees $old..$new

with rules like:

  - if no --show-X is given, show only commits

  - if one or more --show-X is given, show all of them (but nothing else)

  - --objects is equivalent to providing each of --show-commits
    --show-blobs --show-trees --show-tags

-Peff