From: Taylor Blau <me@ttaylorr.com>
Date: Fri, 10 Oct 2025 22:49:54 GMT
Subject: Re: [PATCH 04/49] builtin/repack.c: avoid "the_repository" when removing packs
Message-ID: <aOmNks8E1wQC7MnS@nand.local>
In-Reply-To: <20251010052256.GB1965904@coredump.intra.peff.net>

On Fri, Oct 10, 2025 at 01:22:56AM -0400, Jeff King wrote:
> On Sun, Sep 28, 2025 at 06:07:26PM -0400, Taylor Blau wrote:
>
> > -static void remove_redundant_pack(const char *dir_name, const char *base_name)
> > +static void remove_redundant_pack(struct repository *repo,
> > +				  const char *dir_name, const char *base_name)
> >  {
> >  	struct strbuf buf = STRBUF_INIT;
> > -	struct odb_source *source = the_repository->objects->sources;
> > +	struct odb_source *source = repo->objects->sources;
> >  	struct multi_pack_index *m = get_multi_pack_index(source);
> >  	strbuf_addf(&buf, "%s.pack", base_name);
> >  	if (m && source->local && midx_contains_pack(m, buf.buf))
> > -		clear_midx_file(the_repository);
> > +		clear_midx_file(repo);
> >  	strbuf_insertf(&buf, 0, "%s/", dir_name);
> >  	unlink_pack_path(buf.buf, 1);
> >  	strbuf_release(&buf);
> >  }
>
> Ah, yeah, this is a good example of what I was talking about in the
> other part of the thread. Probably this function could just take the
> object_source pointer to find the midx. But then it wouldn't be able to
> use clear_midx_file(). That function arguably should itself take an
> object_source pointer and not a repo pointer, but it feels like minimal
> gain to go around trying to tighten these (and certainly something we
> could easily do later, even programatically, if we cared).

Yup, I agree. The changes to `clear_midx_file()` seem like a good
candidate for #leftoverbits to me.

Thanks,
Taylor