From: Justin Tobler <jltobler@gmail.com>
Date: Thu, 29 Oct 2026 21:20:35 GMT
Subject: Re: [PATCH v2 4/4] tmp-objdir: merge loose object index when migrating tmpdir
Message-ID: <auO4Photxmne1R5Z@denethor>
In-Reply-To: <auL-ZBontaECu8vo@pks.im>

On 26/10/29 09:05AM, Patrick Steinhardt wrote:
> On Wed, Oct 28, 2026 at 07:24:13PM -0500, Justin Tobler wrote:
> > +	if (loose_object_map_read(repo, dst, map, NULL) < 0) {
> > +		ret = error(_("unable to read loose object index %s"), dst);
> > +		goto out;
> > +	}
> > +
> > +	if (loose_object_map_read(repo, src, map, NULL) < 0) {
> > +		ret = error(_("unable to read loose object index %s"), src);
> > +		goto out;
> > +	}
> 
> It makes sense that we read `dst` under the lock so that we don't risk
> losing data that a concurrent process writes into it. But to minimize
> the critical section, can't we move `src` out of it and read it before
> we lock?

Ya good point. The src file can be read prior to acquiring the lock.
Will update.

> > diff --git a/loose.h b/loose.h
> > index ed663ac550..35b46f4be5 100644
> > --- a/loose.h
> > +++ b/loose.h
> > @@ -23,5 +23,7 @@ int repo_add_loose_object_map(struct odb_source_loose *loose,
> >  			      const struct object_id *compat_oid);
> >  int repo_read_loose_object_map(struct repository *repo);
> >  int repo_write_loose_object_map(struct repository *repo);
> > +int loose_object_idx_merge(struct repository *repo, const char *src,
> > +			   const char *dst);
> 
> Do we maybe want to add a comment to this new function?

Will do!

Thanks,
-Justin