Re: [PATCH v2 4/4] tmp-objdir: merge loose object index when migrating tmpdir
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Oct 29, 2026, 21:20 UTC
- Message-ID
- <auO4Photxmne1R5Z@denethor>
- In-Reply-To
- <auL-ZBontaECu8vo@pks.im>
On 26/10/29 09:05AM, Patrick Steinhardt wrote:
Show 15 quoted lines
> 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.
Show 12 quoted lines
> > 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