Re: [PATCH 8/8] git-gc --auto: run "repack -A -d -l" as necessary.

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> diff --git a/builtin-gc.c b/builtin-gc.c
>> index 34ce35b..a82f6be 100644
>> --- a/builtin-gc.c
>> +++ b/builtin-gc.c
>> @@ -78,6 +83,9 @@ static int too_many_loose_objects(void)
>>  	int num_loose = 0;
>>  	int needed = 0;
>>  
>> +	if (gc_auto_threshold <= 0)
>> +		return 0;
>> +
>
> Heh, patch 6/8 explicitely moved this check out of the function.

Back in 6/8 there was only one toggle to disable auto gc
and it made sense to have it there.  Now the two switches act
together, so that you can say "Don't count my loose objects, but
do check the number of packs" if you wanted to.

>> +		if (!p->pack_local)
>> +			continue;
>> +		suffix = p->pack_name + strlen(p->pack_name) - 5;
>
> I suspect that you need something like
>
> 		int len;
> 		len = strlen(p->pack_name);
> 		if (len < 5)
> 			continue;
>
> before this.

While your additional check would not hurt, I actually think it
is the other way around; the code is already overly cautious.
If it is linked to packed_git list, the file should have already
been checked for having the suffix ".pack".

>> +	/*
>> +	 * If there are too many loose objects, but not too many
>> +	 * packs, we run "repack -d -l".  If there are too many packs,
>> +	 * we run "repack -A -d -l".  Otherwise we tell the caller
>> +	 * there is no need.
>> +	 */
>>  	argv_repack[ac++] = "repack";
>> +	if (too_many_packs())
>> +		argv_repack[ac++] = "-A";
>> +	if (!too_many_loose_objects() && ac == 1)
>> +		return 0;
>
> Why not
>
> 	else if (!too_many_loose_objects())
> 		return 0;
>

Ok.