Re: [PATCH 0/9] Split up "object-file.c"

[Elijah Newren <newren@gmail.com>]

On Tue, Apr 8, 2025 at 3:35 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> Hi,
>
> "object-file.c" is quite a grab-bag of all kinds of different functions.
> Many of these functions aren't really a good fit though and should be
> owned by a different subsystem. This patch series tries to split up
> concerns a bit better by splitting out this functionality into other
> files:
>
>   - `safe_create_leading_directories()` is moved into "dir.c".
>   - `xmmap()` is moved into "wrapper.c".
>   - `git_open_cloexec()` is moved into "compat/open.c".
>   - Several functions attached to `struct index_state` are moved into
>     "read-cache.c".
>   - Several functions related to `struct object_store` are moved into a
>     new file "object-store.c".
>
> "object-file.c" now mostly contains logic to read and write loose object
> files, whereas "object-store.c" contains the higher-level logic to
> manage different object directories for a repository. Eventually, these
> will become the loose object backend as well as the `struct ref_store`
> equivalent for objects, respectively.
>
> The series is built on top of 9d22ac51228 (The third batch, 2025-04-07)
> with ps/object-wo-the-repository at 9442b1c919a (Merge remote-tracking
> branch 'junio/ps/object-wo-the-repository' into HEAD, 2025-04-08) merged
> into it.

I was able to apply with these instructions, and looking over the
series locally with --color-moved was very helpful to verify that you
were moving functions without modifying them (or, to notice where you
did modify them and why, e.g. removing the static declaration).

I very much appreciate the grouping of functions by area that is done
here with read-cache.c, wrapper.c, compat/open.c, dir.c, and the new
object-store.c, as well as the new enums to make the purpose of the
HASH_* flags clearer, and the reduction in the number of globals.

I only had a few comments on a couple of the patch commit messages.