From: Justin Tobler <jltobler@gmail.com>
Date: Wed, 17 Sep 2025 22:32:43 GMT
Subject: Re: [PATCH v5 09/15] packfile: split up responsibilities of `reprepare_packed_git()`
Message-ID: <p6f3xtitrepajnaho4eqwoh7o2qweim6adr3x726dbwg63kmny@cx5n43myawue>
In-Reply-To: <20250915-b4-pks-packfiles-store-v5-9-d6340350934f@pks.im>

On 25/09/15 10:54AM, Patrick Steinhardt wrote:
> In `reprepare_packed_git()` we perform a couple of operations:
> 
>   - We reload alternate object directories.
> 
>   - We clear the loose object cache.
> 
>   - We reprepare packfiles.
> 
> While the logic is hosted in "packfile.c", it clearly reaches into other
> subsystems that aren't related to packfiles.
> 
> Split up the responsibility and introduce `odb_reprepare()` which now
> becomes responsible for repreparing the whole object database. The
> existing `reprepare_packed_git()` function is refactored accordingly and
> only cares about reloading the packfile store now.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> diff --git a/odb.h b/odb.h
> index 1c998a2478..ef34132c58 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -163,6 +163,12 @@ struct object_database {
>  struct object_database *odb_new(struct repository *repo);
>  void odb_clear(struct object_database *o);
>  
> +/*
> + * Clear caches, reload alternates and then reload object sources so that new
> + * objects may become accessible.
> + */
> +void odb_reprepare(struct object_database *o);

nit: IMO, something like `odb_reload()` sounds a bit better, but it's
not a big deal either way.

[snip]
> diff --git a/packfile.h b/packfile.h
> index bf66211986..a85ff607fe 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -112,6 +112,14 @@ void packfile_store_free(struct packfile_store *store);
>   */
>  void packfile_store_close(struct packfile_store *store);
>  
> +/*
> + * Clear the packfile caches and try to look up any new packfiles that have
> + * appeared since last preparing the packfiles store.
> + *
> + * This function must be called under the `odb_read_lock()`.
> + */
> +void packfile_store_reprepare(struct packfile_store *store);

nit: I also think `packfile_store_reload()` sounds a bit nicer here.

Overall this patch looks good. :)

-Justin