Subject: Re: [PATCH 11/11] bundle: unbundle promisor packs

From: Derrick Stolee <[email-masked]>
Date: Mon, 7 Mar 2022 11:10:45 -0500

On 3/7/2022 10:47 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:
> 
>> From: Derrick Stolee <[email-masked]>
>>
>> In order to have a valid pack-file after unbundling a bundle that has
>> the 'filter' capability, we need to generate a .promisor file. The
>> bundle does not promise _where_ the objects can be found, but we can
>> expect that these bundles will be unbundled in repositories with
>> appropriate promisor remotes that can find those missing objects.
>>
>> Use the 'git index-pack --promisor=<message>' option to create this
>> .promisor file. Add "from-bundle" as the message to help anyone diagnose
>> issues with these promisor packs.
>>
>> Signed-off-by: Derrick Stolee <[email-masked]>
>> ---
>>  bundle.c               | 4 ++++
>>  t/t6020-bundle-misc.sh | 8 +++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/bundle.c b/bundle.c
>> index e284ef63062..3d97de40ef0 100644
>> --- a/bundle.c
>> +++ b/bundle.c
>> @@ -631,6 +631,10 @@ int unbundle(struct repository *r, struct bundle_header *header,
>>  	struct child_process ip = CHILD_PROCESS_INIT;
>>  	strvec_pushl(&ip.args, "index-pack", "--fix-thin", "--stdin", NULL);
>>  
>> +	/* If there is a filter, then we need to create the promisor pack. */
>> +	if (header->filter)
>> +		strvec_push(&ip.args, "--promisor=from-bundle");
>> +
>>  	if (extra_index_pack_args) {
>>  		strvec_pushv(&ip.args, extra_index_pack_args->v);
>>  		strvec_clear(extra_index_pack_args);
>> diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
>> index 39cfefafb65..344af34db1e 100755
>> --- a/t/t6020-bundle-misc.sh
>> +++ b/t/t6020-bundle-misc.sh
>> @@ -513,7 +513,13 @@ do
>>  		The bundle uses this filter: $filter
>>  		The bundle records a complete history.
>>  		EOF
>> -		test_cmp expect actual
>> +		test_cmp expect actual &&
>> +
>> +		# This creates the first pack-file in the
>> +		# .git/objects/pack directory. Look for a .promisor.
>> +		git bundle unbundle partial.bdl &&
>> +		ls .git/objects/pack/pack-*.promisor >promisor &&
>> +		test_line_count = 1 promisor
>>  	'
>>  done
> 
> Aside from what Junio mentioned, the preceding commit seems to be
> incomplete here. I.e. I'd expect to see this replace a case where we
> died or whatever before. What happened if we invoked "unbundle" before?

Looking closely, I think the only difference is that this patch
adds the .promisor file. I can push my expanded test to be
earlier in the series so we can verify this.

Thanks,
-Stolee