Re: [PATCH v3 00/10] Split up "object-file.c"

[Elijah Newren <newren@gmail.com>]

On Tue, Apr 15, 2025 at 2:38 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.
>
> Changes in v2:
>   - Fix a grammar issue in one of the commit messages.
>   - Link to v1: https://lore.kernel.org/r/20250408-pks-split-object-file-v1-0-f1fd50191143@pks.im
>
> Changes in v3:
>   - Rename `mkdir_in_gitdir()` to `safe_create_dir_in_gitdir()` to match
>     naming of similar functions.
>   - Move `safe_create_leading_directories()` et al into "path.c" instead
>     of into "dir.c". This also requires us to start injecting a repo via
>     parameters as "path.c" doesn't have `the_repository` available
>     anymore.
>   - Drop the commit that moves `index_blob_stream()` and related
>     functions.
>   - Expand the reasoning why we want to have cached objects per object
>     store instead of globally.
>   - Drop `index_blob_stream()`, which is a trivial wrapper around
>     `index_blob_bulk_checkin()`.
>   - Link to v2: https://lore.kernel.org/r/20250411-pks-split-object-file-v2-0-2bea0c9033ae@pks.im

v3 also addressed my feedback on v1 & v2.

[...]
> Range-diff versus v2:

I read over the range-diff and the three new patches (1, 2, & 7); this
round looks good to me.  I particularly like the extended rationale in
the commit message for what is now patch 9.