Re: [PATCH 2/4] odb/source-files: reload alternates on retry

[Patrick Steinhardt <ps@pks.im>]

On Wed, Oct 21, 2026 at 04:32:34PM +0200, Christian Couder wrote:
> On Tue, Oct 20, 2026 at 11:01 AM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > diff --git a/odb/source-files.c b/odb/source-files.c
> > index d31da6e5d9..86f3fd1136 100644
> > --- a/odb/source-files.c
> > +++ b/odb/source-files.c
> > @@ -425,6 +425,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >                                                               struct strbuf *errmsg)
> >  {
> >         struct odb_source_files *files = odb_source_files_downcast(source);
> > +       struct odb_files_dir **tail = NULL;
> >         enum odb_read_status status = ODB_READ_NOT_FOUND;
> >         enum odb_read_status ret;
> >         bool retry = false;
> > @@ -448,10 +449,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >                         status = ret;
> >         }
> >
> > -       for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
> > -               if (retry)
> > -                       break;
> > -
> > +       /*
> > +        * When we have reprepared alternates then we only re-scan directories
> > +        * that were newly added.
> > +        */
> > +       for (struct odb_files_dir *dir = tail ? *tail : files->dirs; dir; dir = dir->next) {
> >                 ret = odb_source_read_object_info(&dir->loose->base, oid, oi, flags,
> >                                                   status == ODB_READ_NOT_FOUND ? errmsg : NULL);
> >                 if (!ret)
> > @@ -461,6 +463,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >         }
> >
> >         if (!retry && !(flags & OBJECT_INFO_QUICK)) {
> > +               tail = files->dirs_tail;
> > +               prepare_alternates(files, NULL);
> 
> This calls `read_alternates(files->dirs->abspath, ...)`, but when we
> use a temporary primary directory, `files->dirs` is that temporary
> primary directory. The prepare_alternates() call will then quietly
> find nothing new because temporary primary directories have no
> alternates.

*sigh* True. I really wish that this in-place modification of dirs just
went away entirely. But that's a bigger change to make.

[snip]
> I wonder if it would be better to always reload the repository's real
> alternates, instead of `files->dirs`'s alternates. This way, objects
> that moved to an alternate would still be found, not only during a
> receive-pack like before 5946a564cd, but whenever a temporary primary
> directory is used.

Yeah, we should probably use `files->base.path` instead.

Thanks!

Patrick