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

From: Taylor Blau <[email-masked]>
Date: Thu, 7 Aug 2025 18:16:32 -0400

On Thu, Aug 07, 2025 at 10:09:53AM +0200, Patrick Steinhardt wrote:
> 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 <[email-masked]>
> ---
>  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.

> @@ -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