Re: [PATCH 3/7] pack-revindex: make `load_pack_revindex` take a repository

[Derrick Stolee <derrickstolee@github.com>]

On 4/11/2023 5:30 PM, Taylor Blau wrote:
> On Tue, Apr 11, 2023 at 09:45:21AM -0400, Derrick Stolee wrote:

>> @@ -581,7 +580,7 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r)
>>  {
>>  	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
>>
>> -	if (!open_bitmap(r, bitmap_git) && !load_bitmap(bitmap_git))
>> +	if (!open_bitmap(r, bitmap_git) && !load_bitmap(r, bitmap_git))
>>  		return bitmap_git;
>>
>>  	free_bitmap_index(bitmap_git);
> 
> Oops; we are indeed dropping the repository pointer that was given to
> prepare_bitmap_git() here. It's unfortunate that we have to work through
> so many layers to propagate it back down, but I agree that it's the
> right thing to do.
> 
>> @@ -590,9 +589,10 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r)
>>
>>  struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
>>  {
>> +	struct repository *r = the_repository;
> 
> OK; and here we're using the trick you mentioned in the patch message to
> avoid having to propagate this even further out. The rest of the patch
> looks sensible to me.
> 
> In terms of working this one in, it feels odd to include it as a
> separate commit since we know the one immediately prior to it is kind of
> broken.
> 
> Do you want to squash these together? Something else? Anything is fine
> with me here.

Feel free to squash it in, to avoid having a commit where the chain is
broken.

Thanks,
-Stolee