Re: [PATCH v2 3/9] odb: return newly created in-memory sources
On Thu, Aug 07, 2025 at 06:16:32PM -0400, Taylor Blau wrote:
Show 36 quoted lines
> 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.
Show 12 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?
Ah, these are inconsistent indeed, good catch. Will fix.
Patrick