On Thu, Nov 21, 2024 at 05:57:25PM -0500, Taylor Blau wrote:
> 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
