Re: [PATCH v2 3/9] odb: return newly created in-memory sources

From: Taylor Blau <me@ttaylorr.com>
Date: Aug 7, 2025, 22:16 UTC
Message-ID: <aJUlwLJyxSmD4EQF@nand.local>
In-Reply-To: <20250807-b4-pks-midx-deduplicate-source-info-v2-3-bcffb8fc119c@pks.im>

On Thu, Aug 07, 2025 at 10:09:53AM +0200, Patrick Steinhardt wrote:

Show 30 quoted lines

> Callers have no trivial way to obtain the newly created object database
> source when adding it to the in-memory list of alternates. While not yet
> needed anywhere, a subsequent commit will want to obtain that pointer.
>
> Refactor the function to return the source to make it easily accessible.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 37 +++++++++++++++++++++----------------
>  odb.h |  4 ++--
>  2 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 61104b7cb8..7793816f81 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -139,23 +139,22 @@ static void read_info_alternates(struct object_database *odb,
>  				 const char *relative_base,
>  				 int depth);
>
> -static int link_alt_odb_entry(struct object_database *odb,
> -			      const struct strbuf *entry,
> -			      const char *relative_base,
> -			      int depth,
> -			      const char *normalized_objdir)
> +static struct odb_source *link_alt_odb_entry(struct object_database *odb,
> +					     const char *entry,
> +					     const char *relative_base,
> +					     int depth,
> +					     const char *normalized_objdir)

Beyond returning an odb_source pointer from this function, it looks like "entry" was changed form a strbuf to a bare const char *. Is that intentional? If so, it might be worth mentioning it in the commit message, or optionally doing that as a preparatory step.

Show 8 quoted lines

> @@ -316,17 +315,23 @@ void odb_add_to_alternates_file(struct object_database *odb,
>  	free(alts);
>  }
>
> -void odb_add_to_alternates_memory(struct object_database *odb,
> -				  const char *reference)
> +struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
> +						const char *reference)

Not the fault of your patch, but while we're refactoring, do you think it makes sense to rename the second parameter here to "dir", or changing "dir" to "reference" in the declaration of this function over in odb.h?

Thanks, Taylor