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

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 11, 2025, 23:40 UTC
Message-ID: <aMNd0RkdaW4cWMtw@nand.local>
In-Reply-To: <20250909-b4-pks-packfiles-store-v4-0-151c4ba3619f@pks.im>

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/20250902-b4-pks-packfiles-store-v3-0-6925278efeda@pks.im

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.

Thanks, Taylor