From 3e42cb3b67bef525d4bd8416d44ac1f1a1f7a495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Wed, 14 Nov 2018 11:46:18 +0100 Subject: clone: use a more appropriate variable name for the default refspec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd_clone() declares two strbufs 'key' and 'value' on the same line, suggesting that they are used to contruct a config variable's name and value. However, this is not the case: 'key' is used to construct the names of multiple config variables, while 'value' is never used as a value for any of those config variables, or for any other config variable for that matter, but only to contruct the default fetch refspec. Let's rename 'value' to 'default_refspec' to make the intent clearer. Signed-off-by: SZEDER Gábor Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clone.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index fd2c3ef090..290ddc2e5a 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -889,7 +889,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix) const struct ref *our_head_points_at; struct ref *mapped_refs; const struct ref *ref; - struct strbuf key = STRBUF_INIT, value = STRBUF_INIT; + struct strbuf key = STRBUF_INIT; + struct strbuf default_refspec = STRBUF_INIT; struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT; struct transport *transport = NULL; const char *src_ref_prefix = "refs/heads/"; @@ -1066,7 +1067,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin); } - strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf); strbuf_addf(&key, "remote.%s.url", option_origin); git_config_set(key.buf, repo); strbuf_reset(&key); @@ -1080,9 +1080,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); - refspec_append(&rs, value.buf); - - strbuf_reset(&value); + strbuf_addf(&default_refspec, "+%s*:%s*", src_ref_prefix, + branch_top.buf); + refspec_append(&rs, default_refspec.buf); remote = remote_get(option_origin); transport = transport_get(remote, remote->url[0]); @@ -1239,7 +1239,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) strbuf_release(&reflog_msg); strbuf_release(&branch_top); strbuf_release(&key); - strbuf_release(&value); + strbuf_release(&default_refspec); junk_mode = JUNK_LEAVE_ALL; refspec_clear(&rs); -- cgit v1.2.3 From 515be8338237c1f0f98ff826ba98f50d1dff5517 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Wed, 14 Nov 2018 11:46:19 +0100 Subject: clone: respect additional configured fetch refspecs during initial fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial fetch during a clone doesn't transfer refs matching additional fetch refspecs given on the command line as configuration variables, e.g. '-c remote.origin.fetch='. This contradicts the documentation stating that configuration variables specified via 'git clone -c = ...' "take effect immediately after the repository is initialized, but before the remote history is fetched" and the given example specifically mentions "adding additional fetch refspecs to the origin remote". Furthermore, one-shot configuration variables specified via 'git -c = clone ...', though not written to the newly created repository's config file, live during the lifetime of the 'clone' command, including the initial fetch. All this implies that any fetch refspecs specified this way should already be taken into account during the initial fetch. The reason for this is that the initial fetch is not a fully fledged 'git fetch' but a bunch of direct calls into the fetch/transport machinery with clone's own refs-to-refspec matching logic, which bypasses parts of 'git fetch' processing configured fetch refspecs. This logic only considers a single default refspec, potentially influenced by options like '--single-branch' and '--mirror'. The configured refspecs are, however, already read and parsed properly when clone calls remote.c:remote_get(), but it never looks at the parsed refspecs in the resulting 'struct remote'. Modify clone to take the remote's configured fetch refspecs into account to retrieve all matching refs during the initial fetch. Note that we have to explicitly add the default fetch refspec to the remote's refspecs, because at that point the remote only includes the fetch refspecs specified on the command line. Add tests to check that refspecs given both via 'git clone -c ...' and 'git -c ... clone' retrieve all refs matching either the default or the additional refspecs, and that it works even when the user specifies an alternative remote name via '--origin='. Signed-off-by: SZEDER Gábor Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clone.c | 25 +++++++++++++++---------- t/t5611-clone-config.sh | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 290ddc2e5a..c9c840eaf8 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -548,7 +548,7 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch } static struct ref *wanted_peer_refs(const struct ref *refs, - struct refspec_item *refspec) + struct refspec *refspec) { struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD")); struct ref *local_refs = head; @@ -569,13 +569,19 @@ static struct ref *wanted_peer_refs(const struct ref *refs, warning(_("Could not find remote branch %s to clone."), option_branch); else { - get_fetch_map(remote_head, refspec, &tail, 0); + int i; + for (i = 0; i < refspec->nr; i++) + get_fetch_map(remote_head, &refspec->items[i], + &tail, 0); /* if --branch=tag, pull the requested tag explicitly */ get_fetch_map(remote_head, tag_refspec, &tail, 0); } - } else - get_fetch_map(refs, refspec, &tail, 0); + } else { + int i; + for (i = 0; i < refspec->nr; i++) + get_fetch_map(refs, &refspec->items[i], &tail, 0); + } if (!option_mirror && !option_single_branch && !option_no_tags) get_fetch_map(refs, tag_refspec, &tail, 0); @@ -898,7 +904,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) int err = 0, complete_refs_before_fetch = 1; int submodule_progress; - struct refspec rs = REFSPEC_INIT_FETCH; struct argv_array ref_prefixes = ARGV_ARRAY_INIT; fetch_if_missing = 0; @@ -1080,11 +1085,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); + remote = remote_get(option_origin); + strbuf_addf(&default_refspec, "+%s*:%s*", src_ref_prefix, branch_top.buf); - refspec_append(&rs, default_refspec.buf); + refspec_append(&remote->fetch, default_refspec.buf); - remote = remote_get(option_origin); transport = transport_get(remote, remote->url[0]); transport_set_verbosity(transport, option_verbosity, option_progress); transport->family = family; @@ -1139,7 +1145,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) argv_array_push(&ref_prefixes, "HEAD"); - refspec_ref_prefixes(&rs, &ref_prefixes); + refspec_ref_prefixes(&remote->fetch, &ref_prefixes); if (option_branch) expand_ref_prefix(&ref_prefixes, option_branch); if (!option_no_tags) @@ -1148,7 +1154,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) refs = transport_get_remote_refs(transport, &ref_prefixes); if (refs) { - mapped_refs = wanted_peer_refs(refs, &rs.items[0]); + mapped_refs = wanted_peer_refs(refs, &remote->fetch); /* * transport_get_remote_refs() may return refs with null sha-1 * in mapped_refs (see struct transport->get_refs_list @@ -1242,7 +1248,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) strbuf_release(&default_refspec); junk_mode = JUNK_LEAVE_ALL; - refspec_clear(&rs); argv_array_clear(&ref_prefixes); return err; } diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh index 39329eb7a8..60c1ba951b 100755 --- a/t/t5611-clone-config.sh +++ b/t/t5611-clone-config.sh @@ -45,6 +45,53 @@ test_expect_success 'clone -c config is available during clone' ' test_cmp expect child/file ' +test_expect_success 'clone -c remote.origin.fetch= works' ' + rm -rf child && + git update-ref refs/grab/it refs/heads/master && + git update-ref refs/leave/out refs/heads/master && + git clone -c "remote.origin.fetch=+refs/grab/*:refs/grab/*" . child && + git -C child for-each-ref --format="%(refname)" >actual && + + cat >expect <<-\EOF && + refs/grab/it + refs/heads/master + refs/remotes/origin/HEAD + refs/remotes/origin/master + EOF + test_cmp expect actual +' + +test_expect_success 'git -c remote.origin.fetch= clone works' ' + rm -rf child && + git -c "remote.origin.fetch=+refs/grab/*:refs/grab/*" clone . child && + git -C child for-each-ref --format="%(refname)" >actual && + + cat >expect <<-\EOF && + refs/grab/it + refs/heads/master + refs/remotes/origin/HEAD + refs/remotes/origin/master + EOF + test_cmp expect actual +' + +test_expect_success 'clone -c remote..fetch= --origin=' ' + rm -rf child && + git clone --origin=upstream \ + -c "remote.upstream.fetch=+refs/grab/*:refs/grab/*" \ + -c "remote.origin.fetch=+refs/leave/*:refs/leave/*" \ + . child && + git -C child for-each-ref --format="%(refname)" >actual && + + cat >expect <<-\EOF && + refs/grab/it + refs/heads/master + refs/remotes/upstream/HEAD + refs/remotes/upstream/master + EOF + test_cmp expect actual +' + # Tests for the hidden file attribute on windows is_hidden () { # Use the output of `attrib`, ignore the absolute path -- cgit v1.2.3 From 7eae4a3ac4084e6f01bbab2847a399ea5677a099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Wed, 14 Nov 2018 11:46:20 +0100 Subject: Documentation/clone: document ignored configuration variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to limitations in the current implementation, some configuration variables specified via 'git clone -c var=val' (or 'git -c var=val clone') are ignored during the initial fetch and checkout. Let the users know which configuration variables are known to be ignored ('remote.origin.mirror' and 'remote.origin.tagOpt') under the documentation of 'git clone -c', along with hints to use the options '--mirror' and '--no-tags' instead. Signed-off-by: SZEDER Gábor Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-clone.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index a55536f0bf..2fd12524f9 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -189,6 +189,12 @@ objects from the source repository into a pack in the cloned repository. values are given for the same key, each value will be written to the config file. This makes it safe, for example, to add additional fetch refspecs to the origin remote. ++ +Due to limitations of the current implementation, some configuration +variables do not take effect until after the initial fetch and checkout. +Configuration variables known to not take effect are: +`remote..mirror` and `remote..tagOpt`. Use the +corresponding `--mirror` and `--no-tags` options instead. --depth :: Create a 'shallow' clone with a history truncated to the -- cgit v1.2.3