Re: [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Oct 11, 2007, 00:12 UTC
- Message-ID
- <Pine.LNX.4.64.0710110106160.4174@racer.site>
- In-Reply-To
- <470D6509.5020607@nrlssc.navy.mil>
Hi,
On Wed, 10 Oct 2007, Brandon Casey wrote:
Show 23 quoted lines
> 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.
Show 17 quoted lines
> @@ -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