Re: [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Wed, 10 Oct 2007, Brandon Casey wrote:

>  static int need_to_gc(void)
>  {
> -	int ac = 0;
> -
>  	/*
>  	 * Setting gc.auto and gc.autopacklimit to 0 or negative can
>  	 * disable the automatic gc.
> @@ -158,14 +156,10 @@ static int need_to_gc(void)
>  	 * 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";
> +		append_option(argv_repack, "-A", MAX_ADD);
>  	else if (!too_many_loose_objects())
>  		return 0;
> -	argv_repack[ac++] = "-d";
> -	argv_repack[ac++] = "-l";
> -	argv_repack[ac++] = NULL;
>  	return 1;
>  }
>  

This is a nice cleanup, but it does not belong in this patch.

> @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  		prune = 0;
>  		if (!need_to_gc())
>  			return 0;
> +	} else {
> +		/*
> +		 * Use safer (for shared repos) "-A" option to
> +		 * repack when not pruning. Auto-gc makes its
> +                 * own decision.
> +		 */
> +		if (prune)
> +			append_option(argv_repack, "-a", MAX_ADD);
> +		else
> +			append_option(argv_repack, "-A", MAX_ADD);
>  	}
>  
>  	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))

To use the append_option() function, you need to make sure that the argv 
has enough space, so I suggest adding another NULL to argv_repack[].  This 
is enough, since you only ever add one option.  But you should mention it 
somewhere, probably above the definition of argv_repack[].

Other than that: I think your patch makes tons of sense; much better than 
what I tried to cobble together with the cyclic alternates detection.

Ciao,
Dscho