{"thread":{"id":"49834","subject":"[PATCH 0/3] clone: respect configured fetch respecs during initial fetch","startedAt":"2018-11-14T10:46:35Z","lastAt":"2018-11-16T04:14:59Z","messageCount":9,"participants":["SZEDER Gábor","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"363345","messageId":"20181114104620.32478-1-szeder.dev@gmail.com","threadId":"49834","inReplyTo":null,"subject":"[PATCH 0/3] clone: respect configured fetch respecs during initial fetch","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-14T10:46:17Z","receivedAt":"2018-11-14T10:46:35Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nThe initial fetch during a clone doesn't transfer refs matching\nadditional fetch refspecs given on the command line as configuration\nvariables, e.g. '-c remote.origin.fetch='. This contradicts\nthe documentation stating that configuration variables specified via\n'git clone -c = ...' \"take effect immediately after the\nrepository is initialized, but before the remote history is fetched\"\nand the given example specifically mentions \"adding additional fetch\nrefspecs to the origin remote\".\n\nThe second patch in this series makes it work. The other two are\nwhile-at-its: the first is a little cleanup, and the last one\ndocuments other known ignored configuration variables (but whose\nfunctionality is available through command line options).\n\n\nThis patch series should have been marked as v6, but I chose to reset\nthe counter, because:\n\n - v5 was sent out way over a year ago [1], and surely everybody has\n forgotten about it since then anyway. But more importantly:\n\n - A lot has happened since then, most notably we now have a refspec\n API, which makes this patch series much simpler (now it only\n touches 'builtin/clone.c', the previous version had to add stuff\n to 'remote.{c,h}' as well).\n\n\n[1] For reference, though I actually doubt it's worth looking up:\n https://public-inbox.org/git/20170616173849.8071-1-szeder.dev@gmail.com/T/#u\n\n\nSZEDER Gábor (3):\n clone: use a more appropriate variable name for the default refspec\n clone: respect additional configured fetch refspecs during initial\n fetch\n Documentation/clone: document ignored configuration variables\n\n Documentation/git-clone.txt | 6 +++++\n builtin/clone.c | 33 +++++++++++++++-----------\n t/t5611-clone-config.sh | 47 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 14 deletions(-)\n\n-- \n2.19.1.1182.gbfcc7ed3e6\n\n"},{"id":"363346","messageId":"20181114104620.32478-2-szeder.dev@gmail.com","threadId":"49834","inReplyTo":"20181114104620.32478-1-szeder.dev@gmail.com","subject":"[PATCH 1/3] clone: use a more appropriate variable name for the default refspec","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-14T10:46:18Z","receivedAt":"2018-11-14T10:46:37Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"cmd_clone() declares two strbufs 'key' and 'value' on the same line,\nsuggesting that they are used to contruct a config variable's name and\nvalue. However, this is not the case: 'key' is used to construct the\nnames of multiple config variables, while 'value' is never used as a\nvalue for any of those config variables, or for any other config\nvariable for that matter, but only to contruct the default fetch\nrefspec.\n\nLet's rename 'value' to 'default_refspec' to make the intent clearer.\n\nSigned-off-by: SZEDER Gábor \n---\n builtin/clone.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 15b142d646..ae1bf242c6 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -890,7 +890,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tconst struct ref *our_head_points_at;\n \tstruct ref *mapped_refs;\n \tconst struct ref *ref;\n-\tstruct strbuf key = STRBUF_INIT, value = STRBUF_INIT;\n+\tstruct strbuf key = STRBUF_INIT;\n+\tstruct strbuf default_refspec = STRBUF_INIT;\n \tstruct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;\n \tstruct transport *transport = NULL;\n \tconst char *src_ref_prefix = \"refs/heads/\";\n@@ -1067,7 +1068,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_addf(&branch_top, \"refs/remotes/%s/\", option_origin);\n \t}\n \n-\tstrbuf_addf(&value, \"+%s*:%s*\", src_ref_prefix, branch_top.buf);\n \tstrbuf_addf(&key, \"remote.%s.url\", option_origin);\n \tgit_config_set(key.buf, repo);\n \tstrbuf_reset(&key);\n@@ -1081,9 +1081,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_required_reference.nr || option_optional_reference.nr)\n \t\tsetup_reference();\n \n-\trefspec_append(&rs, value.buf);\n-\n-\tstrbuf_reset(&value);\n+\tstrbuf_addf(&default_refspec, \"+%s*:%s*\", src_ref_prefix,\n+\t\t branch_top.buf);\n+\trefspec_append(&rs, default_refspec.buf);\n \n \tremote = remote_get(option_origin);\n \ttransport = transport_get(remote, remote->url[0]);\n@@ -1240,7 +1240,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&reflog_msg);\n \tstrbuf_release(&branch_top);\n \tstrbuf_release(&key);\n-\tstrbuf_release(&value);\n+\tstrbuf_release(&default_refspec);\n \tjunk_mode = JUNK_LEAVE_ALL;\n \n \trefspec_clear(&rs);\n-- \n2.19.1.1182.gbfcc7ed3e6\n\n"},{"id":"363347","messageId":"20181114104620.32478-3-szeder.dev@gmail.com","threadId":"49834","inReplyTo":"20181114104620.32478-1-szeder.dev@gmail.com","subject":"[PATCH 2/3] clone: respect additional configured fetch refspecs during initial fetch","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-14T10:46:19Z","receivedAt":"2018-11-14T10:46:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The initial fetch during a clone doesn't transfer refs matching\nadditional fetch refspecs given on the command line as configuration\nvariables, e.g. '-c remote.origin.fetch='. This contradicts\nthe documentation stating that configuration variables specified via\n'git clone -c = ...' \"take effect immediately after the\nrepository is initialized, but before the remote history is fetched\"\nand the given example specifically mentions \"adding additional fetch\nrefspecs to the origin remote\". Furthermore, one-shot configuration\nvariables specified via 'git -c = clone ...', though not\nwritten to the newly created repository's config file, live during the\nlifetime of the 'clone' command, including the initial fetch. All\nthis implies that any fetch refspecs specified this way should already\nbe taken into account during the initial fetch.\n\nThe reason for this is that the initial fetch is not a fully fledged\n'git fetch' but a bunch of direct calls into the fetch/transport\nmachinery with clone's own refs-to-refspec matching logic, which\nbypasses parts of 'git fetch' processing configured fetch refspecs.\nThis logic only considers a single default refspec, potentially\ninfluenced by options like '--single-branch' and '--mirror'. The\nconfigured refspecs are, however, already read and parsed properly\nwhen clone calls remote.c:remote_get(), but it never looks at the\nparsed refspecs in the resulting 'struct remote'.\n\nModify clone to take the remote's configured fetch refspecs into\naccount to retrieve all matching refs during the initial fetch. Note\nthat we have to explicitly add the default fetch refspec to the\nremote's refspecs, because at that point the remote only includes the\nfetch refspecs specified on the command line.\n\nAdd tests to check that refspecs given both via 'git clone -c ...' and\n'git -c ... clone' retrieve all refs matching either the default or\nthe additional refspecs, and that it works even when the user\nspecifies an alternative remote name via '--origin='.\n\nSigned-off-by: SZEDER Gábor \n---\n builtin/clone.c | 25 +++++++++++++---------\n t/t5611-clone-config.sh | 47 +++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex ae1bf242c6..7c7f98c72c 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -548,7 +548,7 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch\n }\n \n static struct ref *wanted_peer_refs(const struct ref *refs,\n-\t\tstruct refspec_item *refspec)\n+\t\tstruct refspec *refspec)\n {\n \tstruct ref *head = copy_ref(find_ref_by_name(refs, \"HEAD\"));\n \tstruct ref *local_refs = head;\n@@ -569,13 +569,19 @@ static struct ref *wanted_peer_refs(const struct ref *refs,\n \t\t\twarning(_(\"Could not find remote branch %s to clone.\"),\n \t\t\t\toption_branch);\n \t\telse {\n-\t\t\tget_fetch_map(remote_head, refspec, &tail, 0);\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < refspec->nr; i++)\n+\t\t\t\tget_fetch_map(remote_head, &refspec->items[i],\n+\t\t\t\t\t &tail, 0);\n \n \t\t\t/* if --branch=tag, pull the requested tag explicitly */\n \t\t\tget_fetch_map(remote_head, tag_refspec, &tail, 0);\n \t\t}\n-\t} else\n-\t\tget_fetch_map(refs, refspec, &tail, 0);\n+\t} else {\n+\t\tint i;\n+\t\tfor (i = 0; i < refspec->nr; i++)\n+\t\t\tget_fetch_map(refs, &refspec->items[i], &tail, 0);\n+\t}\n \n \tif (!option_mirror && !option_single_branch && !option_no_tags)\n \t\tget_fetch_map(refs, tag_refspec, &tail, 0);\n@@ -899,7 +905,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tint err = 0, complete_refs_before_fetch = 1;\n \tint submodule_progress;\n \n-\tstruct refspec rs = REFSPEC_INIT_FETCH;\n \tstruct argv_array ref_prefixes = ARGV_ARRAY_INIT;\n \n \tfetch_if_missing = 0;\n@@ -1081,11 +1086,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_required_reference.nr || option_optional_reference.nr)\n \t\tsetup_reference();\n \n+\tremote = remote_get(option_origin);\n+\n \tstrbuf_addf(&default_refspec, \"+%s*:%s*\", src_ref_prefix,\n \t\t branch_top.buf);\n-\trefspec_append(&rs, default_refspec.buf);\n+\trefspec_append(&remote->fetch, default_refspec.buf);\n \n-\tremote = remote_get(option_origin);\n \ttransport = transport_get(remote, remote->url[0]);\n \ttransport_set_verbosity(transport, option_verbosity, option_progress);\n \ttransport->family = family;\n@@ -1140,7 +1146,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \n \targv_array_push(&ref_prefixes, \"HEAD\");\n-\trefspec_ref_prefixes(&rs, &ref_prefixes);\n+\trefspec_ref_prefixes(&remote->fetch, &ref_prefixes);\n \tif (option_branch)\n \t\texpand_ref_prefix(&ref_prefixes, option_branch);\n \tif (!option_no_tags)\n@@ -1149,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \trefs = transport_get_remote_refs(transport, &ref_prefixes);\n \n \tif (refs) {\n-\t\tmapped_refs = wanted_peer_refs(refs, &rs.items[0]);\n+\t\tmapped_refs = wanted_peer_refs(refs, &remote->fetch);\n \t\t/*\n \t\t * transport_get_remote_refs() may return refs with null sha-1\n \t\t * in mapped_refs (see struct transport->get_refs_list\n@@ -1243,7 +1249,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&default_refspec);\n \tjunk_mode = JUNK_LEAVE_ALL;\n \n-\trefspec_clear(&rs);\n \targv_array_clear(&ref_prefixes);\n \treturn err;\n }\ndiff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh\nindex 39329eb7a8..60c1ba951b 100755\n--- a/t/t5611-clone-config.sh\n+++ b/t/t5611-clone-config.sh\n@@ -45,6 +45,53 @@ test_expect_success 'clone -c config is available during clone' '\n \ttest_cmp expect child/file\n '\n \n+test_expect_success 'clone -c remote.origin.fetch= works' '\n+\trm -rf child &&\n+\tgit update-ref refs/grab/it refs/heads/master &&\n+\tgit update-ref refs/leave/out refs/heads/master &&\n+\tgit clone -c \"remote.origin.fetch=+refs/grab/*:refs/grab/*\" . child &&\n+\tgit -C child for-each-ref --format=\"%(refname)\" >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\trefs/grab/it\n+\trefs/heads/master\n+\trefs/remotes/origin/HEAD\n+\trefs/remotes/origin/master\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git -c remote.origin.fetch= clone works' '\n+\trm -rf child &&\n+\tgit -c \"remote.origin.fetch=+refs/grab/*:refs/grab/*\" clone . child &&\n+\tgit -C child for-each-ref --format=\"%(refname)\" >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\trefs/grab/it\n+\trefs/heads/master\n+\trefs/remotes/origin/HEAD\n+\trefs/remotes/origin/master\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'clone -c remote..fetch= --origin=' '\n+\trm -rf child &&\n+\tgit clone --origin=upstream \\\n+\t\t -c \"remote.upstream.fetch=+refs/grab/*:refs/grab/*\" \\\n+\t\t -c \"remote.origin.fetch=+refs/leave/*:refs/leave/*\" \\\n+\t\t . child &&\n+\tgit -C child for-each-ref --format=\"%(refname)\" >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\trefs/grab/it\n+\trefs/heads/master\n+\trefs/remotes/upstream/HEAD\n+\trefs/remotes/upstream/master\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n # Tests for the hidden file attribute on windows\n is_hidden () {\n \t# Use the output of `attrib`, ignore the absolute path\n-- \n2.19.1.1182.gbfcc7ed3e6\n\n"},{"id":"363348","messageId":"20181114104620.32478-4-szeder.dev@gmail.com","threadId":"49834","inReplyTo":"20181114104620.32478-1-szeder.dev@gmail.com","subject":"[PATCH 3/3] Documentation/clone: document ignored configuration variables","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-14T10:46:20Z","receivedAt":"2018-11-14T10:46:39Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Due to limitations in the current implementation, some configuration\nvariables specified via 'git clone -c var=val' (or 'git -c var=val\nclone') are ignored during the initial fetch and checkout.\n\nLet the users know which configuration variables are known to be\nignored ('remote.origin.mirror' and 'remote.origin.tagOpt') under the\ndocumentation of 'git clone -c', along with hints to use the options\n'--mirror' and '--no-tags' instead.\n\nSigned-off-by: SZEDER Gábor \n---\n Documentation/git-clone.txt | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex a55536f0bf..2fd12524f9 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -189,6 +189,12 @@ objects from the source repository into a pack in the cloned repository.\n \tvalues are given for the same key, each value will be written to\n \tthe config file. This makes it safe, for example, to add\n \tadditional fetch refspecs to the origin remote.\n++\n+Due to limitations of the current implementation, some configuration\n+variables do not take effect until after the initial fetch and checkout.\n+Configuration variables known to not take effect are:\n+`remote..mirror` and `remote..tagOpt`. Use the\n+corresponding `--mirror` and `--no-tags` options instead.\n \n --depth ::\n \tCreate a 'shallow' clone with a history truncated to the\n-- \n2.19.1.1182.gbfcc7ed3e6\n\n"},{"id":"363420","messageId":"20181115105427.GA19032@sigill.intra.peff.net","threadId":"49834","inReplyTo":"20181114104620.32478-2-szeder.dev@gmail.com","subject":"Re: [PATCH 1/3] clone: use a more appropriate variable name for the default refspec","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T10:54:27Z","receivedAt":"2018-11-15T10:54:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2018 at 11:46:18AM +0100, SZEDER Gábor wrote:\n\n> cmd_clone() declares two strbufs 'key' and 'value' on the same line,\n> suggesting that they are used to contruct a config variable's name and\n> value. However, this is not the case: 'key' is used to construct the\n> names of multiple config variables, while 'value' is never used as a\n> value for any of those config variables, or for any other config\n> variable for that matter, but only to contruct the default fetch\n> refspec.\n> \n> Let's rename 'value' to 'default_refspec' to make the intent clearer.\n\nYep, this is much nicer.\n\n-Peff\n"},{"id":"363421","messageId":"20181115110455.GB19032@sigill.intra.peff.net","threadId":"49834","inReplyTo":"20181114104620.32478-3-szeder.dev@gmail.com","subject":"Re: [PATCH 2/3] clone: respect additional configured fetch refspecs during initial fetch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T11:04:55Z","receivedAt":"2018-11-15T11:04:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2018 at 11:46:19AM +0100, SZEDER Gábor wrote:\n\n> The initial fetch during a clone doesn't transfer refs matching\n> additional fetch refspecs given on the command line as configuration\n> variables, e.g. '-c remote.origin.fetch='. This contradicts\n> the documentation stating that configuration variables specified via\n> 'git clone -c = ...' \"take effect immediately after the\n> repository is initialized, but before the remote history is fetched\"\n> and the given example specifically mentions \"adding additional fetch\n> refspecs to the origin remote\". Furthermore, one-shot configuration\n> variables specified via 'git -c = clone ...', though not\n> written to the newly created repository's config file, live during the\n> lifetime of the 'clone' command, including the initial fetch. All\n> this implies that any fetch refspecs specified this way should already\n> be taken into account during the initial fetch.\n> \n> The reason for this is that the initial fetch is not a fully fledged\n> 'git fetch' but a bunch of direct calls into the fetch/transport\n> machinery with clone's own refs-to-refspec matching logic, which\n> bypasses parts of 'git fetch' processing configured fetch refspecs.\n> This logic only considers a single default refspec, potentially\n> influenced by options like '--single-branch' and '--mirror'. The\n> configured refspecs are, however, already read and parsed properly\n> when clone calls remote.c:remote_get(), but it never looks at the\n> parsed refspecs in the resulting 'struct remote'.\n> \n> Modify clone to take the remote's configured fetch refspecs into\n> account to retrieve all matching refs during the initial fetch. Note\n> that we have to explicitly add the default fetch refspec to the\n> remote's refspecs, because at that point the remote only includes the\n> fetch refspecs specified on the command line.\n\nNicely explained.\n\n> Add tests to check that refspecs given both via 'git clone -c ...' and\n> 'git -c ... clone' retrieve all refs matching either the default or\n> the additional refspecs, and that it works even when the user\n> specifies an alternative remote name via '--origin='.\n\nGood. This is all sufficiently subtle that we definitely want to check\nthe related cases.\n\n> @@ -1081,11 +1086,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tif (option_required_reference.nr || option_optional_reference.nr)\n> \t\tsetup_reference();\n> \n> +\tremote = remote_get(option_origin);\n> +\n> \tstrbuf_addf(&default_refspec, \"+%s*:%s*\", src_ref_prefix,\n> \t\t branch_top.buf);\n> -\trefspec_append(&rs, default_refspec.buf);\n> +\trefspec_append(&remote->fetch, default_refspec.buf);\n\nWow, this is so much nicer than the older versions. :)\n\nI wondered briefly whether this ought to only kick in when the user\ndidn't specify any refspecs. I.e., there is no way with this to say\n\"don't fetch refs/heads/*, but this other thing instead\". But the way\nthe existing documentation is written, it's pretty clear that it's about\nadding to the list of refspecs. We can always something like\n\"--no-default-refspec\" later if somebody wants it.\n\n> [...]\n\nMost of the rest of the patch is just swapping out \"rs\" for\n\"remote->fetch\", which makes sense. So the implementation looks good to\nme.\n\n> diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh\n> index 39329eb7a8..60c1ba951b 100755\n> --- a/t/t5611-clone-config.sh\n> +++ b/t/t5611-clone-config.sh\n> @@ -45,6 +45,53 @@ test_expect_success 'clone -c config is available during clone' '\n> \ttest_cmp expect child/file\n> '\n> \n> +test_expect_success 'clone -c remote.origin.fetch= works' '\n> +\trm -rf child &&\n> +\tgit update-ref refs/grab/it refs/heads/master &&\n> +\tgit update-ref refs/leave/out refs/heads/master &&\n\nChecking one in and one out; nice.\n\n> +\tgit clone -c \"remote.origin.fetch=+refs/grab/*:refs/grab/*\" . child &&\n> +\tgit -C child for-each-ref --format=\"%(refname)\" >actual &&\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\trefs/grab/it\n> +\trefs/heads/master\n> +\trefs/remotes/origin/HEAD\n> +\trefs/remotes/origin/master\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nMakes sense.\n\n> +test_expect_success 'git -c remote.origin.fetch= clone works' '\n> +\trm -rf child &&\n> +\tgit -c \"remote.origin.fetch=+refs/grab/*:refs/grab/*\" clone . child &&\n> +\tgit -C child for-each-ref --format=\"%(refname)\" >actual &&\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\trefs/grab/it\n> +\trefs/heads/master\n> +\trefs/remotes/origin/HEAD\n> +\trefs/remotes/origin/master\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nThis relies on establishing grab/it in the previous test. A minor nit,\nbut we could break that more clear by breaking it out into its own\n\"set up refs for extra refspec tests\" block.\n\n> +test_expect_success 'clone -c remote..fetch= --origin=' '\n> +\trm -rf child &&\n> +\tgit clone --origin=upstream \\\n> +\t\t -c \"remote.upstream.fetch=+refs/grab/*:refs/grab/*\" \\\n> +\t\t -c \"remote.origin.fetch=+refs/leave/*:refs/leave/*\" \\\n> +\t\t . child &&\n\nNice.\n\nThe whole thing looks very cleanly done.\n\n-Peff\n"},{"id":"363422","messageId":"20181115110626.GC19032@sigill.intra.peff.net","threadId":"49834","inReplyTo":"20181114104620.32478-4-szeder.dev@gmail.com","subject":"Re: [PATCH 3/3] Documentation/clone: document ignored configuration variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T11:06:26Z","receivedAt":"2018-11-15T11:06:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2018 at 11:46:20AM +0100, SZEDER Gábor wrote:\n\n> Due to limitations in the current implementation, some configuration\n> variables specified via 'git clone -c var=val' (or 'git -c var=val\n> clone') are ignored during the initial fetch and checkout.\n> \n> Let the users know which configuration variables are known to be\n> ignored ('remote.origin.mirror' and 'remote.origin.tagOpt') under the\n> documentation of 'git clone -c', along with hints to use the options\n> '--mirror' and '--no-tags' instead.\n\nGood idea.\n\n> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> index a55536f0bf..2fd12524f9 100644\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -189,6 +189,12 @@ objects from the source repository into a pack in the cloned repository.\n> \tvalues are given for the same key, each value will be written to\n> \tthe config file. This makes it safe, for example, to add\n> \tadditional fetch refspecs to the origin remote.\n> ++\n> +Due to limitations of the current implementation, some configuration\n> +variables do not take effect until after the initial fetch and checkout.\n> +Configuration variables known to not take effect are:\n> +`remote..mirror` and `remote..tagOpt`. Use the\n> +corresponding `--mirror` and `--no-tags` options instead.\n\nThis looks good. I considered at first that this might want to go in a\nBUGS section of the manpage, but it makes the most sense being right\nnext to the definition of \"-c\".\n\n-Peff\n"},{"id":"363423","messageId":"20181115110802.GD19032@sigill.intra.peff.net","threadId":"49834","inReplyTo":"20181114104620.32478-1-szeder.dev@gmail.com","subject":"Re: [PATCH 0/3] clone: respect configured fetch respecs during initial fetch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T11:08:02Z","receivedAt":"2018-11-15T11:08:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2018 at 11:46:17AM +0100, SZEDER Gábor wrote:\n\n> This patch series should have been marked as v6, but I chose to reset\n> the counter, because:\n> \n> - v5 was sent out way over a year ago [1], and surely everybody has\n> forgotten about it since then anyway. But more importantly:\n> \n> - A lot has happened since then, most notably we now have a refspec\n> API, which makes this patch series much simpler (now it only\n> touches 'builtin/clone.c', the previous version had to add stuff\n> to 'remote.{c,h}' as well).\n\nThanks for sticking with this!\n\nI skimmed over the old discussion, mostly just to make sure there wasn't\nanything subtle that might have been forgotten. But nope, all of the\nsubtlety went away because of the refspec API you mentioned.\n\nThe whole series looks good to me.\n\n-Peff\n"},{"id":"363463","messageId":"xmqq7ehdlku9.fsf@gitster-ct.c.googlers.com","threadId":"49834","inReplyTo":"20181115110802.GD19032@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] clone: respect configured fetch respecs during initial fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-16T04:14:54Z","receivedAt":"2018-11-16T04:14:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 14, 2018 at 11:46:17AM +0100, SZEDER Gábor wrote:\n>\n>> This patch series should have been marked as v6, but I chose to reset\n>> the counter, because:\n>> \n>> - v5 was sent out way over a year ago [1], and surely everybody has\n>> forgotten about it since then anyway. But more importantly:\n>> \n>> - A lot has happened since then, most notably we now have a refspec\n>> API, which makes this patch series much simpler (now it only\n>> touches 'builtin/clone.c', the previous version had to add stuff\n>> to 'remote.{c,h}' as well).\n>\n> Thanks for sticking with this!\n>\n> I skimmed over the old discussion, mostly just to make sure there wasn't\n> anything subtle that might have been forgotten. But nope, all of the\n> subtlety went away because of the refspec API you mentioned.\n>\n> The whole series looks good to me.\n\nThanks, both.\n"}]}