Re: [PATCH v2 0/6] PATH WALK I: The path-walk API

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 25, 2024, 08:56 UTC
Message-ID: <Z0Q7oGF6Q5U-f4VX@pks.im>
In-Reply-To: <Zz+61fat+vGgb+xL@nand.local>

On Thu, Nov 21, 2024 at 05:57:25PM -0500, Taylor Blau wrote:

Show 28 quoted lines

> On Sat, Nov 09, 2024 at 07:41:06PM +0000, Derrick Stolee via GitGitGadget wrote:
> > Derrick Stolee (6):
> >   path-walk: introduce an object walk by path
> >   test-lib-functions: add test_cmp_sorted
> >   t6601: add helper for testing path-walk API
> >   path-walk: allow consumer to specify object types
> >   path-walk: visit tags and cached objects
> >   path-walk: mark trees and blobs as UNINTERESTING
> 
> My apologies for taking so long to review this. Having rad through the
> patches in detail, a couple of thoughts:
> 
>   - First, I like the structure that you decided on for this series. It
>     nicely demonstrates a minimal caller for this new API instead of
>     implementing a bunch of untested code. I think that's a great way to
>     lay out things up until this point.
> 
>   - Second, I read through the existing API and only had minor comments.
>     I read through the implementation in detail and found it to match my
>     expectation of how each step should function.
> 
> So my take-away from spending a few hours with this series is that
> everything seems on track so far, and I think this is in a good spot to
> build on for more path-walk features.
> 
> That all said, I am still not totally sold on the idea that we need a
> separate path-based traversal given the significant benefits of the
> full-name hash approach that I reviewed earlier today.

The repo size reductions achieved via the path-walk API was only one of the selling points of this series. And from my current understanding we will likely not end up realizing those gains via path-walk, but rather via the much simpler full-name hash algorithm indeed.

But there were two more selling points:

  - git-survey(1) as a native replacement for git-sizer(1). I think it
    is a great idea to have a native tool that allows us to gain deep
    insights into a repository so that we get better signals from our
    users in case they face problems with their repository. I'd love to
    have this tool as a baseline for an extensible format where we can
    eventually also start reporting the health state of refs as well as
    any auxiliary data structures.

  - git-backfill(1) as a helper to fetch blobs more efficiently from a
    promisor remote. This is a boon to have as well in our odyssey
    towards a better UI/UX with huge monorepos.

Both of these tools are quite exciting to me, and there is a need for having such tools from my point of view.

The question of course is whether these tools require the path-walk API, or whether they could be built on top of existing functionality. But if there are good reasons why the existing functionality is insufficient then I'd be all for having the path-walk API, even if it doesn't help us with repo size reductions as we initially thought.

Patrick