Re: [PATCH 01/14] fsck: drop `the_repository` in `fsck_walk()`

[Patrick Steinhardt <ps@pks.im>]

On Fri, Mar 20, 2026 at 04:09:24PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The function `fsck_walk()` and its object type specific functions
> > `fsck_walk_tree()` et al implicitly rely on `the_repository`. Remove
> > this dependency by injecting the repository as a parameter instead.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  builtin/fsck.c           |  6 +++---
> >  builtin/index-pack.c     |  2 +-
> >  builtin/unpack-objects.c |  2 +-
> >  fsck.c                   | 40 ++++++++++++++++++++++++++--------------
> >  fsck.h                   | 15 ++++++++++++---
> >  5 files changed, 43 insertions(+), 22 deletions(-)
> 
> OK.
> 
> While the conversion is cleanly done, the resulting code makes me
> wonder if fsck_options struct
> 
>         struct fsck_options {
>                 fsck_walk_func walk;
>                 fsck_error error_func;
>                 unsigned strict;
>                 unsigned verbose;
>                 enum fsck_msg_type *msg_type;
>                 struct oidset skip_oids;
>                 struct oidset gitmodules_found;
>                 struct oidset gitmodules_done;
>                 struct oidset gitattributes_found;
>                 struct oidset gitattributes_done;
>                 kh_oid_map_t *object_names;
>         };
> 
> should gain a repository pointer.  After all, these oidset and oidmap
> are about objects in one particular repository.

That's fair, and indeed this was my first attempt at solving the
dependency on `the_repository`. The reason why I discarded that approach
though is the way that we currenlty initialize `struct fsck_options` via
a set of macros. And as we often do this in file-static scope we don't
have a repository available there.

I had a second look at this though and have now introduced a new
function to initialize the options instead. It's a tiny bit ugly, and
ideally we'd just not have the options in global scope in the first
place. But I guess that overall it's okayish.

Will update and send the revised version soonish.

Thanks!

Patrick