Subject: Re: [PATCH v4 00/15] packfile: carve out a new packfile store

From: Taylor Blau <[email-masked]>
Date: Thu, 11 Sep 2025 19:42:16 -0400

On Thu, Sep 11, 2025 at 07:40:01PM -0400, Taylor Blau wrote:
> On Tue, Sep 09, 2025 at 01:02:59PM +0200, Patrick Steinhardt wrote:
> > Changes in v4:
> >   - Small code style improvement as suggested by Junio.
> >   - Some commit message improvements as suggested by Karthik.
> >   - Link to v3: https://lore.kernel.org/r/[email-masked]
>
> Thanks for these changes. I think that this series is getting closer,
> but I do not think that it is quite ready yet.
>
> The largest outstanding topic is that this round claims that
> get_all_packs() behaves identically to get_packed_git(), but this is not
> the case. I mentioned in a response to the patch that removes the latter
> function, but I think we should:
>
>  - Permit both get_all_packs() and get_packed_git() to coexist for now.
>
>  - In a follow-up series, transition get_packed_git() callers one-by-one
>    to use get_all_packs() instead. Each of these commits should include
>    IMHO a justification that the change is safe as-is, or include fixes
>    to make it safe. I think any "fixes" here are limited to "if
>    (p->multi_pack_index) continue;".
>
>  - Once there are no longer any callers of get_packed_git(), we can
>    remove it.
>
> If you want to pursue that in this series, I am happy to review and
> discuss it, but IMHO there is already enough going on here that I think
> it makes more sense to do that in a separate follow-up.

Oops, hit "send" too early.

The other outstanding topic that I want to raise is that I think
transitioning away from get_all_packs() to the new API in all callers is
premature. I would *much* rather see us do this call-by-call when it is
necessary to do so rather than forcing all callers onto the new API.

If there is a compelling reason that we must force callers to all use
the new API now, I'm happy to discuss that, but as-is I worry that we
are changing things too quickly here.

Thanks,
Taylor