Re: [PATCH 3/5] odb/source-files: make the second read an implementation detail

[Patrick Steinhardt <ps@pks.im>]

On Tue, Oct 13, 2026 at 04:28:37PM -0500, Justin Tobler wrote:
> On 26/10/13 09:36AM, Patrick Steinhardt wrote:
> > Note that the "packed" source also used this flag to figure out whether
> > it should try to surface an object via a packfile that should have the
> > object as determined by a multi-pack index, but where the packfile has
> > vanished. The multi-pack index is stale in that case, and that scenario
> > should really only happen when the process is racing with a concurrent
> > maintenance task.
> > 
> > This code is adapted to instead use `OBJECT_INFO_QUICK`, which means
> > that the fallback may trigger on the first read already, but not in a
> > case where callers know to handle false negatives already. In the worst
> > case this could cause another scan through all packfiles covered by the
> > multi-pack index. But that scan should be fast enough anyway, and it
> > only triggers when we hit the race. So the effect thereof should be
> > limited.
> 
> Would it be more correct to say that we are now using the absence of the
> OBJECT_INFO_QUICK flag trigger the fallback?

I dunno. Whether we check for absence or presence, in either case we use
the flag. So I'm inclined to leave this as-is as it starts to read a bit
awkward otherwise.

Let me know in case you feel strongly though.

> > diff --git a/odb/source-files.c b/odb/source-files.c
> > index 54235821d5..9788a5adf4 100644
> > --- a/odb/source-files.c
> > +++ b/odb/source-files.c
> > @@ -444,6 +449,9 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >  	}
> >  
> >  	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
> > +		if (retry)
> > +			break;
> 
> IIUC, we don't need to bother checking the loose objects a second time
> so we exit the look early.
> 
> Nit: Mostly just a style preference, but instead of entering the loop
> just to break out early for retries, could we just return early prior
> the loop starting? It looks like the rest of the function is skipped on
> the second read anyways.

Yes. But I've got a follow-up patch series that has some fixes for how
we reload alternates, and as part of that I'll start to iterate over
loose object directories that have been added after re-reading
".git/objects/info/alternates". So this is a bit awkward right now, but
makes the follow-up series a bit easier.

Patrick