Re: [PATCH 08/17] pack-objects: add --path-walk option

From: Derrick Stolee <stolee@gmail.com>
Date: Oct 31, 2024, 02:04 UTC
Message-ID: <eff447c5-fa0b-446b-989e-b3e34a685811@gmail.com>
In-Reply-To: <ZyFet+Gamc8g7Hg6@nand.local>

On 10/29/24 6:16 PM, Taylor Blau wrote:

Show 11 quoted lines

> On Tue, Oct 29, 2024 at 02:36:57PM -0700, Jonathan Tan wrote:
>> By incompatibility, do you mean the incompatibility between bitmaps
>> and the overall --path-walk feature as implemented collectively by the
>> patches in Stolee's patch set? If so, I suspect that we will need a
>> parallel code path that takes in the "want" and "uninteresting" commits
>> and emits the list of objects (possibly before sorting the objects by
>> path hash), much like in builtin/pack-objects.c, so I think there will
>> be some effort involved in making the two work together.
> 
> I am not sure yet, in all honesty, because I haven't had enough time to
> spend yet reviewing these patches to have anything intelligent to say.

I think that the --path-walk option is fundamentally incompatible with the --use-bitmap-index option (using reachability bitmaps to reduce how many objects we parse to discover reachability) but is not necessarily incompatible with writing bitmaps. But it would require testing to be sure that there are no surprises due to something like an object order changing or something like that.

The feature is also not currently integrated with delta islands, so that would need integration and testing to make sure things are grouped together and delta chains go in the right direction.

These things may be something possible to overcome in the future, but the lack of current integration points and testing makes me want to leave this version with guard rails that prevent users from getting into a bind.

Thanks, -Stolee