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

[Patrick Steinhardt <ps@pks.im>]

On Thu, Aug 07, 2025 at 06:16:32PM -0400, Taylor Blau wrote:
> 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 <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.

Done now.

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

Ah, these are inconsistent indeed, good catch. Will fix.

Patrick