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.<name>.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 <remote> '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 <mhagger@xxxxxxxxxxxx> --- The change to builtin/fetch.c:get_ref_map() has the side effect of changing the order that the (struct ref) objects are listed in ref_map. It seems to me that this could probably only matter in the case of duplicates. But later ref_remove_duplicates() is called, so duplicates are eliminated. However, ref_remove_duplicates() always retains the first instance of duplicates and discards the rest. It is conceivable that the boolean flags (which are not inspected by ref_remove_duplicates()) could differ among the duplicates, and that therefore changing the order of the items in the original list has the effect of changing the flags on the items that are retained. I don't know this code well enough to judge whether this might be a problem. If it is, then the correct approach is probably either to teach ref_remove_duplicates() to ensure that the flags are also consistent across duplicates, or to somehow combine the flags from all duplicates into the one that is retained. Please let me know if this is needed. Documentation/fetch-options.txt | 8 +++--- builtin/fetch.c | 46 +++++++++++++++++++------------- 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, 71 insertions(+), 32 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index ba1fe49..0e6d2ac 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. + This is a short-hand requesting that all tags be fetched from + the remote in addition to whatever else is being fetched. It + is similar to using 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 61e8117..7edb1ea 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -271,7 +271,7 @@ 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) { + if (refspec_count) { struct ref **old_tail; for (i = 0; i < refspec_count; i++) { @@ -279,11 +279,9 @@ static struct ref *get_ref_map(struct transport *transport, 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 @@ -334,8 +332,13 @@ static struct ref *get_ref_map(struct transport *transport, tail = &ref_map->next; } } - 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); + ref_remove_duplicates(ref_map); return ref_map; @@ -826,31 +829,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 b946fd9..dac7e1c 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 tags; you probably meant:" echo " git fetch --tags" exit 1 esac diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 8328be1..02e5901 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 1669cc4..0f70f66 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 8a74935..ab473a6 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 4fbf7a1..45815f7 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 -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html