Re: [PATCH] midx: apply gitconfig to midx repack

From: Son Luong Ngoc <sluongng@gmail.com>
Date: May 6, 2020, 08:56 UTC
Message-ID: <E80AD11E-4151-4B6D-988C-B91D8A93F6B6@gmail.com>
In-Reply-To: <74A7FE73-6B5F-4DCF-9A57-AD11306CFAF8@gmail.com>

Hi,

Show 24 quoted lines

> On May 5, 2020, at 18:03, Son Luong Ngoc <sluongng@gmail.com> wrote:
>> On May 5, 2020, at 15:50, Derrick Stolee <stolee@gmail.com> wrote:
>>> +	if (write_bitmaps > 0)
>>> +		argv_array_push(&cmd.args, "--write-bitmap-index");
>>> +	else if (write_bitmaps < 0)
>>> +		argv_array_push(&cmd.args, "--write-bitmap-index-quiet");
>> 
>> These make less sense. Unless --batch-size=0 and there are no .keep
>> packs (with the patch below) I'm not sure we _can_ write bitmap indexes
>> here. The pack-file is not necessarily closed under reachability. Or,
>> will supplying these arguments to 'git pack-objects' actually do that
>> closure?
>> 
>> I would be happy to special-case these options to the "--batch-size=0"
>> situation and otherwise ignore them. This then gets into enough
>> complication that we should update the documentation as in the patch
>> below.
> 
> You make a great point here. 
> I completely missed this as I have been largely testing with repacking only 2 packs,
> effectively with --batch-size=0.
> 
> I think having the bitmaps index is highly desirable in `--batch-size=0` case.
> I will try to include that in V2 (with Documentation).

Hmm, I just realized that there is a check for `--all` is being passed on pack-objects side.

	if (batch_size == 0) {
		argv_array_push(&cmd.args, "--all");
		if (write_bitmaps > 0)
			argv_array_push(&cmd.args, "--write-bitmap-index");
		else if (write_bitmaps < 0)
			argv_array_push(&cmd.args, "--write-bitmap-index-quiet");
	}

If I do something like this, the midx repack will become tremendously slow as I think pack-objects needs to scan for all revs (fed from midx) and all refs. Perhaps special exception needed to be made on pack-objects side to trust that midx is feeding it everything there is?

I think adding `write_bitmaps` support would be a bit out of my hand for now, so I will settle with the delta configs and Derrick's patch for V2. (sending it later today)

Show 6 quoted lines

>> Thanks,
>> -Stolee
> 
> Cheers,
> Son Luong
>