From 2004658b19bf781d446e14e3237f519e80ab0297 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 23 Oct 2013 17:50:34 +0200 Subject: t5510: use the correct tag name in test Fix an apparent copy-paste error: A few lines earlier, a tag "refs/tags/sometag" is created. Check for the (non-)existence of that tag, not "somebranch", which is otherwise never mentioned in the script. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t5510-fetch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 1f0f8e682..c5e5dfc81 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -121,7 +121,7 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br git fetch --prune --tags origin && git rev-parse origin/master && - test_must_fail git rev-parse somebranch + test_must_fail git rev-parse sometag ' test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' ' -- cgit v1.2.1 From d0d06e892a1a7bc6a1dfd76374da09f0170cae2c Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 23 Oct 2013 17:50:35 +0200 Subject: t5510: prepare test refs more straightforwardly "git fetch" was being used with contrived refspecs to create tags and remote-tracking branches in test repositories in preparation for the actual tests. This is obscure and also makes one wonder whether this is indeed just preparation or whether some side-effect of "git fetch" is being tested. So use the more straightforward commands "git tag" / "git update-ref" when preparing branches in test repositories. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t5510-fetch.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index c5e5dfc81..08d8dbb91 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -88,7 +88,7 @@ test_expect_success 'fetch --prune on its own works as expected' ' cd "$D" && git clone . prune && cd prune && - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && + git update-ref refs/remotes/origin/extrabranch master && git fetch --prune origin && test_must_fail git rev-parse origin/extrabranch @@ -98,7 +98,7 @@ test_expect_success 'fetch --prune with a branch name keeps branches' ' cd "$D" && git clone . prune-branch && cd prune-branch && - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && + git update-ref refs/remotes/origin/extrabranch master && git fetch --prune origin master && git rev-parse origin/extrabranch @@ -117,7 +117,7 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br cd "$D" && git clone . prune-tags && cd prune-tags && - git fetch origin refs/heads/master:refs/tags/sometag && + git tag sometag master && git fetch --prune --tags origin && git rev-parse origin/master && @@ -128,7 +128,7 @@ test_expect_success 'fetch --prune --tags with branch does not delete other remo cd "$D" && git clone . prune-tags-branch && cd prune-tags-branch && - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && + git update-ref refs/remotes/origin/extrabranch master && git fetch --prune --tags origin master && git rev-parse origin/extrabranch -- cgit v1.2.1 From 68a304d5fed6657e452e8c1571fe9ca93d0a272a Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 23 Oct 2013 17:50:36 +0200 Subject: t5510: check that "git fetch --prune --tags" does not prune branches "git fetch --prune --tags" is currently interpreted as follows: * "--tags" is equivalent to specifying a refspec "refs/tags/*:refs/tags/*", and supersedes any default refspecs configured via remote.$REMOTE.fetch. * "--prune" only operates on the refspecs being fetched. Therefore, "git fetch --prune --tags" prunes tags in refs/tags/* but does not fetch or prune other references. The fact that this command does not prune references outside of refs/tags/* was previously untested. So add a test that verifies the status quo. However, the status quo is surprising, so it will be changed later in this patch series. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t5510-fetch.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 08d8dbb91..8328be134 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -118,9 +118,13 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br git clone . prune-tags && cd prune-tags && git tag sometag master && + # Create what looks like a remote-tracking branch from an earlier + # fetch that has since been deleted from the remote: + git update-ref refs/remotes/origin/fake-remote master && git fetch --prune --tags origin && git rev-parse origin/master && + git rev-parse origin/fake-remote && test_must_fail git rev-parse sometag ' -- cgit v1.2.1 From 5b2515f4007ce0fe3d61197905c41eee5a3d06c1 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 23 Oct 2013 17:50:37 +0200 Subject: api-remote.txt: correct section "struct refspec" * Replace reference to function parse_ref_spec() with references to functions parse_fetch_refspec() and parse_push_refspec(). * Correct description of src and dst: they *do* include the '*' characters. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/technical/api-remote.txt | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/technical/api-remote.txt b/Documentation/technical/api-remote.txt index 4be87768f..5d245aa9d 100644 --- a/Documentation/technical/api-remote.txt +++ b/Documentation/technical/api-remote.txt @@ -58,16 +58,16 @@ default remote, given the current branch and configuration. struct refspec -------------- -A struct refspec holds the parsed interpretation of a refspec. If it -will force updates (starts with a '+'), force is true. If it is a -pattern (sides end with '*') pattern is true. src and dest are the two -sides (if a pattern, only the part outside of the wildcards); if there -is only one side, it is src, and dst is NULL; if sides exist but are -empty (i.e., the refspec either starts or ends with ':'), the -corresponding side is "". - -This parsing can be done to an array of strings to give an array of -struct refpsecs with parse_ref_spec(). +A struct refspec holds the parsed interpretation of a refspec. If it +will force updates (starts with a '+'), force is true. If it is a +pattern (sides end with '*') pattern is true. src and dest are the +two sides (including '*' characters if present); if there is only one +side, it is src, and dst is NULL; if sides exist but are empty (i.e., +the refspec either starts or ends with ':'), the corresponding side is +"". + +An array of strings can be parsed into an array of struct refspecs +using parse_fetch_refspec() or parse_push_refspec(). remote_find_tracking(), given a remote and a struct refspec with either src or dst filled out, will fill out the other such that the -- cgit v1.2.1 From f137a45e0db9823d386b18a7a9f5e0fc06c0b67d Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 23 Oct 2013 17:50:38 +0200 Subject: get_ref_map(): rename local variables Rename "refs" -> "refspecs" and "ref_count" -> "refspec_count" to reduce confusion, because they describe an array of "struct refspec", as opposed to the "struct ref" objects that are also used in this function. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index bd7a10164..2248abfd3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -165,8 +165,8 @@ static void find_non_local_tags(struct transport *transport, struct ref ***tail); static struct ref *get_ref_map(struct transport *transport, - struct refspec *refs, int ref_count, int tags, - int *autotags) + struct refspec *refspecs, int refspec_count, + int tags, int *autotags) { int i; struct ref *rm; @@ -175,12 +175,12 @@ static struct ref *get_ref_map(struct transport *transport, const struct ref *remote_refs = transport_get_remote_refs(transport); - if (ref_count || tags == TAGS_SET) { + if (refspec_count || tags == TAGS_SET) { struct ref **old_tail; - for (i = 0; i < ref_count; i++) { - get_fetch_map(remote_refs, &refs[i], &tail, 0); - if (refs[i].dst && refs[i].dst[0]) + for (i = 0; i < refspec_count; i++) { + get_fetch_map(remote_refs, &refspecs[i], &tail, 0); + if (refspecs[i].dst && refspecs[i].dst[0]) *autotags = 1; } /* Merge everything on the command line, but not --tags */ -- cgit v1.2.1 From a0fbb5a329ea138e02698312f14ac1aea4b9b4c9 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:32:55 +0100 Subject: builtin/fetch.c: reorder function definitions Reorder function definitions to avoid the need for a forward declaration of function find_non_local_tags(). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch.c | 198 +++++++++++++++++++++++++++----------------------------- 1 file changed, 97 insertions(+), 101 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 2248abfd3..61e8117c4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -160,9 +160,105 @@ static void add_merge_config(struct ref **head, } } +static int add_existing(const char *refname, const unsigned char *sha1, + int flag, void *cbdata) +{ + struct string_list *list = (struct string_list *)cbdata; + struct string_list_item *item = string_list_insert(list, refname); + item->util = xmalloc(20); + hashcpy(item->util, sha1); + return 0; +} + +static int will_fetch(struct ref **head, const unsigned char *sha1) +{ + struct ref *rm = *head; + while (rm) { + if (!hashcmp(rm->old_sha1, sha1)) + return 1; + rm = rm->next; + } + return 0; +} + static void find_non_local_tags(struct transport *transport, struct ref **head, - struct ref ***tail); + struct ref ***tail) +{ + struct string_list existing_refs = STRING_LIST_INIT_DUP; + struct string_list remote_refs = STRING_LIST_INIT_NODUP; + const struct ref *ref; + struct string_list_item *item = NULL; + + for_each_ref(add_existing, &existing_refs); + for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) { + if (prefixcmp(ref->name, "refs/tags/")) + continue; + + /* + * The peeled ref always follows the matching base + * ref, so if we see a peeled ref that we don't want + * to fetch then we can mark the ref entry in the list + * as one to ignore by setting util to NULL. + */ + if (!suffixcmp(ref->name, "^{}")) { + if (item && !has_sha1_file(ref->old_sha1) && + !will_fetch(head, ref->old_sha1) && + !has_sha1_file(item->util) && + !will_fetch(head, item->util)) + item->util = NULL; + item = NULL; + continue; + } + + /* + * If item is non-NULL here, then we previously saw a + * ref not followed by a peeled reference, so we need + * to check if it is a lightweight tag that we want to + * fetch. + */ + if (item && !has_sha1_file(item->util) && + !will_fetch(head, item->util)) + item->util = NULL; + + item = NULL; + + /* skip duplicates and refs that we already have */ + if (string_list_has_string(&remote_refs, ref->name) || + string_list_has_string(&existing_refs, ref->name)) + continue; + + item = string_list_insert(&remote_refs, ref->name); + item->util = (void *)ref->old_sha1; + } + string_list_clear(&existing_refs, 1); + + /* + * We may have a final lightweight tag that needs to be + * checked to see if it needs fetching. + */ + if (item && !has_sha1_file(item->util) && + !will_fetch(head, item->util)) + item->util = NULL; + + /* + * For all the tags in the remote_refs string list, + * add them to the list of refs to be fetched + */ + for_each_string_list_item(item, &remote_refs) { + /* Unless we have already decided to ignore this item... */ + if (item->util) + { + struct ref *rm = alloc_ref(item->string); + rm->peer_ref = alloc_ref(item->string); + hashcpy(rm->old_sha1, item->util); + **tail = rm; + *tail = &rm->next; + } + } + + string_list_clear(&remote_refs, 0); +} static struct ref *get_ref_map(struct transport *transport, struct refspec *refspecs, int refspec_count, @@ -612,106 +708,6 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map) return result; } -static int add_existing(const char *refname, const unsigned char *sha1, - int flag, void *cbdata) -{ - struct string_list *list = (struct string_list *)cbdata; - struct string_list_item *item = string_list_insert(list, refname); - item->util = xmalloc(20); - hashcpy(item->util, sha1); - return 0; -} - -static int will_fetch(struct ref **head, const unsigned char *sha1) -{ - struct ref *rm = *head; - while (rm) { - if (!hashcmp(rm->old_sha1, sha1)) - return 1; - rm = rm->next; - } - return 0; -} - -static void find_non_local_tags(struct transport *transport, - struct ref **head, - struct ref ***tail) -{ - struct string_list existing_refs = STRING_LIST_INIT_DUP; - struct string_list remote_refs = STRING_LIST_INIT_NODUP; - const struct ref *ref; - struct string_list_item *item = NULL; - - for_each_ref(add_existing, &existing_refs); - for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) { - if (prefixcmp(ref->name, "refs/tags/")) - continue; - - /* - * The peeled ref always follows the matching base - * ref, so if we see a peeled ref that we don't want - * to fetch then we can mark the ref entry in the list - * as one to ignore by setting util to NULL. - */ - if (!suffixcmp(ref->name, "^{}")) { - if (item && !has_sha1_file(ref->old_sha1) && - !will_fetch(head, ref->old_sha1) && - !has_sha1_file(item->util) && - !will_fetch(head, item->util)) - item->util = NULL; - item = NULL; - continue; - } - - /* - * If item is non-NULL here, then we previously saw a - * ref not followed by a peeled reference, so we need - * to check if it is a lightweight tag that we want to - * fetch. - */ - if (item && !has_sha1_file(item->util) && - !will_fetch(head, item->util)) - item->util = NULL; - - item = NULL; - - /* skip duplicates and refs that we already have */ - if (string_list_has_string(&remote_refs, ref->name) || - string_list_has_string(&existing_refs, ref->name)) - continue; - - item = string_list_insert(&remote_refs, ref->name); - item->util = (void *)ref->old_sha1; - } - string_list_clear(&existing_refs, 1); - - /* - * We may have a final lightweight tag that needs to be - * checked to see if it needs fetching. - */ - if (item && !has_sha1_file(item->util) && - !will_fetch(head, item->util)) - item->util = NULL; - - /* - * For all the tags in the remote_refs string list, - * add them to the list of refs to be fetched - */ - for_each_string_list_item(item, &remote_refs) { - /* Unless we have already decided to ignore this item... */ - if (item->util) - { - struct ref *rm = alloc_ref(item->string); - rm->peer_ref = alloc_ref(item->string); - hashcpy(rm->old_sha1, item->util); - **tail = rm; - *tail = &rm->next; - } - } - - string_list_clear(&remote_refs, 0); -} - static void check_not_current_branch(struct ref *ref_map) { struct branch *current_branch = branch_get(NULL); -- cgit v1.2.1 From f166db26af524278b42caac8f092d8de5e3e9c29 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:32:56 +0100 Subject: get_expanded_map(): add docstring Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/remote.c b/remote.c index e9fedfa91..bef81cdb6 100644 --- a/remote.c +++ b/remote.c @@ -1553,6 +1553,13 @@ static int ignore_symref_update(const char *refname) return (flag & REF_ISSYMREF); } +/* + * Create and return a list of (struct ref) consisting of copies of + * each remote_ref that matches refspec. refspec must be a pattern. + * Fill in the copies' peer_ref to describe the local tracking refs to + * which they map. Omit any references that would map to an existing + * local symbolic ref. + */ static struct ref *get_expanded_map(const struct ref *remote_refs, const struct refspec *refspec) { -- cgit v1.2.1 From e31a17f7416a85ee8c3260f8c8b7519ff7ea82b6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:32:57 +0100 Subject: get_expanded_map(): avoid memory leak The old code could leak *expn_name if match_name_with_pattern() succeeded but ignore_symref_update() returned true. So make sure that *expn_name is freed in any case. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/remote.c b/remote.c index bef81cdb6..a83dc9031 100644 --- a/remote.c +++ b/remote.c @@ -1567,9 +1567,9 @@ static struct ref *get_expanded_map(const struct ref *remote_refs, struct ref *ret = NULL; struct ref **tail = &ret; - char *expn_name; - for (ref = remote_refs; ref; ref = ref->next) { + char *expn_name = NULL; + if (strchr(ref->name, '^')) continue; /* a dereference item */ if (match_name_with_pattern(refspec->src, ref->name, @@ -1578,12 +1578,12 @@ static struct ref *get_expanded_map(const struct ref *remote_refs, struct ref *cpy = copy_ref(ref); cpy->peer_ref = alloc_ref(expn_name); - free(expn_name); if (refspec->force) cpy->peer_ref->force = 1; *tail = cpy; tail = &cpy->next; } + free(expn_name); } return ret; -- cgit v1.2.1 From 0281c930f17042473e973f39810c8f7c93955d7d Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:32:58 +0100 Subject: fetch: only opportunistically update references based on command line The old code processed (tags == TAGS_SET) before adding the entries used to opportunistically update references mentioned on the command line. The result was that all tags were also considered candidates for opportunistic updating. This is harmless for two reasons: (a) because it would only add entries if there is a configured refspec that covers tags *and* both --tags and another refspec appear on the command-line; (b) because any extra entries would be deleted later by the call to ref_remove_duplicates() anyway. But, to avoid extra work and extra memory usage, and to make the implementation better match the intention, change the algorithm slightly: compute the opportunistic refspecs based only on the command-line arguments, storing the results into a separate temporary list. Then add the tags (which have to come earlier in the list so that they are not de-duped in favor of an opportunistic entry). Then concatenate the temporary list onto the main list. This change will also make later changes easier. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 61e8117c4..0849f09de 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -272,32 +272,50 @@ static struct ref *get_ref_map(struct transport *transport, const struct ref *remote_refs = transport_get_remote_refs(transport); if (refspec_count || tags == TAGS_SET) { - struct ref **old_tail; + /* opportunistically-updated references: */ + struct ref *orefs = NULL, **oref_tail = &orefs; for (i = 0; i < refspec_count; i++) { get_fetch_map(remote_refs, &refspecs[i], &tail, 0); if (refspecs[i].dst && refspecs[i].dst[0]) *autotags = 1; } - /* Merge everything on the command line, but not --tags */ + /* Merge everything on the command line (but not --tags) */ for (rm = ref_map; rm; rm = rm->next) rm->fetch_head_status = FETCH_HEAD_MERGE; - if (tags == TAGS_SET) - get_fetch_map(remote_refs, tag_refspec, &tail, 0); /* - * For any refs that we happen to be fetching via command-line - * arguments, take the opportunity to update their configured - * counterparts. However, we do not want to mention these - * entries in FETCH_HEAD at all, as they would simply be - * duplicates of existing entries. + * For any refs that we happen to be fetching via + * command-line arguments, the destination ref might + * have been missing or have been different than the + * remote-tracking ref that would be derived from the + * configured refspec. In these cases, we want to + * take the opportunity to update their configured + * remote-tracking reference. However, we do not want + * to mention these entries in FETCH_HEAD at all, as + * they would simply be duplicates of existing + * entries, so we set them FETCH_HEAD_IGNORE below. + * + * We compute these entries now, based only on the + * refspecs specified on the command line. But we add + * them to the list following the refspecs resulting + * from the tags option so that one of the latter, + * which has FETCH_HEAD_NOT_FOR_MERGE, is not removed + * by ref_remove_duplicates() in favor of one of these + * opportunistic entries with FETCH_HEAD_IGNORE. */ - old_tail = tail; for (i = 0; i < transport->remote->fetch_refspec_nr; i++) get_fetch_map(ref_map, &transport->remote->fetch[i], - &tail, 1); - for (rm = *old_tail; rm; rm = rm->next) + &oref_tail, 1); + + if (tags == TAGS_SET) + get_fetch_map(remote_refs, tag_refspec, &tail, 0); + + *tail = orefs; + for (rm = orefs; rm; rm = rm->next) { rm->fetch_head_status = FETCH_HEAD_IGNORE; + tail = &rm->next; + } } else { /* Use the defaults */ struct remote *remote = transport->remote; @@ -334,8 +352,10 @@ static struct ref *get_ref_map(struct transport *transport, tail = &ref_map->next; } } + if (tags == TAGS_DEFAULT && *autotags) find_non_local_tags(transport, &ref_map, &tail); + ref_remove_duplicates(ref_map); return ref_map; -- cgit v1.2.1 From c5a84e92a2fe9e8748e32341c344d7a6c0f52a50 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:32:59 +0100 Subject: fetch --tags: fetch tags *in addition to* other stuff Previously, fetch's "--tags" option was considered equivalent to specifying the refspec "refs/tags/*:refs/tags/*" on the command line; in particular, it caused the remote..refspec configuration to be ignored. But it is not very useful to fetch tags without also fetching other references, whereas it *is* quite useful to be able to fetch tags *in addition to* other references. So change the semantics of this option to do the latter. If a user wants to fetch *only* tags, then it is still possible to specifying an explicit refspec: git fetch 'refs/tags/*:refs/tags/*' Please note that the documentation prior to 1.8.0.3 was ambiguous about this aspect of "fetch --tags" behavior. Commit f0cb2f137c 2012-12-14 fetch --tags: clarify documentation made the documentation match the old behavior. This commit changes the documentation to match the new behavior. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/fetch-options.txt | 8 ++--- builtin/fetch.c | 59 +++++++++++++++++++------------- git-pull.sh | 2 +- t/t5510-fetch.sh | 22 ++++++++++-- t/t5515/fetch.br-unconfig_--tags_.._.git | 1 + t/t5515/fetch.master_--tags_.._.git | 1 + t/t5525-fetch-tagopt.sh | 23 ++++++++++--- 7 files changed, 78 insertions(+), 38 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index ba1fe4958..921f6be8d 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -61,11 +61,9 @@ endif::git-pull[] ifndef::git-pull[] -t:: --tags:: - This is a short-hand for giving `refs/tags/*:refs/tags/*` - refspec from the command line, to ask all tags to be fetched - and stored locally. Because this acts as an explicit - refspec, the default refspecs (configured with the - remote.$name.fetch variable) are overridden and not used. + Request that all tags be fetched from the remote in addition + to whatever else is being fetched. Its effect is similar to + that of the refspec `refs/tags/*:refs/tags/*`. --recurse-submodules[=yes|on-demand|no]:: This option controls if and under what conditions new commits of diff --git a/builtin/fetch.c b/builtin/fetch.c index 0849f09de..887fa3e58 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -269,12 +269,12 @@ static struct ref *get_ref_map(struct transport *transport, struct ref *ref_map = NULL; struct ref **tail = &ref_map; - const struct ref *remote_refs = transport_get_remote_refs(transport); + /* opportunistically-updated references: */ + struct ref *orefs = NULL, **oref_tail = &orefs; - if (refspec_count || tags == TAGS_SET) { - /* opportunistically-updated references: */ - struct ref *orefs = NULL, **oref_tail = &orefs; + const struct ref *remote_refs = transport_get_remote_refs(transport); + if (refspec_count) { for (i = 0; i < refspec_count; i++) { get_fetch_map(remote_refs, &refspecs[i], &tail, 0); if (refspecs[i].dst && refspecs[i].dst[0]) @@ -310,12 +310,6 @@ static struct ref *get_ref_map(struct transport *transport, if (tags == TAGS_SET) get_fetch_map(remote_refs, tag_refspec, &tail, 0); - - *tail = orefs; - for (rm = orefs; rm; rm = rm->next) { - rm->fetch_head_status = FETCH_HEAD_IGNORE; - tail = &rm->next; - } } else { /* Use the defaults */ struct remote *remote = transport->remote; @@ -353,9 +347,19 @@ static struct ref *get_ref_map(struct transport *transport, } } - if (tags == TAGS_DEFAULT && *autotags) + if (tags == TAGS_SET) + /* also fetch all tags */ + get_fetch_map(remote_refs, tag_refspec, &tail, 0); + else if (tags == TAGS_DEFAULT && *autotags) find_non_local_tags(transport, &ref_map, &tail); + /* Now append any refs to be updated opportunistically: */ + *tail = orefs; + for (rm = orefs; rm; rm = rm->next) { + rm->fetch_head_status = FETCH_HEAD_IGNORE; + tail = &rm->next; + } + ref_remove_duplicates(ref_map); return ref_map; @@ -846,31 +850,38 @@ static int do_fetch(struct transport *transport, goto cleanup; } if (prune) { - /* - * If --tags was specified, pretend that the user gave us - * the canonical tags refspec - */ + struct refspec *prune_refspecs; + int prune_refspec_count; + + if (ref_count) { + prune_refspecs = refs; + prune_refspec_count = ref_count; + } else { + prune_refspecs = transport->remote->fetch; + prune_refspec_count = transport->remote->fetch_refspec_nr; + } + if (tags == TAGS_SET) { + /* + * --tags was specified. Pretend that the user also + * gave us the canonical tags refspec + */ const char *tags_str = "refs/tags/*:refs/tags/*"; struct refspec *tags_refspec, *refspec; /* Copy the refspec and add the tags to it */ - refspec = xcalloc(ref_count + 1, sizeof(struct refspec)); + refspec = xcalloc(prune_refspec_count + 1, sizeof(*refspec)); tags_refspec = parse_fetch_refspec(1, &tags_str); - memcpy(refspec, refs, ref_count * sizeof(struct refspec)); - memcpy(&refspec[ref_count], tags_refspec, sizeof(struct refspec)); - ref_count++; + memcpy(refspec, prune_refspecs, prune_refspec_count * sizeof(*refspec)); + memcpy(&refspec[prune_refspec_count], tags_refspec, sizeof(*refspec)); - prune_refs(refspec, ref_count, ref_map); + prune_refs(refspec, prune_refspec_count + 1, ref_map); - ref_count--; /* The rest of the strings belong to fetch_one */ free_refspec(1, tags_refspec); free(refspec); - } else if (ref_count) { - prune_refs(refs, ref_count, ref_map); } else { - prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map); + prune_refs(prune_refspecs, prune_refspec_count, ref_map); } } free_refs(ref_map); diff --git a/git-pull.sh b/git-pull.sh index b946fd975..97716b885 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -172,7 +172,7 @@ error_on_no_merge_candidates () { do case "$opt" in -t|--t|--ta|--tag|--tags) - echo "Fetching tags only, you probably meant:" + echo "It doesn't make sense to pull all tags; you probably meant:" echo " git fetch --tags" exit 1 esac diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 8328be134..02e5901ab 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -113,7 +113,7 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' ' git rev-parse origin/master ' -test_expect_success 'fetch --prune --tags does not delete the remote-tracking branches' ' +test_expect_success 'fetch --prune --tags prunes tags and branches' ' cd "$D" && git clone . prune-tags && cd prune-tags && @@ -124,7 +124,7 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br git fetch --prune --tags origin && git rev-parse origin/master && - git rev-parse origin/fake-remote && + test_must_fail git rev-parse origin/fake-remote && test_must_fail git rev-parse sometag ' @@ -132,10 +132,26 @@ test_expect_success 'fetch --prune --tags with branch does not delete other remo cd "$D" && git clone . prune-tags-branch && cd prune-tags-branch && + git tag sometag master && git update-ref refs/remotes/origin/extrabranch master && git fetch --prune --tags origin master && - git rev-parse origin/extrabranch + git rev-parse origin/extrabranch && + test_must_fail git rev-parse sometag +' + +test_expect_success 'fetch --prune --tags with refspec prunes based on refspec' ' + cd "$D" && + git clone . prune-tags-refspec && + cd prune-tags-refspec && + git tag sometag master && + git update-ref refs/remotes/origin/foo/otherbranch master && + git update-ref refs/remotes/origin/extrabranch master && + + git fetch --prune --tags origin refs/heads/foo/*:refs/remotes/origin/foo/* && + test_must_fail git rev-parse refs/remotes/origin/foo/otherbranch && + git rev-parse origin/extrabranch && + test_must_fail git rev-parse sometag ' test_expect_success 'fetch tags when there is no tags' ' diff --git a/t/t5515/fetch.br-unconfig_--tags_.._.git b/t/t5515/fetch.br-unconfig_--tags_.._.git index 1669cc4af..0f70f66c7 100644 --- a/t/t5515/fetch.br-unconfig_--tags_.._.git +++ b/t/t5515/fetch.br-unconfig_--tags_.._.git @@ -1,4 +1,5 @@ # br-unconfig --tags ../.git +0567da4d5edd2ff4bb292a465ba9e64dcad9536b ../ 6c9dec2b923228c9ff994c6cfe4ae16c12408dc5 not-for-merge tag 'tag-master' of ../ 8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../ 22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../ diff --git a/t/t5515/fetch.master_--tags_.._.git b/t/t5515/fetch.master_--tags_.._.git index 8a7493537..ab473a6e1 100644 --- a/t/t5515/fetch.master_--tags_.._.git +++ b/t/t5515/fetch.master_--tags_.._.git @@ -1,4 +1,5 @@ # master --tags ../.git +0567da4d5edd2ff4bb292a465ba9e64dcad9536b ../ 6c9dec2b923228c9ff994c6cfe4ae16c12408dc5 not-for-merge tag 'tag-master' of ../ 8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../ 22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../ diff --git a/t/t5525-fetch-tagopt.sh b/t/t5525-fetch-tagopt.sh index 4fbf7a120..45815f737 100755 --- a/t/t5525-fetch-tagopt.sh +++ b/t/t5525-fetch-tagopt.sh @@ -8,7 +8,8 @@ setup_clone () { git clone --mirror . $1 && git remote add remote_$1 $1 && (cd $1 && - git tag tag_$1) + git tag tag_$1 && + git branch branch_$1) } test_expect_success setup ' @@ -21,21 +22,33 @@ test_expect_success setup ' test_expect_success "fetch with tagopt=--no-tags does not get tag" ' git fetch remote_one && - test_must_fail git show-ref tag_one + test_must_fail git show-ref tag_one && + git show-ref remote_one/branch_one ' test_expect_success "fetch --tags with tagopt=--no-tags gets tag" ' + ( + cd one && + git branch second_branch_one + ) && git fetch --tags remote_one && - git show-ref tag_one + git show-ref tag_one && + git show-ref remote_one/second_branch_one ' test_expect_success "fetch --no-tags with tagopt=--tags does not get tag" ' git fetch --no-tags remote_two && - test_must_fail git show-ref tag_two + test_must_fail git show-ref tag_two && + git show-ref remote_two/branch_two ' test_expect_success "fetch with tagopt=--tags gets tag" ' + ( + cd two && + git branch second_branch_two + ) && git fetch remote_two && - git show-ref tag_two + git show-ref tag_two && + git show-ref remote_two/second_branch_two ' test_done -- cgit v1.2.1 From 0838bf47b3b37f6ff13b8de183316483958bf3bb Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:00 +0100 Subject: fetch --prune: prune only based on explicit refspecs The old behavior of "fetch --prune" was to prune whatever was being fetched. In particular, "fetch --prune --tags" caused tags not only to be fetched, but also to be pruned. This is inappropriate because there is only one tags namespace that is shared among the local repository and all remotes. Therefore, if the user defines a local tag and then runs "git fetch --prune --tags", then the local tag is deleted. Moreover, "--prune" and "--tags" can also be configured via fetch.prune / remote..prune and remote..tagopt, making it even less obvious that an invocation of "git fetch" could result in tag lossage. Since the command "git remote update" invokes "git fetch", it had the same problem. The command "git remote prune", on the other hand, disregarded the setting of remote..tagopt, and so its behavior was inconsistent with that of the other commands. So the old behavior made it too easy to lose tags. To fix this problem, change "fetch --prune" to prune references based only on refspecs specified explicitly by the user, either on the command line or via remote..fetch. Thus, tags are no longer made subject to pruning by the --tags option or the remote..tagopt setting. However, tags *are* still subject to pruning if they are fetched as part of a refspec, and that is good. For example: * On the command line, git fetch --prune 'refs/tags/*:refs/tags/*' causes tags, and only tags, to be fetched and pruned, and is therefore a simple way for the user to get the equivalent of the old behavior of "--prune --tag". * For a remote that was configured with the "--mirror" option, the configuration is set to include [remote "name"] fetch = +refs/*:refs/* , which causes tags to be subject to pruning along with all other references. This is the behavior that will typically be desired for a mirror. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/config.txt | 4 ++-- Documentation/fetch-options.txt | 19 ++++++++++++++----- builtin/fetch.c | 39 +++++++++------------------------------ t/t5510-fetch.sh | 10 +++++----- 4 files changed, 30 insertions(+), 42 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ab26963d6..a4058063c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2087,8 +2087,8 @@ remote..vcs:: remote..prune:: When set to true, fetching from this remote by default will also - remove any remote-tracking branches which no longer exist on the - remote (as if the `--prune` option was give on the command line). + remove any remote-tracking references that no longer exist on the + remote (as if the `--prune` option was given on the command line). Overrides `fetch.prune` settings, if any. remotes.:: diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 921f6be8d..12b1d92a2 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -41,8 +41,14 @@ ifndef::git-pull[] -p:: --prune:: - After fetching, remove any remote-tracking branches which - no longer exist on the remote. + After fetching, remove any remote-tracking references that no + longer exist on the remote. Tags are not subject to pruning + if they are fetched only because of the default tag + auto-following or due to a --tags option. However, if tags + are fetched due to an explicit refspec (either on the command + line or in the remote configuration, for example if the remote + was cloned with the --mirror option), then they are also + subject to pruning. endif::git-pull[] ifdef::git-pull[] @@ -61,9 +67,12 @@ endif::git-pull[] ifndef::git-pull[] -t:: --tags:: - Request that all tags be fetched from the remote in addition - to whatever else is being fetched. Its effect is similar to - that of the refspec `refs/tags/*:refs/tags/*`. + Fetch all tags from the remote (i.e., fetch remote tags + `refs/tags/*` into local tags with the same name), in addition + to whatever else would otherwise be fetched. Using this + option alone does not subject tags to pruning, even if --prune + is used (though tags may be pruned anyway if they are also the + destination of an explicit refspec; see '--prune'). --recurse-submodules[=yes|on-demand|no]:: This option controls if and under what conditions new commits of diff --git a/builtin/fetch.c b/builtin/fetch.c index 887fa3e58..1514b908d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -850,38 +850,17 @@ static int do_fetch(struct transport *transport, goto cleanup; } if (prune) { - struct refspec *prune_refspecs; - int prune_refspec_count; - + /* + * We only prune based on refspecs specified + * explicitly (via command line or configuration); we + * don't care whether --tags was specified. + */ if (ref_count) { - prune_refspecs = refs; - prune_refspec_count = ref_count; - } else { - prune_refspecs = transport->remote->fetch; - prune_refspec_count = transport->remote->fetch_refspec_nr; - } - - if (tags == TAGS_SET) { - /* - * --tags was specified. Pretend that the user also - * gave us the canonical tags refspec - */ - const char *tags_str = "refs/tags/*:refs/tags/*"; - struct refspec *tags_refspec, *refspec; - - /* Copy the refspec and add the tags to it */ - refspec = xcalloc(prune_refspec_count + 1, sizeof(*refspec)); - tags_refspec = parse_fetch_refspec(1, &tags_str); - memcpy(refspec, prune_refspecs, prune_refspec_count * sizeof(*refspec)); - memcpy(&refspec[prune_refspec_count], tags_refspec, sizeof(*refspec)); - - prune_refs(refspec, prune_refspec_count + 1, ref_map); - - /* The rest of the strings belong to fetch_one */ - free_refspec(1, tags_refspec); - free(refspec); + prune_refs(refs, ref_count, ref_map); } else { - prune_refs(prune_refspecs, prune_refspec_count, ref_map); + prune_refs(transport->remote->fetch, + transport->remote->fetch_refspec_nr, + ref_map); } } free_refs(ref_map); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 02e5901ab..5d4581dac 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -113,7 +113,7 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' ' git rev-parse origin/master ' -test_expect_success 'fetch --prune --tags prunes tags and branches' ' +test_expect_success 'fetch --prune --tags prunes branches but not tags' ' cd "$D" && git clone . prune-tags && cd prune-tags && @@ -125,10 +125,10 @@ test_expect_success 'fetch --prune --tags prunes tags and branches' ' git fetch --prune --tags origin && git rev-parse origin/master && test_must_fail git rev-parse origin/fake-remote && - test_must_fail git rev-parse sometag + git rev-parse sometag ' -test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' ' +test_expect_success 'fetch --prune --tags with branch does not prune other things' ' cd "$D" && git clone . prune-tags-branch && cd prune-tags-branch && @@ -137,7 +137,7 @@ test_expect_success 'fetch --prune --tags with branch does not delete other remo git fetch --prune --tags origin master && git rev-parse origin/extrabranch && - test_must_fail git rev-parse sometag + git rev-parse sometag ' test_expect_success 'fetch --prune --tags with refspec prunes based on refspec' ' @@ -151,7 +151,7 @@ test_expect_success 'fetch --prune --tags with refspec prunes based on refspec' git fetch --prune --tags origin refs/heads/foo/*:refs/remotes/origin/foo/* && test_must_fail git rev-parse refs/remotes/origin/foo/otherbranch && git rev-parse origin/extrabranch && - test_must_fail git rev-parse sometag + git rev-parse sometag ' test_expect_success 'fetch tags when there is no tags' ' -- cgit v1.2.1 From 049bff8f0ec722dbd4ca9582df7891c920e41165 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:01 +0100 Subject: query_refspecs(): move some constants out of the loop Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/remote.c b/remote.c index a83dc9031..4d45f5dc7 100644 --- a/remote.c +++ b/remote.c @@ -825,6 +825,8 @@ static int query_refspecs(struct refspec *refs, int ref_count, struct refspec *q { int i; int find_src = !query->src; + const char *needle = find_src ? query->dst : query->src; + char **result = find_src ? &query->src : &query->dst; if (find_src && !query->dst) return error("query_refspecs: need either src or dst"); @@ -833,8 +835,6 @@ static int query_refspecs(struct refspec *refs, int ref_count, struct refspec *q struct refspec *refspec = &refs[i]; const char *key = find_src ? refspec->dst : refspec->src; const char *value = find_src ? refspec->src : refspec->dst; - const char *needle = find_src ? query->dst : query->src; - char **result = find_src ? &query->src : &query->dst; if (!refspec->dst) continue; -- cgit v1.2.1 From ce2223fde87c6df2a3d837b188d9509527de7264 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:02 +0100 Subject: builtin/remote.c: reorder function definitions Reorder function definitions to remove the need for forward declarations. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/remote.c | 159 +++++++++++++++++++++++++++---------------------------- 1 file changed, 78 insertions(+), 81 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 4e1489109..ecedd968d 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -77,9 +77,6 @@ static const char * const builtin_remote_seturl_usage[] = { static int verbose; -static int show_all(void); -static int prune_remote(const char *remote, int dry_run); - static inline int postfixcmp(const char *string, const char *postfix) { int len1 = strlen(string), len2 = strlen(postfix); @@ -1084,6 +1081,64 @@ static int show_push_info_item(struct string_list_item *item, void *cb_data) return 0; } +static int get_one_entry(struct remote *remote, void *priv) +{ + struct string_list *list = priv; + struct strbuf url_buf = STRBUF_INIT; + const char **url; + int i, url_nr; + + if (remote->url_nr > 0) { + strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]); + string_list_append(list, remote->name)->util = + strbuf_detach(&url_buf, NULL); + } else + string_list_append(list, remote->name)->util = NULL; + if (remote->pushurl_nr) { + url = remote->pushurl; + url_nr = remote->pushurl_nr; + } else { + url = remote->url; + url_nr = remote->url_nr; + } + for (i = 0; i < url_nr; i++) + { + strbuf_addf(&url_buf, "%s (push)", url[i]); + string_list_append(list, remote->name)->util = + strbuf_detach(&url_buf, NULL); + } + + return 0; +} + +static int show_all(void) +{ + struct string_list list = STRING_LIST_INIT_NODUP; + int result; + + list.strdup_strings = 1; + result = for_each_remote(get_one_entry, &list); + + if (!result) { + int i; + + sort_string_list(&list); + for (i = 0; i < list.nr; i++) { + struct string_list_item *item = list.items + i; + if (verbose) + printf("%s\t%s\n", item->string, + item->util ? (const char *)item->util : ""); + else { + if (i && !strcmp((item - 1)->string, item->string)) + continue; + printf("%s\n", item->string); + } + } + } + string_list_clear(&list, 1); + return result; +} + static int show(int argc, const char **argv) { int no_query = 0, result = 0, query_flag = 0; @@ -1246,26 +1301,6 @@ static int set_head(int argc, const char **argv) return result; } -static int prune(int argc, const char **argv) -{ - int dry_run = 0, result = 0; - struct option options[] = { - OPT__DRY_RUN(&dry_run, N_("dry run")), - OPT_END() - }; - - argc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage, - 0); - - if (argc < 1) - usage_with_options(builtin_remote_prune_usage, options); - - for (; argc; argc--, argv++) - result |= prune_remote(*argv, dry_run); - - return result; -} - static int prune_remote(const char *remote, int dry_run) { int result = 0, i; @@ -1304,6 +1339,26 @@ static int prune_remote(const char *remote, int dry_run) return result; } +static int prune(int argc, const char **argv) +{ + int dry_run = 0, result = 0; + struct option options[] = { + OPT__DRY_RUN(&dry_run, N_("dry run")), + OPT_END() + }; + + argc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage, + 0); + + if (argc < 1) + usage_with_options(builtin_remote_prune_usage, options); + + for (; argc; argc--, argv++) + result |= prune_remote(*argv, dry_run); + + return result; +} + static int get_remote_default(const char *key, const char *value, void *priv) { if (strcmp(key, "remotes.default") == 0) { @@ -1505,64 +1560,6 @@ static int set_url(int argc, const char **argv) return 0; } -static int get_one_entry(struct remote *remote, void *priv) -{ - struct string_list *list = priv; - struct strbuf url_buf = STRBUF_INIT; - const char **url; - int i, url_nr; - - if (remote->url_nr > 0) { - strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]); - string_list_append(list, remote->name)->util = - strbuf_detach(&url_buf, NULL); - } else - string_list_append(list, remote->name)->util = NULL; - if (remote->pushurl_nr) { - url = remote->pushurl; - url_nr = remote->pushurl_nr; - } else { - url = remote->url; - url_nr = remote->url_nr; - } - for (i = 0; i < url_nr; i++) - { - strbuf_addf(&url_buf, "%s (push)", url[i]); - string_list_append(list, remote->name)->util = - strbuf_detach(&url_buf, NULL); - } - - return 0; -} - -static int show_all(void) -{ - struct string_list list = STRING_LIST_INIT_NODUP; - int result; - - list.strdup_strings = 1; - result = for_each_remote(get_one_entry, &list); - - if (!result) { - int i; - - sort_string_list(&list); - for (i = 0; i < list.nr; i++) { - struct string_list_item *item = list.items + i; - if (verbose) - printf("%s\t%s\n", item->string, - item->util ? (const char *)item->util : ""); - else { - if (i && !strcmp((item - 1)->string, item->string)) - continue; - printf("%s\n", item->string); - } - } - } - string_list_clear(&list, 1); - return result; -} - int cmd_remote(int argc, const char **argv, const char *prefix) { struct option options[] = { -- cgit v1.2.1 From 8607590e74814c6c6a352cbd099e2617a69bb2c6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:03 +0100 Subject: builtin/remote.c:update(): use struct argv_array Use struct argv_array for calling the "git fetch" subprocesses. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/remote.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index ecedd968d..bffe2f9f7 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -6,6 +6,7 @@ #include "strbuf.h" #include "run-command.h" #include "refs.h" +#include "argv-array.h" static const char * const builtin_remote_usage[] = { N_("git remote [-v | --verbose]"), @@ -1376,36 +1377,36 @@ static int update(int argc, const char **argv) N_("prune remotes after fetching")), OPT_END() }; - const char **fetch_argv; - int fetch_argc = 0; + struct argv_array fetch_argv = ARGV_ARRAY_INIT; int default_defined = 0; - - fetch_argv = xmalloc(sizeof(char *) * (argc+5)); + int retval; argc = parse_options(argc, argv, NULL, options, builtin_remote_update_usage, PARSE_OPT_KEEP_ARGV0); - fetch_argv[fetch_argc++] = "fetch"; + argv_array_push(&fetch_argv, "fetch"); if (prune) - fetch_argv[fetch_argc++] = "--prune"; + argv_array_push(&fetch_argv, "--prune"); if (verbose) - fetch_argv[fetch_argc++] = "-v"; - fetch_argv[fetch_argc++] = "--multiple"; + argv_array_push(&fetch_argv, "-v"); + argv_array_push(&fetch_argv, "--multiple"); if (argc < 2) - fetch_argv[fetch_argc++] = "default"; + argv_array_push(&fetch_argv, "default"); for (i = 1; i < argc; i++) - fetch_argv[fetch_argc++] = argv[i]; + argv_array_push(&fetch_argv, argv[i]); - if (strcmp(fetch_argv[fetch_argc-1], "default") == 0) { + if (strcmp(fetch_argv.argv[fetch_argv.argc-1], "default") == 0) { git_config(get_remote_default, &default_defined); - if (!default_defined) - fetch_argv[fetch_argc-1] = "--all"; + if (!default_defined) { + argv_array_pop(&fetch_argv); + argv_array_push(&fetch_argv, "--all"); + } } - fetch_argv[fetch_argc] = NULL; - - return run_command_v_opt(fetch_argv, RUN_GIT_CMD); + retval = run_command_v_opt(fetch_argv.argv, RUN_GIT_CMD); + argv_array_clear(&fetch_argv); + return retval; } static int remove_all_fetch_refspecs(const char *remote, const char *key) -- cgit v1.2.1 From 90765fa3e01658ac2fb75a4eb740cee7dad4a2dc Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:04 +0100 Subject: fetch, remote: properly convey --no-prune options to subprocesses If --no-prune is passed to one of the following commands: git fetch --all git fetch --multiple git fetch --recurse-submodules git remote update then it must also be passed to the "fetch" subprocesses that those commands use to do their work. Otherwise there might be a fetch.prune or remote..prune configuration setting that causes pruning to occur, contrary to the user's express wish. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch.c | 4 ++-- builtin/remote.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 1514b908d..5ddb9af05 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -936,8 +936,8 @@ static void add_options_to_argv(struct argv_array *argv) { if (dry_run) argv_array_push(argv, "--dry-run"); - if (prune > 0) - argv_array_push(argv, "--prune"); + if (prune != -1) + argv_array_push(argv, prune ? "--prune" : "--no-prune"); if (update_head_ok) argv_array_push(argv, "--update-head-ok"); if (force) diff --git a/builtin/remote.c b/builtin/remote.c index bffe2f9f7..f532f3545 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1371,7 +1371,7 @@ static int get_remote_default(const char *key, const char *value, void *priv) static int update(int argc, const char **argv) { - int i, prune = 0; + int i, prune = -1; struct option options[] = { OPT_BOOL('p', "prune", &prune, N_("prune remotes after fetching")), @@ -1386,8 +1386,8 @@ static int update(int argc, const char **argv) argv_array_push(&fetch_argv, "fetch"); - if (prune) - argv_array_push(&fetch_argv, "--prune"); + if (prune != -1) + argv_array_push(&fetch_argv, prune ? "--prune" : "--no-prune"); if (verbose) argv_array_push(&fetch_argv, "-v"); argv_array_push(&fetch_argv, "--multiple"); -- cgit v1.2.1 From 01ca90c2e5e7ffab035c4746d8b7c412fca75874 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:05 +0100 Subject: fetch-options.txt: simplify ifdef/ifndef/endif usage Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/fetch-options.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 12b1d92a2..f0ef7d02a 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -51,13 +51,10 @@ ifndef::git-pull[] subject to pruning. endif::git-pull[] -ifdef::git-pull[] ---no-tags:: -endif::git-pull[] ifndef::git-pull[] -n:: ---no-tags:: endif::git-pull[] +--no-tags:: By default, tags that point at objects that are downloaded from the remote repository are fetched and stored locally. This option disables this automatic tag following. The default -- cgit v1.2.1 From 37f0dcbdc1585dafd81c393aa750292e8634035d Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:06 +0100 Subject: git-fetch.txt: improve description of tag auto-following Make it clearer that tags are fetched independent of which branches were fetched from the remote in any particular fetch. (Tags are even fetched if they point at objects that are in the current repository but not reachable, which is probably a bug.) Put less emphasis on the mechanism and more on the effect of tag auto-following. Also mention the options and configuration settings that can change the tag-fetching behavior. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/git-fetch.txt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index e08a02894..10657134a 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -24,13 +24,13 @@ The ref names and their object names of fetched refs are stored in `.git/FETCH_HEAD`. This information is left for a later merge operation done by 'git merge'. -When stores the fetched result in remote-tracking branches, -the tags that point at these branches are automatically -followed. This is done by first fetching from the remote using -the given s, and if the repository has objects that are -pointed by remote tags that it does not yet have, then fetch -those missing tags. If the other end has tags that point at -branches you are not interested in, you will not get them. +By default, tags are auto-followed. This means that when fetching +from a remote, any tags on the remote that point to objects that exist +in the local repository are fetched. The effect is to fetch tags that +point at branches that you are interested in. This default behavior +can be changed by using the --tags or --no-tags options, by +configuring remote..tagopt, or by using a refspec that fetches +tags explicitly. 'git fetch' can fetch from either a single named repository, or from several repositories at once if is given and -- cgit v1.2.1 From 09ea1f8e0e85dd65307b29219ef1f5a3958eb918 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:07 +0100 Subject: ref_remove_duplicates(): avoid redundant bisection The old code called string_list_lookup(), and if that failed called string_list_insert(), thus doing the bisection search through the string list twice in the latter code path. Instead, just call string_list_insert() right away. If an entry for that peer reference name already existed, then its util pointer is always non-NULL. Of course this doesn't change the fact that the repeated string_list_insert() calls make the function scale like O(N^2) if the input reference list is not already approximately sorted. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/remote.c b/remote.c index 4d45f5dc7..1fb87de01 100644 --- a/remote.c +++ b/remote.c @@ -750,13 +750,15 @@ void ref_remove_duplicates(struct ref *ref_map) struct string_list refs = STRING_LIST_INIT_NODUP; struct string_list_item *item = NULL; struct ref *prev = NULL, *next = NULL; + for (; ref_map; prev = ref_map, ref_map = next) { next = ref_map->next; if (!ref_map->peer_ref) continue; - item = string_list_lookup(&refs, ref_map->peer_ref->name); - if (item) { + item = string_list_insert(&refs, ref_map->peer_ref->name); + if (item->util) { + /* Entry already existed */ if (strcmp(((struct ref *)item->util)->name, ref_map->name)) die("%s tracks both %s and %s", @@ -767,11 +769,9 @@ void ref_remove_duplicates(struct ref *ref_map) free(ref_map->peer_ref); free(ref_map); ref_map = prev; /* skip this; we freed it */ - continue; + } else { + item->util = ref_map; } - - item = string_list_insert(&refs, ref_map->peer_ref->name); - item->util = ref_map; } string_list_clear(&refs, 0); } -- cgit v1.2.1 From 2071e05ed27df847e300b51aca661a34a765aea3 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:08 +0100 Subject: t5536: new test of refspec conflicts when fetching Add some tests that "git fetch" handles refspec conflicts (i.e., when the same local reference should be updated from two different remote references) correctly. There is a small bug when updating references opportunistically, namely that an explicit user wish like git fetch origin \ refs/heads/branch1:refs/remotes/origin/branch2 \ refs/heads/branch2:refs/remotes/origin/branch1 should override a configured refspec like +refs/heads/*:refs/remotes/origin/* The current code incorrectly treats this as a fatal error. In a few commits we will improve the error messages for refspec conflicts in general and also turn this buggy fatal error into a warning. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t5536-fetch-conflicts.sh | 96 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100755 t/t5536-fetch-conflicts.sh diff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh new file mode 100755 index 000000000..679c53e04 --- /dev/null +++ b/t/t5536-fetch-conflicts.sh @@ -0,0 +1,96 @@ +#!/bin/sh + +test_description='fetch handles conflicting refspecs correctly' + +. ./test-lib.sh + +D=$(pwd) + +setup_repository () { + git init "$1" && ( + cd "$1" && + git config remote.origin.url "$D" && + shift && + for refspec in "$@" + do + git config --add remote.origin.fetch "$refspec" + done + ) +} + +verify_stderr () { + cat >expected && + # We're not interested in the error + # "fatal: The remote end hung up unexpectedly": + grep -v "hung up" actual && + test_cmp expected actual +} + +test_expect_success 'setup' ' + git commit --allow-empty -m "Initial" && + git branch branch1 && + git tag tag1 && + git commit --allow-empty -m "First" && + git branch branch2 && + git tag tag2 +' + +test_expect_success 'fetch with no conflict' ' + setup_repository ok "+refs/heads/*:refs/remotes/origin/*" && ( + cd ok && + git fetch origin + ) +' + +test_expect_success 'fetch conflict: config vs. config' ' + setup_repository ccc \ + "+refs/heads/branch1:refs/remotes/origin/branch1" \ + "+refs/heads/branch2:refs/remotes/origin/branch1" && ( + cd ccc && + test_must_fail git fetch origin 2>error && + verify_stderr <<-\EOF + fatal: refs/remotes/origin/branch1 tracks both refs/heads/branch1 and refs/heads/branch2 + EOF + ) +' + +test_expect_success 'fetch duplicate: config vs. config' ' + setup_repository dcc \ + "+refs/heads/*:refs/remotes/origin/*" \ + "+refs/heads/branch1:refs/remotes/origin/branch1" && ( + cd dcc && + git fetch origin + ) +' + +test_expect_success 'fetch conflict: arg overrides config' ' + setup_repository aoc \ + "+refs/heads/*:refs/remotes/origin/*" && ( + cd aoc && + git fetch origin refs/heads/branch2:refs/remotes/origin/branch1 + ) +' + +test_expect_success 'fetch conflict: arg vs. arg' ' + setup_repository caa && ( + cd caa && + test_must_fail git fetch origin \ + refs/heads/*:refs/remotes/origin/* \ + refs/heads/branch2:refs/remotes/origin/branch1 2>error && + verify_stderr <<-\EOF + fatal: refs/remotes/origin/branch1 tracks both refs/heads/branch1 and refs/heads/branch2 + EOF + ) +' + +test_expect_failure 'fetch conflict: criss-cross args' ' + setup_repository xaa \ + "+refs/heads/*:refs/remotes/origin/*" && ( + cd xaa && + git fetch origin \ + refs/heads/branch1:refs/remotes/origin/branch2 \ + refs/heads/branch2:refs/remotes/origin/branch1 + ) +' + +test_done -- cgit v1.2.1 From b9afe6654db2bfb776db933f832e7e03052adf98 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:09 +0100 Subject: ref_remove_duplicates(): simplify loop logic Change the loop body into the more straightforward * remove item from the front of the old list * if necessary, add it to the tail of the new list and return a pointer to the new list (even though it is currently always the same as the input argument, because the first element in the list is currently never deleted). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch.c | 4 +--- remote.c | 52 +++++++++++++++++++++++++++++++--------------------- remote.h | 8 ++++++-- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 5ddb9af05..3d978eb58 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -360,9 +360,7 @@ static struct ref *get_ref_map(struct transport *transport, tail = &rm->next; } - ref_remove_duplicates(ref_map); - - return ref_map; + return ref_remove_duplicates(ref_map); } #define STORE_REF_ERROR_OTHER 1 diff --git a/remote.c b/remote.c index 1fb87de01..f80399076 100644 --- a/remote.c +++ b/remote.c @@ -745,35 +745,45 @@ int for_each_remote(each_remote_fn fn, void *priv) return result; } -void ref_remove_duplicates(struct ref *ref_map) +struct ref *ref_remove_duplicates(struct ref *ref_map) { struct string_list refs = STRING_LIST_INIT_NODUP; - struct string_list_item *item = NULL; - struct ref *prev = NULL, *next = NULL; + struct ref *retval = NULL; + struct ref **p = &retval; - for (; ref_map; prev = ref_map, ref_map = next) { - next = ref_map->next; - if (!ref_map->peer_ref) - continue; + while (ref_map) { + struct ref *ref = ref_map; + + ref_map = ref_map->next; + ref->next = NULL; - item = string_list_insert(&refs, ref_map->peer_ref->name); - if (item->util) { - /* Entry already existed */ - if (strcmp(((struct ref *)item->util)->name, - ref_map->name)) - die("%s tracks both %s and %s", - ref_map->peer_ref->name, - ((struct ref *)item->util)->name, - ref_map->name); - prev->next = ref_map->next; - free(ref_map->peer_ref); - free(ref_map); - ref_map = prev; /* skip this; we freed it */ + if (!ref->peer_ref) { + *p = ref; + p = &ref->next; } else { - item->util = ref_map; + struct string_list_item *item = + string_list_insert(&refs, ref->peer_ref->name); + + if (item->util) { + /* Entry already existed */ + if (strcmp(((struct ref *)item->util)->name, + ref->name)) + die("%s tracks both %s and %s", + ref->peer_ref->name, + ((struct ref *)item->util)->name, + ref->name); + free(ref->peer_ref); + free(ref); + } else { + *p = ref; + p = &ref->next; + item->util = ref; + } } } + string_list_clear(&refs, 0); + return retval; } int remote_has_url(struct remote *remote, const char *url) diff --git a/remote.h b/remote.h index 131130a61..c07eb9950 100644 --- a/remote.h +++ b/remote.h @@ -149,9 +149,13 @@ int resolve_remote_symref(struct ref *ref, struct ref *list); int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1); /* - * Removes and frees any duplicate refs in the map. + * Remove and free all but the first of any entries in the input list + * that map the same remote reference to the same local reference. If + * there are two entries that map different remote references to the + * same local reference, emit an error message and die. Return a + * pointer to the head of the resulting list. */ -void ref_remove_duplicates(struct ref *ref_map); +struct ref *ref_remove_duplicates(struct ref *ref_map); int valid_fetch_refspec(const char *refspec); struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec); -- cgit v1.2.1 From df02ebdac8bdf0edd30e85c9902d177dccfac276 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:10 +0100 Subject: ref_remote_duplicates(): extract a function handle_duplicate() It will become more complex in a moment. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/remote.c b/remote.c index f80399076..4bed101da 100644 --- a/remote.c +++ b/remote.c @@ -745,6 +745,15 @@ int for_each_remote(each_remote_fn fn, void *priv) return result; } +static void handle_duplicate(struct ref *ref1, struct ref *ref2) +{ + if (strcmp(ref1->name, ref2->name)) + die("%s tracks both %s and %s", + ref2->peer_ref->name, ref1->name, ref2->name); + free(ref2->peer_ref); + free(ref2); +} + struct ref *ref_remove_duplicates(struct ref *ref_map) { struct string_list refs = STRING_LIST_INIT_NODUP; @@ -766,14 +775,7 @@ struct ref *ref_remove_duplicates(struct ref *ref_map) if (item->util) { /* Entry already existed */ - if (strcmp(((struct ref *)item->util)->name, - ref->name)) - die("%s tracks both %s and %s", - ref->peer_ref->name, - ((struct ref *)item->util)->name, - ref->name); - free(ref->peer_ref); - free(ref); + handle_duplicate((struct ref *)item->util, ref); } else { *p = ref; p = &ref->next; -- cgit v1.2.1 From 76ea6717fe7dda28966f586e09e02b7b0d5b76cf Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:11 +0100 Subject: handle_duplicate(): mark error message for translation Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 4bed101da..0848faffa 100644 --- a/remote.c +++ b/remote.c @@ -748,7 +748,7 @@ int for_each_remote(each_remote_fn fn, void *priv) static void handle_duplicate(struct ref *ref1, struct ref *ref2) { if (strcmp(ref1->name, ref2->name)) - die("%s tracks both %s and %s", + die(_("%s tracks both %s and %s"), ref2->peer_ref->name, ref1->name, ref2->name); free(ref2->peer_ref); free(ref2); -- cgit v1.2.1 From f096e6e826678c29e4bfde4d9d1ae1df79074ce3 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 30 Oct 2013 06:33:12 +0100 Subject: fetch: improve the error messages emitted for conflicting refspecs If we find two refspecs that want to update the same local reference, emit an error message that is more informative based on whether one of the conflicting refspecs is an opportunistic update during a fetch with explicit command-line refspecs. And especially, do not die if an opportunistic reference update conflicts with an express wish of the user; rather, just emit a warning and skip the opportunistic reference update. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- remote.c | 25 ++++++++++++++++++++++--- t/t5536-fetch-conflicts.sh | 14 +++++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/remote.c b/remote.c index 0848faffa..dc56619d2 100644 --- a/remote.c +++ b/remote.c @@ -747,9 +747,28 @@ int for_each_remote(each_remote_fn fn, void *priv) static void handle_duplicate(struct ref *ref1, struct ref *ref2) { - if (strcmp(ref1->name, ref2->name)) - die(_("%s tracks both %s and %s"), - ref2->peer_ref->name, ref1->name, ref2->name); + if (strcmp(ref1->name, ref2->name)) { + if (ref1->fetch_head_status != FETCH_HEAD_IGNORE && + ref2->fetch_head_status != FETCH_HEAD_IGNORE) { + die(_("Cannot fetch both %s and %s to %s"), + ref1->name, ref2->name, ref2->peer_ref->name); + } else if (ref1->fetch_head_status != FETCH_HEAD_IGNORE && + ref2->fetch_head_status == FETCH_HEAD_IGNORE) { + warning(_("%s usually tracks %s, not %s"), + ref2->peer_ref->name, ref2->name, ref1->name); + } else if (ref1->fetch_head_status == FETCH_HEAD_IGNORE && + ref2->fetch_head_status == FETCH_HEAD_IGNORE) { + die(_("%s tracks both %s and %s"), + ref2->peer_ref->name, ref1->name, ref2->name); + } else { + /* + * This last possibility doesn't occur because + * FETCH_HEAD_IGNORE entries always appear at + * the end of the list. + */ + die(_("Internal error")); + } + } free(ref2->peer_ref); free(ref2); } diff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh index 679c53e04..6c5d3a4ce 100755 --- a/t/t5536-fetch-conflicts.sh +++ b/t/t5536-fetch-conflicts.sh @@ -22,7 +22,7 @@ verify_stderr () { cat >expected && # We're not interested in the error # "fatal: The remote end hung up unexpectedly": - grep -v "hung up" actual && + grep -E '^(fatal|warning):' actual | sort && test_cmp expected actual } @@ -49,7 +49,7 @@ test_expect_success 'fetch conflict: config vs. config' ' cd ccc && test_must_fail git fetch origin 2>error && verify_stderr <<-\EOF - fatal: refs/remotes/origin/branch1 tracks both refs/heads/branch1 and refs/heads/branch2 + fatal: Cannot fetch both refs/heads/branch1 and refs/heads/branch2 to refs/remotes/origin/branch1 EOF ) ' @@ -78,18 +78,22 @@ test_expect_success 'fetch conflict: arg vs. arg' ' refs/heads/*:refs/remotes/origin/* \ refs/heads/branch2:refs/remotes/origin/branch1 2>error && verify_stderr <<-\EOF - fatal: refs/remotes/origin/branch1 tracks both refs/heads/branch1 and refs/heads/branch2 + fatal: Cannot fetch both refs/heads/branch1 and refs/heads/branch2 to refs/remotes/origin/branch1 EOF ) ' -test_expect_failure 'fetch conflict: criss-cross args' ' +test_expect_success 'fetch conflict: criss-cross args' ' setup_repository xaa \ "+refs/heads/*:refs/remotes/origin/*" && ( cd xaa && git fetch origin \ refs/heads/branch1:refs/remotes/origin/branch2 \ - refs/heads/branch2:refs/remotes/origin/branch1 + refs/heads/branch2:refs/remotes/origin/branch1 2>error && + verify_stderr <<-\EOF + warning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2 + warning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1 + EOF ) ' -- cgit v1.2.1