On Thu, 11 Oct 2007, Johannes Schindelin wrote:
Hi, On Wed, 10 Oct 2007, Brandon Casey wrote:
<snip>
This is a nice cleanup, but it does not belong in this patch.
I was thinking the same thing right after I sent it off.
@@ -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[].
Shouldn't argv_repack have space for MAX_ADD (10) items? append_option() is already used with argv_repack when --aggressive is used.
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.
Thanks. -brandon - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
