git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: git-clone --config order & fetching extra refs during initial clone

From
Jeff King <peff@peff.net>
Date
May 9, 2017, 02:10 UTC
Message-ID
<20170509021028.fr5mc76kcbpnn4zs@sigill.intra.peff.net>
In-Reply-To
<xmqq4lwu7r0s.fsf@gitster.mtv.corp.google.com>
On Tue, May 09, 2017 at 10:33:39AM +0900, Junio C Hamano wrote:
Show 16 quoted lines
> >> 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
Previous: Junio C HamanoNext: Jeff King
Message 12 of 15 in “git-clone --config order & fetching extra refs during initial clone”
  1. Robin H. JohnsonFeb 25, 2017
  2. Jeff KingFeb 25, 2017
  3. Jeff KingFeb 25, 2017
  4. Junio C HamanoFeb 27, 2017
  5. Jeff KingFeb 27, 2017
  6. SZEDER GáborMar 11, 2017
  7. Jeff KingMar 15, 2017
  8. SZEDER GáborMay 3, 2017
  9. Jeff KingMay 3, 2017
  10. Sebastian SchuberthMay 4, 2017
  11. Junio C HamanoMay 9, 2017
  12. Jeff KingMay 9, 2017
  13. Jeff KingMay 9, 2017
  14. Junio C HamanoMay 9, 2017
  15. Ævar Arnfjörð BjarmasonMay 4, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.