{"thread":{"id":"28685","subject":"[PATCHv4 0/5] Be more careful when prunning","startedAt":"2011-10-15T05:04:21Z","lastAt":"2011-10-16T05:10:11Z","messageCount":8,"participants":["Carlos Martín Nieto","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"177704","messageId":"1318655066-29001-1-git-send-email-cmn@elego.de","threadId":"28685","inReplyTo":null,"subject":"[PATCHv4 0/5] Be more careful when prunning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-15T05:04:21Z","receivedAt":"2011-10-15T05:04:21Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Hello,\n\nThis version covers Junio's concerns about code and functionality\nduplication with the third patch. The order of arguments for\nget_stale_heads is not changed anymore in the fourth patch.\n\nThe first patch is not that big a deal, but it's better if we're\nfreeing the refspecs, we might as well free all of them.\n\nThe second patch introduces expected failures for the features that\nthis series fixes.\n\nThe third patch is new and moves the logic out of remote_find_tracking\ninto a query_refspecs function. remote_find_tracking and\napply_refspecs are both changed to be wrappers around this\nfunction. This way, the logic is in one function instead of three\ndifferent ones.\n\nThe fourth patch changes prune_refs and get_stale_heads so the caller\nhas to decide which refspecs are the appropriate ones to use. For\nexample, running\n\n    git fetch --prune origin refs/heads/master:refs/heads/master\n\ndoesn't remove the other branches anymore. For a more interesting (and\nbelievable) example, let's take\n\n    git fetch --prune origin refs/heads/b/*:refs/heads/b/*\n\nbecause you want to prune the refs inside the b/ namespace\nonly. Currently git will delete all the refs that aren't under that\nnamespace. With the second patch applied, git won't remove any refs\noutside the b/ namespace.\n\nWhat is probably the most usual case is covered by the fifth patch,\nwhich pretends that a \"refs/tags/*:refs/tags/*\" refspec was given on\nthe command-line. That fixes the\n\n    git fetch --prune --tags origin\n\ncase. The non-tag refs are kept now.\n\nCheers,\n   cmn\n\nCarlos Martín Nieto (5):\n  fetch: free all the additional refspecs\n  t5510: add tests for fetch --prune\n  remote: separate out the remote_find_tracking logic into\n    query_refspecs\n  fetch: honor the user-provided refspecs when pruning refs\n  fetch: treat --tags like refs/tags/*:refs/tags/* when pruning\n\n builtin/fetch.c  |   33 ++++++++++++++---\n builtin/remote.c |    3 +-\n remote.c         |  106 +++++++++++++++++++++++++++++------------------------\n remote.h         |    2 +-\n t/t5510-fetch.sh |   50 +++++++++++++++++++++++++\n 5 files changed, 139 insertions(+), 55 deletions(-)\n\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177709","messageId":"1318655066-29001-2-git-send-email-cmn@elego.de","threadId":"28685","inReplyTo":"1318655066-29001-1-git-send-email-cmn@elego.de","subject":"[PATCH 1/5] fetch: free all the additional refspecs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-15T05:04:22Z","receivedAt":"2011-10-15T05:04:22Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/fetch.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e422ced..605d1bf 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -918,7 +918,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)\n \tatexit(unlock_pack);\n \trefspec = parse_fetch_refspec(ref_nr, refs);\n \texit_code = do_fetch(transport, refspec, ref_nr);\n-\tfree(refspec);\n+\tfree_refspec(ref_nr, refspec);\n \ttransport_disconnect(transport);\n \ttransport = NULL;\n \treturn exit_code;\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177705","messageId":"1318655066-29001-3-git-send-email-cmn@elego.de","threadId":"28685","inReplyTo":"1318655066-29001-1-git-send-email-cmn@elego.de","subject":"[PATCH 2/5] t5510: add tests for fetch --prune","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-15T05:04:23Z","receivedAt":"2011-10-15T05:04:23Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"The failures will be fixed in later commits.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5510-fetch.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 50 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 7e433b1..8b5e925 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -76,6 +76,56 @@ test_expect_success \"fetch test for-merge\" '\n \tcut -f -2 .git/FETCH_HEAD >actual &&\n \ttest_cmp expected actual'\n \n+test_expect_success 'fetch --prune on its own works as expected' '\n+\tcd \"$D\" &&\n+\tgit clone . prune &&\n+\tcd prune &&\n+\tgit fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&\n+\n+\tgit fetch --prune origin &&\n+\ttest_must_fail git rev-parse origin/extrabranch\n+'\n+\n+test_expect_failure 'fetch --prune with a branch name keeps branches' '\n+\tcd \"$D\" &&\n+\tgit clone . prune-branch &&\n+\tcd prune-branch &&\n+\tgit fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&\n+\n+\tgit fetch --prune origin master &&\n+\tgit rev-parse origin/extrabranch\n+'\n+\n+test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '\n+\tcd \"$D\" &&\n+\tgit clone . prune-namespace &&\n+\tcd prune-namespace &&\n+\n+\tgit fetch --prune origin refs/heads/a/*:refs/remotes/origin/a/* &&\n+\tgit rev-parse origin/master\n+'\n+\n+test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '\n+\tcd \"$D\" &&\n+\tgit clone . prune-tags &&\n+\tcd prune-tags &&\n+\tgit fetch origin refs/heads/master:refs/tags/sometag &&\n+\n+\tgit fetch --prune --tags origin &&\n+\tgit rev-parse origin/master &&\n+\ttest_must_fail git rev-parse somebranch\n+'\n+\n+test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '\n+\tcd \"$D\" &&\n+\tgit clone . prune-tags-branch &&\n+\tcd prune-tags-branch &&\n+\tgit fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&\n+\n+\tgit fetch --prune --tags origin master &&\n+\tgit rev-parse origin/extrabranch\n+'\n+\n test_expect_success 'fetch tags when there is no tags' '\n \n     cd \"$D\" &&\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177708","messageId":"1318655066-29001-4-git-send-email-cmn@elego.de","threadId":"28685","inReplyTo":"1318655066-29001-1-git-send-email-cmn@elego.de","subject":"[PATCH 3/5] remote: separate out the remote_find_tracking logic into query_refspecs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-15T05:04:24Z","receivedAt":"2011-10-15T05:04:24Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Move the body of remote_find_tracking to a new function query_refspecs\nwhich does the same (find a refspec that matches and apply the\ntransformation) but explicitely wants the list of refspecs.\n\nMake remote_find_tracking and apply_refspecs use query_refspecs.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n remote.c |   70 ++++++++++++++++++++++++++++++-------------------------------\n 1 files changed, 34 insertions(+), 36 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex b8ecfa5..e94c6d2 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -828,59 +828,57 @@ static int match_name_with_pattern(const char *key, const char *name,\n \treturn ret;\n }\n \n-char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\n-\t\t     const char *name)\n+static int query_refspecs(struct refspec *refs, int ref_count, struct refspec *query)\n {\n \tint i;\n-\tchar *ret = NULL;\n-\tfor (i = 0; i < nr_refspec; i++) {\n-\t\tstruct refspec *refspec = refspecs + i;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(refspec->src, name,\n-\t\t\t\t\t\t    refspec->dst, &ret))\n-\t\t\t\treturn ret;\n-\t\t} else if (!strcmp(refspec->src, name))\n-\t\t\treturn strdup(refspec->dst);\n-\t}\n-\treturn NULL;\n-}\n+\tint find_src = query->src == NULL;\n \n-int remote_find_tracking(struct remote *remote, struct refspec *refspec)\n-{\n-\tint find_src = refspec->src == NULL;\n-\tchar *needle, **result;\n-\tint i;\n+\tif (find_src && !query->dst)\n+\t\treturn error(\"query_refspecs: need either src or dst\");\n \n-\tif (find_src) {\n-\t\tif (!refspec->dst)\n-\t\t\treturn error(\"find_tracking: need either src or dst\");\n-\t\tneedle = refspec->dst;\n-\t\tresult = &refspec->src;\n-\t} else {\n-\t\tneedle = refspec->src;\n-\t\tresult = &refspec->dst;\n-\t}\n+\tfor (i = 0; i < ref_count; i++) {\n+\t\tstruct refspec *refspec = &refs[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\t\tconst char *needle = find_src ? query->dst : query->src;\n+\t\tchar **result = find_src ? &query->src : &query->dst;\n \n-\tfor (i = 0; i < remote->fetch_refspec_nr; i++) {\n-\t\tstruct refspec *fetch = &remote->fetch[i];\n-\t\tconst char *key = find_src ? fetch->dst : fetch->src;\n-\t\tconst char *value = find_src ? fetch->src : fetch->dst;\n-\t\tif (!fetch->dst)\n+\t\tif (!refspec->dst)\n \t\t\tcontinue;\n-\t\tif (fetch->pattern) {\n+\t\tif (refspec->pattern) {\n \t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n-\t\t\t\trefspec->force = fetch->force;\n+\t\t\t\tquery->force = refspec->force;\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t} else if (!strcmp(needle, key)) {\n \t\t\t*result = xstrdup(value);\n-\t\t\trefspec->force = fetch->force;\n+\t\t\tquery->force = refspec->force;\n \t\t\treturn 0;\n \t\t}\n \t}\n+\n \treturn -1;\n }\n \n+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\n+\t\t     const char *name)\n+{\n+\tstruct refspec query;\n+\n+\tmemset(&query, 0x0, sizeof(struct refspec));\n+\tquery.src = (char *) name;\n+\n+\tif (query_refspecs(refspecs, nr_refspec, &query))\n+\t\treturn NULL;\n+\n+\treturn query.dst;\n+}\n+\n+int remote_find_tracking(struct remote *remote, struct refspec *refspec)\n+{\n+\treturn query_refspecs(remote->fetch, remote->fetch_refspec_nr, refspec);\n+}\n+\n static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,\n \t\tconst char *name)\n {\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177706","messageId":"1318655066-29001-5-git-send-email-cmn@elego.de","threadId":"28685","inReplyTo":"1318655066-29001-1-git-send-email-cmn@elego.de","subject":"[PATCH 4/5] fetch: honor the user-provided refspecs when pruning refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-15T05:04:25Z","receivedAt":"2011-10-15T05:04:25Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"If the user gave us refspecs on the command line, we should use those\nwhen deciding whether to prune a ref instead of relying on the\nrefspecs in the config.\n\nPreviously, running\n\n    git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n\nwould delete every other ref under the origin namespace because we\nwere using the refspec to filter the available refs but using the\nconfigured refspec to figure out if a ref had been deleted on the\nremote. This is clearly the wrong thing to do.\n\nChange prune_refs and get_stale_heads to simply accept a list of\nreferences and a list of refspecs. The caller of either function needs\nto decide what refspecs should be used to decide whether a ref is\nstale.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n builtin/fetch.c  |   12 ++++++++----\n builtin/remote.c |    3 ++-\n remote.c         |   36 ++++++++++++++++++++++++------------\n remote.h         |    2 +-\n t/t5510-fetch.sh |    4 ++--\n 5 files changed, 37 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 605d1bf..e295d97 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -540,10 +540,10 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \treturn ret;\n }\n \n-static int prune_refs(struct transport *transport, struct ref *ref_map)\n+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n {\n \tint result = 0;\n-\tstruct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);\n+\tstruct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);\n \tconst char *dangling_msg = dry_run\n \t\t? _(\"   (%s will become dangling)\\n\")\n \t\t: _(\"   (%s has become dangling)\\n\");\n@@ -734,8 +734,12 @@ static int do_fetch(struct transport *transport,\n \t\tfree_refs(ref_map);\n \t\treturn 1;\n \t}\n-\tif (prune)\n-\t\tprune_refs(transport, ref_map);\n+\tif (prune) {\n+\t\tif (ref_count)\n+\t\t\tprune_refs(refs, ref_count, ref_map);\n+\t\telse\n+\t\t\tprune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);\n+\t}\n \tfree_refs(ref_map);\n \n \t/* if neither --no-tags nor --tags was specified, do automated tag\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex f2a9c26..de52367 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -349,7 +349,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat\n \t\telse\n \t\t\tstring_list_append(&states->tracked, abbrev_branch(ref->name));\n \t}\n-\tstale_refs = get_stale_heads(states->remote, fetch_map);\n+\tstale_refs = get_stale_heads(states->remote->fetch,\n+\t\t\t\t     states->remote->fetch_refspec_nr, fetch_map);\n \tfor (ref = stale_refs; ref; ref = ref->next) {\n \t\tstruct string_list_item *item =\n \t\t\tstring_list_append(&states->stale, abbrev_branch(ref->name));\ndiff --git a/remote.c b/remote.c\nindex e94c6d2..069b1ff 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1679,36 +1679,48 @@ struct ref *guess_remote_head(const struct ref *head,\n }\n \n struct stale_heads_info {\n-\tstruct remote *remote;\n \tstruct string_list *ref_names;\n \tstruct ref **stale_refs_tail;\n+\tstruct refspec *refs;\n+\tint ref_count;\n };\n \n static int get_stale_heads_cb(const char *refname,\n \tconst unsigned char *sha1, int flags, void *cb_data)\n {\n \tstruct stale_heads_info *info = cb_data;\n-\tstruct refspec refspec;\n-\tmemset(&refspec, 0, sizeof(refspec));\n-\trefspec.dst = (char *)refname;\n-\tif (!remote_find_tracking(info->remote, &refspec)) {\n-\t\tif (!((flags & REF_ISSYMREF) ||\n-\t\t    string_list_has_string(info->ref_names, refspec.src))) {\n-\t\t\tstruct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);\n-\t\t\thashcpy(ref->new_sha1, sha1);\n-\t\t}\n+\tstruct refspec query;\n+\tmemset(&query, 0, sizeof(struct refspec));\n+\tquery.dst = (char *)refname;\n+\n+\tif (query_refspecs(info->refs, info->ref_count, &query))\n+\t    return 0; /* No matches */\n+\n+\n+\t/*\n+\t * If we did find a suitable refspec and it's not a symref and\n+\t * it's not in the list of refs that currently exist in that\n+\t * remote we consider it to be stale.\n+\t */\n+\tif (!((flags & REF_ISSYMREF) ||\n+\t      string_list_has_string(info->ref_names, query.src))) {\n+\t\tstruct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);\n+\t\thashcpy(ref->new_sha1, sha1);\n \t}\n+\n+\tfree(query.src);\n \treturn 0;\n }\n \n-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)\n+struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fetch_map)\n {\n \tstruct ref *ref, *stale_refs = NULL;\n \tstruct string_list ref_names = STRING_LIST_INIT_NODUP;\n \tstruct stale_heads_info info;\n-\tinfo.remote = remote;\n \tinfo.ref_names = &ref_names;\n \tinfo.stale_refs_tail = &stale_refs;\n+\tinfo.refs = refs;\n+\tinfo.ref_count = ref_count;\n \tfor (ref = fetch_map; ref; ref = ref->next)\n \t\tstring_list_append(&ref_names, ref->name);\n \tsort_string_list(&ref_names);\ndiff --git a/remote.h b/remote.h\nindex 9a30a9d..f2541b5 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -164,6 +164,6 @@ struct ref *guess_remote_head(const struct ref *head,\n \t\t\t      int all);\n \n /* Return refs which no longer exist on remote */\n-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);\n+struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fetch_map);\n \n #endif\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 8b5e925..581049b 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -86,7 +86,7 @@ test_expect_success 'fetch --prune on its own works as expected' '\n \ttest_must_fail git rev-parse origin/extrabranch\n '\n \n-test_expect_failure 'fetch --prune with a branch name keeps branches' '\n+test_expect_success 'fetch --prune with a branch name keeps branches' '\n \tcd \"$D\" &&\n \tgit clone . prune-branch &&\n \tcd prune-branch &&\n@@ -96,7 +96,7 @@ test_expect_failure 'fetch --prune with a branch name keeps branches' '\n \tgit rev-parse origin/extrabranch\n '\n \n-test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '\n+test_expect_success 'fetch --prune with a namespace keeps other namespaces' '\n \tcd \"$D\" &&\n \tgit clone . prune-namespace &&\n \tcd prune-namespace &&\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177707","messageId":"1318655066-29001-6-git-send-email-cmn@elego.de","threadId":"28685","inReplyTo":"1318655066-29001-1-git-send-email-cmn@elego.de","subject":"[PATCH 5/5] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-15T05:04:26Z","receivedAt":"2011-10-15T05:04:26Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"If --tags is specified, add that refspec to the list given to\nprune_refs so it knows to treat it as a filter on what refs to\nshould consider for prunning. This way\n\n    git fetch --prune --tags origin\n\nonly prunes tags and doesn't delete the branch refs.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n builtin/fetch.c  |   23 +++++++++++++++++++++--\n t/t5510-fetch.sh |    4 ++--\n 2 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e295d97..9d481f8 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -735,10 +735,29 @@ static int do_fetch(struct transport *transport,\n \t\treturn 1;\n \t}\n \tif (prune) {\n-\t\tif (ref_count)\n+\t\t/* If --tags was specified, pretend the user gave us the canonical tags refspec */\n+\t\tif (tags == TAGS_SET) {\n+\t\t\tconst char *tags_str = \"refs/tags/*:refs/tags/*\";\n+\t\t\tstruct refspec *tags_refspec, *refspec;\n+\n+\t\t\t/* Copy the refspec and add the tags to it */\n+\t\t\trefspec = xcalloc(ref_count + 1, sizeof(struct refspec));\n+\t\t\ttags_refspec = parse_fetch_refspec(1, &tags_str);\n+\t\t\tmemcpy(refspec, refs, ref_count * sizeof(struct refspec));\n+\t\t\tmemcpy(&refspec[ref_count], tags_refspec, sizeof(struct refspec));\n+\t\t\tref_count++;\n+\n+\t\t\tprune_refs(refspec, ref_count, ref_map);\n+\n+\t\t\tref_count--;\n+\t\t\t/* The rest of the strings belong to fetch_one */\n+\t\t\tfree_refspec(1, tags_refspec);\n+\t\t\tfree(refspec);\n+\t\t} else if (ref_count) {\n \t\t\tprune_refs(refs, ref_count, ref_map);\n-\t\telse\n+\t\t} else {\n \t\t\tprune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);\n+\t\t}\n \t}\n \tfree_refs(ref_map);\n \ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 581049b..e0af4c4 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -105,7 +105,7 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' '\n \tgit rev-parse origin/master\n '\n \n-test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '\n+test_expect_success 'fetch --prune --tags does not delete the remote-tracking branches' '\n \tcd \"$D\" &&\n \tgit clone . prune-tags &&\n \tcd prune-tags &&\n@@ -116,7 +116,7 @@ test_expect_failure 'fetch --prune --tags does not delete the remote-tracking br\n \ttest_must_fail git rev-parse somebranch\n '\n \n-test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '\n+test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' '\n \tcd \"$D\" &&\n \tgit clone . prune-tags-branch &&\n \tcd prune-tags-branch &&\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177757","messageId":"7vobxhtugo.fsf@alter.siamese.dyndns.org","threadId":"28685","inReplyTo":"1318655066-29001-4-git-send-email-cmn@elego.de","subject":"Re: [PATCH 3/5] remote: separate out the remote_find_tracking logic into query_refspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-16T04:45:59Z","receivedAt":"2011-10-16T04:45:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto <cmn@elego.de> writes:\n\n> Move the body of remote_find_tracking to a new function query_refspecs\n> which does the same (find a refspec that matches and apply the\n> transformation) but explicitly wants the list of refspecs.\n>\n> Make remote_find_tracking and apply_refspecs use query_refspecs.\n>\n> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\n> ---\n>  remote.c |   70 ++++++++++++++++++++++++++++++-------------------------------\n>  1 files changed, 34 insertions(+), 36 deletions(-)\n\nLooks very sensible, especially knowing what you want to do in the next\npatch ;-).\n\nThanks.\n\n>\n> diff --git a/remote.c b/remote.c\n> index b8ecfa5..e94c6d2 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -828,59 +828,57 @@ static int match_name_with_pattern(const char *key, const char *name,\n>  \treturn ret;\n>  }\n>  \n> -char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\n> -\t\t     const char *name)\n> +static int query_refspecs(struct refspec *refs, int ref_count, struct refspec *query)\n>  {\n>  \tint i;\n> -\tchar *ret = NULL;\n> -\tfor (i = 0; i < nr_refspec; i++) {\n> -\t\tstruct refspec *refspec = refspecs + i;\n> -\t\tif (refspec->pattern) {\n> -\t\t\tif (match_name_with_pattern(refspec->src, name,\n> -\t\t\t\t\t\t    refspec->dst, &ret))\n> -\t\t\t\treturn ret;\n> -\t\t} else if (!strcmp(refspec->src, name))\n> -\t\t\treturn strdup(refspec->dst);\n> -\t}\n> -\treturn NULL;\n> -}\n> +\tint find_src = query->src == NULL;\n>  \n> -int remote_find_tracking(struct remote *remote, struct refspec *refspec)\n> -{\n> -\tint find_src = refspec->src == NULL;\n> -\tchar *needle, **result;\n> -\tint i;\n> +\tif (find_src && !query->dst)\n> +\t\treturn error(\"query_refspecs: need either src or dst\");\n>  \n> -\tif (find_src) {\n> -\t\tif (!refspec->dst)\n> -\t\t\treturn error(\"find_tracking: need either src or dst\");\n> -\t\tneedle = refspec->dst;\n> -\t\tresult = &refspec->src;\n> -\t} else {\n> -\t\tneedle = refspec->src;\n> -\t\tresult = &refspec->dst;\n> -\t}\n> +\tfor (i = 0; i < ref_count; i++) {\n> +\t\tstruct refspec *refspec = &refs[i];\n> +\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n> +\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n> +\t\tconst char *needle = find_src ? query->dst : query->src;\n> +\t\tchar **result = find_src ? &query->src : &query->dst;\n>  \n> -\tfor (i = 0; i < remote->fetch_refspec_nr; i++) {\n> -\t\tstruct refspec *fetch = &remote->fetch[i];\n> -\t\tconst char *key = find_src ? fetch->dst : fetch->src;\n> -\t\tconst char *value = find_src ? fetch->src : fetch->dst;\n> -\t\tif (!fetch->dst)\n> +\t\tif (!refspec->dst)\n>  \t\t\tcontinue;\n> -\t\tif (fetch->pattern) {\n> +\t\tif (refspec->pattern) {\n>  \t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n> -\t\t\t\trefspec->force = fetch->force;\n> +\t\t\t\tquery->force = refspec->force;\n>  \t\t\t\treturn 0;\n>  \t\t\t}\n>  \t\t} else if (!strcmp(needle, key)) {\n>  \t\t\t*result = xstrdup(value);\n> -\t\t\trefspec->force = fetch->force;\n> +\t\t\tquery->force = refspec->force;\n>  \t\t\treturn 0;\n>  \t\t}\n>  \t}\n> +\n>  \treturn -1;\n>  }\n>  \n> +char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\n> +\t\t     const char *name)\n> +{\n> +\tstruct refspec query;\n> +\n> +\tmemset(&query, 0x0, sizeof(struct refspec));\n> +\tquery.src = (char *) name;\n> +\n> +\tif (query_refspecs(refspecs, nr_refspec, &query))\n> +\t\treturn NULL;\n> +\n> +\treturn query.dst;\n> +}\n> +\n> +int remote_find_tracking(struct remote *remote, struct refspec *refspec)\n> +{\n> +\treturn query_refspecs(remote->fetch, remote->fetch_refspec_nr, refspec);\n> +}\n> +\n>  static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,\n>  \t\tconst char *name)\n>  {\n"},{"id":"177758","messageId":"7vhb39ttcc.fsf@alter.siamese.dyndns.org","threadId":"28685","inReplyTo":"7vobxhtugo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/5] remote: separate out the remote_find_tracking logic into query_refspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-16T05:10:11Z","receivedAt":"2011-10-16T05:10:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Carlos Martín Nieto <cmn@elego.de> writes:\n>\n>> Move the body of remote_find_tracking to a new function query_refspecs\n>> which does the same (find a refspec that matches and apply the\n>> transformation) but explicitly wants the list of refspecs.\n>>\n>> Make remote_find_tracking and apply_refspecs use query_refspecs.\n>>\n>> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\n>> ---\n>>  remote.c |   70 ++++++++++++++++++++++++++++++-------------------------------\n>>  1 files changed, 34 insertions(+), 36 deletions(-)\n>\n> Looks very sensible, especially knowing what you want to do in the next\n> patch ;-).\n>\n> Thanks.\n\nI notice that before this update, passing a refspec[] with an element that\nlacks its dst side to apply_refspecs() would have segfaulted but with this\nupdate such an element in the refspec[] will simply be ignored.\n\nThis helper function was added in 72ff894 (Allow helper to map private ref\nnames into normal names, 2009-11-18) for transport-helper's use. I am sure\nthe change will not introduce a regression due to this skippage (the code\nwithout this patch would have simply crashed with such an input anyway),\nbut I thought people involved in the transport layer may want to know.\n"}]}