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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2007, 19:54 UTC
Message-ID: <7vlkb5yso5.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <Pine.LNX.4.64.0709171044170.28586@racer.site>

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

Show 13 quoted lines

>> 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.

Show 12 quoted lines

>> +		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".

Show 17 quoted lines

>> +	/*
>> +	 * 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.