Re: [PATCH 6/6] packfile: rename `packfile_store_get_all_packs()`

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 9, 2025, 06:36 UTC
Message-ID: <aOdX4_e9K_1p1pyv@pks.im>
In-Reply-To: <aObOFciwYsvTWT0e@nand.local>

On Wed, Oct 08, 2025 at 04:48:21PM -0400, Taylor Blau wrote:

Show 18 quoted lines

> On Tue, Oct 07, 2025 at 02:41:12PM +0200, Patrick Steinhardt wrote:
> > ---
> >  builtin/fast-import.c  | 4 ++--
> >  builtin/pack-objects.c | 4 ++--
> >  packfile.c             | 2 +-
> >  packfile.h             | 4 ++--
> >  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> Hmm. I wonder if we should perform this step at a later date. My fear is
> that another topic in fight might introduce a new use of the "get_packs"
> assuming the old semantics.
> 
> Merging this topic and that hypothetical one together wouldn't produce a
> textual conflict, but it could introduce bugs where the hypothetical new
> code expects the old behavior.
> 
> Perhaps I'm overthinking this, but figured I'd write down the concern
> nonetheless.

I think that in many cases, a callsite that doesn't handle MIDX'd packs specifically with `get_packs()` is almost guaranteed to be wrong in some cases anyway due to `get_packs()` and `get_all_packs()` influencing each other's results. In this series we already saw that the callsites handle this correctly even though they use `get_packs()`, and any new callsites would probably have to do the same.

So I don't think this is too worrisome overall.

Patrick