Re: [PATCH 3/6] object-file: move iteration over loose files into backend

[Kaartic Sivaraam <kaartic.sivaraam@gmail.com>]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> The logic to iterate over files part of the "loose" backend is contained
> in "object-file.c" even though it's an implementation detail of the
> backend itself. Move the logic into the backend accordingly.
> 
> Note that the function is a bit on the weird side: it accepts a generic
> `struct odb_source` as input and doesn't cast it down to its backend at
> all. This is because all it requires access to is the `path` member of
> the source, and from thereon it knows how to iterate the subdirectories
> of the "loose" backend.
> 
> We should adapt this function to accept `struct odb_source_loose` to
> properly reflect that it expects a path to a loose object database. But
> that would require changes to a bunch of callers without much benefit.
> So for now, the function signature is left as-is. But eventually we may
> want to change it once we have reworked a couple of its callers.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   builtin/count-objects.c |  1 +
>   builtin/gc.c            |  1 +
>   builtin/prune.c         |  1 +
>   object-file.c           | 92 -------------------------------------------------
>   object-file.h           | 44 -----------------------
>   odb/source-loose.c      | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
>   odb/source-loose.h      | 33 ++++++++++++++++++
>   prune-packed.c          |  1 +
>   8 files changed, 129 insertions(+), 136 deletions(-)
> 
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index 3f7b20fde53..aae040567e3 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -14,6 +14,7 @@
>   #include "quote.h"
>   #include "packfile.h"
>   #include "object-file.h"
> +#include "odb/source-loose.h"
>   

It seems like we can remove the include of object-file.h while at it.
>   static unsigned long garbage;
>   static off_t size_garbage;
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 81cecf6b6c3..d3ba6f0c9e9 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -31,6 +31,7 @@
>   #include "packfile.h"
>   #include "object-file.h"
>   #include "odb.h"
> +#include "odb/source-loose.h"
>   #include "path.h"
>   #include "reflog.h"
>   #include "rerere.h"
> diff --git a/builtin/prune.c b/builtin/prune.c
> index 97f4c1418bc..af2a547d931 100644
> --- a/builtin/prune.c
> +++ b/builtin/prune.c
> @@ -17,6 +17,7 @@
>   #include "object-file.h"
>   #include "object-name.h"
>   #include "odb.h"
> +#include "odb/source-loose.h"
>   #include "shallow.h"
>   

Ditto for the above two.

>   
> +static int for_each_file_in_obj_subdir(unsigned int subdir_nr,
> +				       struct strbuf *path,
> +				       const struct git_hash_algo *algop,
> +				       each_loose_object_fn obj_cb,
> +				       each_loose_cruft_fn cruft_cb,
> +				       each_loose_subdir_fn subdir_cb,
> +				       void *data)

This was made 'static' during the move but that seems to make sense to me.

> diff --git a/prune-packed.c b/prune-packed.c
> index b6749b8d132..d0a8a062b97 100644
> --- a/prune-packed.c
> +++ b/prune-packed.c
> @@ -3,6 +3,7 @@
>   #include "git-compat-util.h"
>   #include "gettext.h"
>   #include "object-file.h"
> +#include "odb/source-loose.h"

This could also benefit from losing the include of object-file.h

Rest of the patch looks good to me.

-- 
Sivaraam