Re: [PATCH v3 4/4] tmp-objdir: merge loose object index when migrating tmpdir

From: Justin Tobler <jltobler@gmail.com>
Date: Oct 30, 2026, 15:40 UTC
Message-ID: <auS4SzrVGYfAG8KA@denethor>
In-Reply-To: <CAOLa=ZR2AmC4EDM7V2A9UainUZ5md=dQT_pnJycxuqV7BhfxxA@mail.gmail.com>

On 26/10/30 08:55PM, Karthik Nayak wrote:

Show 17 quoted lines

> Justin Tobler <jltobler@gmail.com> writes:
> 
> > diff --git a/loose.c b/loose.c
> > index f2e111c25e..0ce9c7486e 100644
> > --- a/loose.c
> > +++ b/loose.c
> > @@ -311,3 +311,50 @@ void loose_object_map_clear(struct loose_object_map **map)
> >  	free(m);
> >  	*map = NULL;
> >  }
> > +
> > +int loose_object_idx_merge(struct repository *repo, const char *src,
> > +			   const char *dst)
> >
> 
> Nit: should we name this `loose_object_map_merge()` to stay consistent
> with the other `loose_object_map...()` functions?

I specifically did not call it `loose_object_map_merge()` because, unlike the other similarly named functions, this one does not update the in-memory loose object map for the object source. Here we are only merging the "loose-object-idx" files.

Show 11 quoted lines

> > +	if (loose_object_map_write(repo, map, fd) < 0) {
> > +		ret = error_errno(_("unable to write %s"),
> > +				  get_lock_file_path(&lock));
> > +		goto out;
> > +	}
> > +
> > +	if (commit_lock_file(&lock) < 0) {
> > +		ret = error_errno(_("unable to write %s"), dst);
> 
> Nit: Shouldn't this be unable to commit/rename? This will differentiate
> from the above write error

I choose "write" here because logically we are trying atomically write the final "loose-object-idx" file. IMO, I think the difference in filepath should be sufficient to differentiate between the two as well. Ultimately though, I don't feel strongly either way about this and am not sure it's worth a reroll, so I'm inclined to leave it as-is.

-Justin