From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 11 Dec 2025 09:29:43 GMT
Subject: Re: [PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`
Message-ID: <aTqPBygCfm1hWtL-@pks.im>
In-Reply-To: <aTpxB8gS7wG7rRJQ@szeder.dev>

On Thu, Dec 11, 2025 at 08:21:43AM +0100, SZEDER Gábor wrote:
> On Wed, Dec 10, 2025 at 04:32:37PM +0100, Patrick Steinhardt wrote:
> > When calling `odb_add_to_alternates_file()` we know to add the newly
> > added source to the object database in case we have already loaded
> > alternates. This is done so that we can make its objects accessible
> > immediately without having to fully reload all alternates.
> > 
> > The way we do this though is to call `link_alt_odb_entries()`, which
> > adds _multiple_ sources to the object database source in case we have
> > newline-separated entries. This behaviour is not documented in the
> > function documentation of `odb_add_to_alternates_file()`, and all
> > callers only ever pass a single directory to it. It's thus entirely
> > surprising and a conceptual mismatch.
> > 
> > Fix this issue by directly calling `odb_add_source()` instead.
> 
> OK, but:
> 
> > diff --git a/odb.c b/odb.c
> > index e314f86c3b..d97e50fb61 100644
> > --- a/odb.c
> > +++ b/odb.c
> > @@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
> >  		if (commit_lock_file(&lock))
> >  			die_errno(_("unable to move new alternates file into place"));
> >  		if (odb->loaded_alternates)
> > -			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
> > +			odb_add_source(odb, dir, 0);
> 
>       CC odb.o
>   odb.c: In function ‘odb_add_to_alternates_file’:
>   odb.c:341:25: error: implicit declaration of function ‘odb_add_source’; did you mean ‘odb_find_source’? [-Werror=implicit-function-declaration]
>     341 |                         odb_add_source(odb, dir, 0);
>         |                         ^~~~~~~~~~~~~~
>         |                         odb_find_source
>   cc1: all warnings being treated as errors
>   make: *** [Makefile:2864: odb.o] Error 1

Hrmpf, I only fixed this callsite in a later commit indeed.

> Note, that several commit messages also refer to this non-existing
> function from the previous round.

True. Will fix both of these issues, thanks!

Patrick