Re: [PATCH] Make gc a builtin.

A good (second) try.

James Bowes [off-list ref] wrote:

Actually I think you want to use:

static int pack_refs = -1;

See below for why...

+static int gc_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "gc.packrefs"))
+		if (strlen(value) == 0 || !strcmp(value, "notbare"))
+			pack_refs = !is_bare_repository();
+		else
+			pack_refs = git_config_bool(var, value);
+	else
+		return git_default_config(var, value);
+	return 0;
+}

Gaaah.  How about some curly braces around the then part of that
first if?

Actually, we typically just write this more like:

static int gc_config(const char *var, const char *value)
{
	if (!strcmp(var, "gc.packrefs")) {
		if (!strcmp(value, "notbare"))
			pack_refs = -1;
		else
			pack_refs = git_config_bool(var, value);
	}
	return git_default_config(var, value);
}

+int cmd_gc(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	int prune = 0;
+
+	git_config(gc_config);

if (pack_refs < 0)
	pack_refs = !is_bare_repository();

The is_bare_repository function guesses until the configuration
is done parsing; once the configuration has been parsed it has a
definate answer one way or the other.  So what I'm suggesting you
do here is set pack_refs = -1 to mean use the is_bare_repository
setting, otherwise it stays what it was set to.

+    if (pack_refs)
+	    if (run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
+            goto failure;

....

+    if (prune)
+        if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
+            goto failure;

Gaah.  Tabs-vs-spaces, not to mention that these aren't even lining
up the same way.  I too prefer what Dsco suggested already:

	if (prune && run_command_v_opt(argv_prune, RUN_GIT_CMD))
		return error("failed to run %s", argv_prune[0]);

-- 
Shawn.