On Tue, May 09, 2017 at 10:33:39AM +0900, Junio C Hamano wrote:

quoted

quoted

My patch deals with 'remote.<name>.refspec', i.e. 'remote->fetch'.
Apparently some extra care is necessary for 'remote.<name>.tagOpt' and
'remote->fetch_tags', too.  Perhaps there are more, I haven't checked
again, and maybe we'll add similar config variables in the future.  So
I don't think that dealing with such config variables one by one in
'git clone', too, is the right long-term solution...  but perhaps it's
sufficient for the time being?

I think your patch is a strict improvement and we don't need to hold up
waiting for a perfect fix (and because of the --single-branch thing you
mentioned, this may be the best we can do anyway).

OK, so where does this patch stand now?  It already is too late for
the upcoming release, but should we merge it to 'next' once the
release is made, cook it in 'next' and shoot for the next release
as-is, or do we want to allow minor tweaks before it hits 'next'?

I'd be OK with it as-is, but here are my nitpicks as a diff (keep the
assignment of refspec_count in one place, and free fetch_patterns as
soon as it is no longer used).

I actually think it might be nice to pull the whole thing out into its
own function, but the parameters it takes would be oddly specific.

diff --git a/builtin/clone.c b/builtin/clone.c
index 0630202c8..577529a11 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c

@@ -861,7 +861,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	int err = 0, complete_refs_before_fetch = 1;
 
 	struct refspec *refspec;
-	unsigned int refspec_count = 1;
+	unsigned int refspec_count;
 	const char **fetch_patterns;
 	const struct string_list *config_fetch_patterns;
 

@@ -994,6 +994,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	config_fetch_patterns = git_config_get_value_multi(key.buf);
 	if (config_fetch_patterns)
 		refspec_count = 1 + config_fetch_patterns->nr;
+	else
+		refspec_count = 1;
 	fetch_patterns = xcalloc(refspec_count, sizeof(*fetch_patterns));
 	fetch_patterns[0] = value.buf;
 	if (config_fetch_patterns) {

@@ -1003,6 +1005,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			fetch_patterns[i++] = fp->string;
 	}
 	refspec = parse_fetch_refspec(refspec_count, fetch_patterns);
+	free(fetch_patterns);
 
 	strbuf_reset(&key);
 	strbuf_reset(&value);

@@ -1129,7 +1132,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	strbuf_release(&value);
 	junk_mode = JUNK_LEAVE_ALL;
 
-	free(fetch_patterns);
 	free(refspec);
 	return err;
 }

-Peff