{"thread":{"id":"28616","subject":"[PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs","startedAt":"2011-10-06T20:19:42Z","lastAt":"2011-10-07T16:40:01Z","messageCount":15,"participants":["Carlos Martín Nieto","Ben Boeckel","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"177101","messageId":"1317932385-28604-1-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":null,"subject":"[PATCH 0/3] Be more careful when prunning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T20:19:42Z","receivedAt":"2011-10-06T20:19:42Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Hello,\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 teaches get_stale_heads to use the user-provided\nrefspecs instead of the ones in the config. For example, 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 third patch,\nwhich pretends that a \"refs/tags/*:refs/tags/*\" refspec was given on\nthe command-line.\n\nCheers,\n cmn\n\nCarlos Martín Nieto (3):\n fetch: free all the additional 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 | 19 ++++++++++---\n builtin/remote.c | 2 +-\n remote.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-----\n remote.h | 3 +-\n 4 files changed, 84 insertions(+), 14 deletions(-)\n\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177103","messageId":"1317932385-28604-2-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"1317932385-28604-1-git-send-email-cmn@elego.de","subject":"[PATCH 1/3] fetch: free all the additional refspecs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T20:19:43Z","receivedAt":"2011-10-06T20:19:43Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Signed-off-by: Carlos Martín Nieto \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 7a4e41c..30b485e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -883,7 +883,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":"177100","messageId":"1317932385-28604-3-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"1317932385-28604-1-git-send-email-cmn@elego.de","subject":"[PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T20:19:44Z","receivedAt":"2011-10-06T20:19:44Z","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 tag 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\nTeach get_stale_heads about user-provided refspecs and use them if\nthey're available.\n\nSigned-off-by: Carlos Martín Nieto \n---\n builtin/fetch.c | 6 ++--\n builtin/remote.c | 2 +-\n remote.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-----\n remote.h | 3 +-\n 4 files changed, 73 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 30b485e..b937d71 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -505,10 +505,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 transport *transport, struct refspec *refs, int n, 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(transport->remote, ref_map, refs, n);\n \tconst char *dangling_msg = dry_run\n \t\t? _(\" (%s will become dangling)\\n\")\n \t\t: _(\" (%s has become dangling)\\n\");\n@@ -700,7 +700,7 @@ static int do_fetch(struct transport *transport,\n \t\treturn 1;\n \t}\n \tif (prune)\n-\t\tprune_refs(transport, ref_map);\n+\t\tprune_refs(transport, refs, ref_count, ref_map);\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 b25dfb4..91a2148 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -349,7 +349,7 @@ 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_map, NULL, 0);\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 7840d2f..72a26d3 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1684,26 +1684,84 @@ 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+/*\n+ * Find a refspec to a remote's\n+ * Returns 0 on success, -1 if it couldn't find a the refspec\n+ */\n+static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)\n+{\n+\tint i;\n+\tstruct refspec *refspec;\n+\n+\tfor (i = 0; i < ref_count; ++i) {\n+\t\trefspec = &refs[i];\n+\n+\t\t/*\n+\t\t * No '*' means that it must match exactly. If it does\n+\t\t * have it, try to match it against the pattern. If\n+\t\t * the refspec matches, store the ref name as it would\n+\t\t * appear in the server in query->src.\n+\t\t */\n+\t\tif (!strchr(refspec->dst, '*')) {\n+\t\t\tif (!strcmp(query->dst, refspec->dst)) {\n+\t\t\t\tquery->src = xstrdup(refspec->src);\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (match_name_with_pattern(refspec->dst, query->dst,\n+\t\t\t\t\t\t refspec->src, &query->src)) {\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\treturn -1;\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+\tint ret;\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+\n+\t/*\n+\t * If the user speicified refspecs on the command line, we\n+\t * should only use those to check. Otherwise, look in the\n+\t * remote's configuration for the branch.\n+\t */\n+\tif (info->ref_count)\n+\t\tret = find_in_refs(info->refs, info->ref_count, &refspec);\n+\telse\n+\t\tret = remote_find_tracking(info->remote, &refspec);\n+\n+\t/* No matches */\n+\tif (ret)\n+\t\treturn 0;\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, refspec.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(refspec.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 remote *remote, struct ref *fetch_map,\n+\t\t\t struct refspec *refs, int ref_count)\n {\n \tstruct ref *ref, *stale_refs = NULL;\n \tstruct string_list ref_names = STRING_LIST_INIT_NODUP;\n@@ -1711,6 +1769,8 @@ struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)\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..2f753a0 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -164,6 +164,7 @@ 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 remote *remote, struct ref *fetch_map,\n+\t\t\t struct refspec *refs, int ref_count);\n \n #endif\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177102","messageId":"1317932385-28604-4-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"1317932385-28604-1-git-send-email-cmn@elego.de","subject":"[PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T20:19:45Z","receivedAt":"2011-10-06T20:19:45Z","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 prune_refs\nso it knows to treat it as a filter on what refs to should consider\nfor 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 \n---\n builtin/fetch.c | 11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex b937d71..94b2bd3 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,\n \t\tfree_refs(ref_map);\n \t\treturn 1;\n \t}\n-\tif (prune)\n+\tif (prune) {\n+\t\t/* If --tags was specified, we need to tell prune_refs\n+\t\t * that we're filtering the refs from the remote */\n+\t\tif (tags == TAGS_SET) {\n+\t\t\tconst char * tags_refspec = \"refs/tags/*:refs/tags/*\";\n+\t\t\trefs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));\n+\t\t\trefs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);\n+\t\t\tref_count++;\n+\t\t}\n \t\tprune_refs(transport, refs, ref_count, ref_map);\n+\t}\n \tfree_refs(ref_map);\n \n \t/* if neither --no-tags nor --tags was specified, do automated tag\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177107","messageId":"20111006205103.GA1271@erythro.kitwarein.com","threadId":"28616","inReplyTo":"1317932385-28604-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH 0/3] Be more careful when prunning","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2011-10-06T20:51:03Z","receivedAt":"2011-10-06T20:51:03Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"On Thu, Oct 06, 2011 at 22:19:42 +0200, Carlos Martín Nieto wrote:\n> The first patch is not that big a deal, but it's better if we're\n> freeing the refspecs, we might as well free all of them.\n> \n> The second patch teaches get_stale_heads to use the user-provided\n> refspecs instead of the ones in the config. For example, running\n> \n> git fetch --prune origin refs/heads/master:refs/heads/master\n> \n> doesn't remove the other branches anymore. For a more interesting (and\n> believable) example, let's take\n> \n> git fetch --prune origin refs/heads/b/*:refs/heads/b/*\n> \n> because you want to prune the refs inside the b/ namespace\n> only. Currently git will delete all the refs that aren't under that\n> namespace. With the second patch applied, git won't remove any refs\n> outside the b/ namespace.\n> \n> What is probably the most usual case is covered by the third patch,\n> which pretends that a \"refs/tags/*:refs/tags/*\" refspec was given on\n> the command-line.\n\nI applied the patches to current master (7f41b6b) and got a segfault\nwith:\n\n git fetch -p -t origin master\n\nIt does not happen with master.\n\nBacktrace:\n\n(gdb) bt\n#0 0x00007ffff7395d18 in __strchr_sse42 () from /lib64/libc.so.6\n#1 0x00000000004b2d39 in find_in_refs (query=0x7fffffffdb90, ref_count=2, refs=) at remote.c:1709\n#2 get_stale_heads_cb (refname=0x7a8f31 \"refs/heads/a/branch/name\", sha1=0x7a8f09 \"\\367\\343\\375C٩\\223u\\305OG\\233)z\\347X\\370\\333\\325\", , flags=0, cb_data=0x7fffffffdc50) at remote.c:1740\n#3 0x00000000004adf19 in do_for_each_ref (submodule=, base=0x4ea1c2 \"\", fn=0x4b2ca0 , trim=0, flags=0, cb_data=0x7fffffffdc50) at refs.c:684\n#4 0x00000000004b4249 in get_stale_heads (remote=, fetch_map=, refs=, ref_count=) at remote.c:1777\n#5 0x0000000000426cfb in prune_refs (ref_map=, n=, refs=, transport=0x78e040) at builtin/fetch.c:511\n#6 do_fetch (ref_count=, refs=, transport=0x78e040) at builtin/fetch.c:711\n#7 fetch_one (remote=, argc=, argv=) at builtin/fetch.c:894\n#8 0x0000000000427550 in cmd_fetch (argc=2, argv=0x7fffffffe070, prefix=0x0) at builtin/fetch.c:955\n#9 0x0000000000405084 in run_builtin (argv=0x7fffffffe070, argc=5, p=0x731b08) at git.c:308\n#10 handle_internal_command (argc=5, argv=0x7fffffffe070) at git.c:466\n#11 0x000000000040448b in run_argv (argv=0x7fffffffdf10, argcp=0x7fffffffdf1c) at git.c:512\n#12 main (argc=5, argv=0x7fffffffe070) at git.c:585\n\n--Ben\n"},{"id":"177109","messageId":"1317934704.4619.1.camel@centaur.lab.cmartin.tk","threadId":"28616","inReplyTo":"20111006205103.GA1271@erythro.kitwarein.com","subject":"Re: [PATCH 0/3] Be more careful when prunning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T20:58:24Z","receivedAt":"2011-10-06T20:58:24Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Thu, 2011-10-06 at 16:51 -0400, Ben Boeckel wrote:\n> On Thu, Oct 06, 2011 at 22:19:42 +0200, Carlos Martín Nieto wrote:\n> > The first patch is not that big a deal, but it's better if we're\n> > freeing the refspecs, we might as well free all of them.\n> > \n> > The second patch teaches get_stale_heads to use the user-provided\n> > refspecs instead of the ones in the config. For example, running\n> > \n> > git fetch --prune origin refs/heads/master:refs/heads/master\n> > \n> > doesn't remove the other branches anymore. For a more interesting (and\n> > believable) example, let's take\n> > \n> > git fetch --prune origin refs/heads/b/*:refs/heads/b/*\n> > \n> > because you want to prune the refs inside the b/ namespace\n> > only. Currently git will delete all the refs that aren't under that\n> > namespace. With the second patch applied, git won't remove any refs\n> > outside the b/ namespace.\n> > \n> > What is probably the most usual case is covered by the third patch,\n> > which pretends that a \"refs/tags/*:refs/tags/*\" refspec was given on\n> > the command-line.\n> \n> I applied the patches to current master (7f41b6b) and got a segfault\n> with:\n> \n> git fetch -p -t origin master\n> \n> It does not happen with master.\n\nI thought I'd got rid of those problems. Thanks for noticing. I'll\ninvestigate.\n\n> \n> Backtrace:\n> \n> (gdb) bt\n> #0 0x00007ffff7395d18 in __strchr_sse42 () from /lib64/libc.so.6\n> #1 0x00000000004b2d39 in find_in_refs (query=0x7fffffffdb90, ref_count=2, refs=) at remote.c:1709\n> #2 get_stale_heads_cb (refname=0x7a8f31 \"refs/heads/a/branch/name\", sha1=0x7a8f09 \"\\367\\343\\375C٩\\223u\\305OG\\233)z\\347X\\370\\333\\325\", , flags=0, cb_data=0x7fffffffdc50) at remote.c:1740\n> #3 0x00000000004adf19 in do_for_each_ref (submodule=, base=0x4ea1c2 \"\", fn=0x4b2ca0 , trim=0, flags=0, cb_data=0x7fffffffdc50) at refs.c:684\n> #4 0x00000000004b4249 in get_stale_heads (remote=, fetch_map=, refs=, ref_count=) at remote.c:1777\n> #5 0x0000000000426cfb in prune_refs (ref_map=, n=, refs=, transport=0x78e040) at builtin/fetch.c:511\n> #6 do_fetch (ref_count=, refs=, transport=0x78e040) at builtin/fetch.c:711\n> #7 fetch_one (remote=, argc=, argv=) at builtin/fetch.c:894\n> #8 0x0000000000427550 in cmd_fetch (argc=2, argv=0x7fffffffe070, prefix=0x0) at builtin/fetch.c:955\n> #9 0x0000000000405084 in run_builtin (argv=0x7fffffffe070, argc=5, p=0x731b08) at git.c:308\n> #10 handle_internal_command (argc=5, argv=0x7fffffffe070) at git.c:466\n> #11 0x000000000040448b in run_argv (argv=0x7fffffffdf10, argcp=0x7fffffffdf1c) at git.c:512\n> #12 main (argc=5, argv=0x7fffffffe070) at git.c:585\n> \n> --Ben\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n\n\n"},{"id":"177110","messageId":"1317936107-1230-1-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"20111006205103.GA1271@erythro.kitwarein.com","subject":"[PATCHv2 0/3] Be more careful when prunning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T21:21:44Z","receivedAt":"2011-10-06T21:21:44Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Hello,\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 teaches get_stale_heads to use the user-provided\nrefspecs instead of the ones in the config. For example, 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 third patch,\nwhich pretends that a \"refs/tags/*:refs/tags/*\" refspec was given on\nthe command-line.\n\nVersion 1 assumed that a refspec would have its dst filled\nautomatically. This is not the case and was fixed in the second patch.\n\nCheers,\n cmn\n\nCarlos Martín Nieto (3):\n fetch: free all the additional 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 | 19 ++++++++++---\n builtin/remote.c | 2 +-\n remote.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-----\n remote.h | 3 +-\n 4 files changed, 84 insertions(+), 14 deletions(-)\n\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177111","messageId":"1317936107-1230-2-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"1317936107-1230-1-git-send-email-cmn@elego.de","subject":"[PATCH 1/3] fetch: free all the additional refspecs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T21:21:45Z","receivedAt":"2011-10-06T21:21:45Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Signed-off-by: Carlos Martín Nieto \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 7a4e41c..30b485e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -883,7 +883,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":"177113","messageId":"1317936107-1230-3-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"1317936107-1230-1-git-send-email-cmn@elego.de","subject":"[PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T21:21:46Z","receivedAt":"2011-10-06T21:21:46Z","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 tag 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\nTeach get_stale_heads about user-provided refspecs and use them if\nthey're available.\n\nSigned-off-by: Carlos Martín Nieto \n---\n builtin/fetch.c | 6 ++--\n builtin/remote.c | 2 +-\n remote.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-----\n remote.h | 3 +-\n 4 files changed, 77 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 30b485e..b937d71 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -505,10 +505,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 transport *transport, struct refspec *refs, int n, 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(transport->remote, ref_map, refs, n);\n \tconst char *dangling_msg = dry_run\n \t\t? _(\" (%s will become dangling)\\n\")\n \t\t: _(\" (%s has become dangling)\\n\");\n@@ -700,7 +700,7 @@ static int do_fetch(struct transport *transport,\n \t\treturn 1;\n \t}\n \tif (prune)\n-\t\tprune_refs(transport, ref_map);\n+\t\tprune_refs(transport, refs, ref_count, ref_map);\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 b25dfb4..91a2148 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -349,7 +349,7 @@ 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_map, NULL, 0);\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 7840d2f..28f7917 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1684,26 +1684,88 @@ 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+/*\n+ * Find a refspec to a remote's\n+ * Returns 0 on success, -1 if it couldn't find a the refspec\n+ */\n+static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)\n+{\n+\tint i;\n+\tstruct refspec *refspec;\n+\n+\tfor (i = 0; i < ref_count; ++i) {\n+\t\trefspec = &refs[i];\n+\n+\t\t/* No dst means it can't be used for prunning. */\n+\t\tif (!refspec->dst)\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * No '*' means that it must match exactly. If it does\n+\t\t * have it, try to match it against the pattern. If\n+\t\t * the refspec matches, store the ref name as it would\n+\t\t * appear in the server in query->src.\n+\t\t */\n+\t\tif (!strchr(refspec->dst, '*')) {\n+\t\t\tif (!strcmp(query->dst, refspec->dst)) {\n+\t\t\t\tquery->src = xstrdup(refspec->src);\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (match_name_with_pattern(refspec->dst, query->dst,\n+\t\t\t\t\t\t refspec->src, &query->src)) {\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\treturn -1;\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+\tint ret;\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+\n+\t/*\n+\t * If the user speicified refspecs on the command line, we\n+\t * should only use those to check. Otherwise, look in the\n+\t * remote's configuration for the branch.\n+\t */\n+\tif (info->ref_count)\n+\t\tret = find_in_refs(info->refs, info->ref_count, &refspec);\n+\telse\n+\t\tret = remote_find_tracking(info->remote, &refspec);\n+\n+\t/* No matches */\n+\tif (ret)\n+\t\treturn 0;\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, refspec.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(refspec.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 remote *remote, struct ref *fetch_map,\n+\t\t\t struct refspec *refs, int ref_count)\n {\n \tstruct ref *ref, *stale_refs = NULL;\n \tstruct string_list ref_names = STRING_LIST_INIT_NODUP;\n@@ -1711,6 +1773,8 @@ struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)\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..2f753a0 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -164,6 +164,7 @@ 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 remote *remote, struct ref *fetch_map,\n+\t\t\t struct refspec *refs, int ref_count);\n \n #endif\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177112","messageId":"1317936107-1230-4-git-send-email-cmn@elego.de","threadId":"28616","inReplyTo":"1317936107-1230-1-git-send-email-cmn@elego.de","subject":"[PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-06T21:21:47Z","receivedAt":"2011-10-06T21:21:47Z","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 prune_refs\nso it knows to treat it as a filter on what refs to should consider\nfor 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 \n---\n builtin/fetch.c | 11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex b937d71..94b2bd3 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,\n \t\tfree_refs(ref_map);\n \t\treturn 1;\n \t}\n-\tif (prune)\n+\tif (prune) {\n+\t\t/* If --tags was specified, we need to tell prune_refs\n+\t\t * that we're filtering the refs from the remote */\n+\t\tif (tags == TAGS_SET) {\n+\t\t\tconst char * tags_refspec = \"refs/tags/*:refs/tags/*\";\n+\t\t\trefs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));\n+\t\t\trefs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);\n+\t\t\tref_count++;\n+\t\t}\n \t\tprune_refs(transport, refs, ref_count, ref_map);\n+\t}\n \tfree_refs(ref_map);\n \n \t/* if neither --no-tags nor --tags was specified, do automated tag\n-- \n1.7.5.2.354.g349bf\n"},{"id":"177160","messageId":"20111007162625.GB4399@sigill.intra.peff.net","threadId":"28616","inReplyTo":"1317936107-1230-3-git-send-email-cmn@elego.de","subject":"Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-07T16:26:26Z","receivedAt":"2011-10-07T16:26:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 06, 2011 at 11:21:46PM +0200, Carlos Martín Nieto wrote:\n\n> If the user gave us refspecs on the command line, we should use those\n> when deciding whether to prune a ref instead of relying on the\n> refspecs in the config.\n> \n> Previously, running\n> \n> git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> \n> would delete every other tag under the origin namespace because we\n\nI assume you mean s/tag/branch/ in the last line?\n\n> ---\n> builtin/fetch.c | 6 ++--\n> builtin/remote.c | 2 +-\n> remote.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-----\n> remote.h | 3 +-\n> 4 files changed, 77 insertions(+), 12 deletions(-)\n\nTests?\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> +\tint ret;\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> +\n> +\t/*\n> +\t * If the user speicified refspecs on the command line, we\n> +\t * should only use those to check. Otherwise, look in the\n> +\t * remote's configuration for the branch.\n> +\t */\n> +\tif (info->ref_count)\n> +\t\tret = find_in_refs(info->refs, info->ref_count, &refspec);\n> +\telse\n> +\t\tret = remote_find_tracking(info->remote, &refspec);\n\nMinor typo in the comment. But more importantly, this feels like a very\nlow-level place to be thinking about things like what the user gave us\non the command line.\n\nShouldn't get_stale_heads not get a remote at all, and just get a set of\nrefspecs? Those should be the minimal information it needs to get its\nanswer, right?\n\n-Peff\n"},{"id":"177161","messageId":"20111007163319.GC4399@sigill.intra.peff.net","threadId":"28616","inReplyTo":"1317936107-1230-4-git-send-email-cmn@elego.de","subject":"Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-07T16:33:19Z","receivedAt":"2011-10-07T16:33:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:\n\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index b937d71..94b2bd3 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,\n> \t\tfree_refs(ref_map);\n> \t\treturn 1;\n> \t}\n> -\tif (prune)\n> +\tif (prune) {\n> +\t\t/* If --tags was specified, we need to tell prune_refs\n> +\t\t * that we're filtering the refs from the remote */\n> +\t\tif (tags == TAGS_SET) {\n> +\t\t\tconst char * tags_refspec = \"refs/tags/*:refs/tags/*\";\n> +\t\t\trefs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));\n> +\t\t\trefs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);\n> +\t\t\tref_count++;\n> +\t\t}\n> \t\tprune_refs(transport, refs, ref_count, ref_map);\n> +\t}\n\nI don't think we can realloc refs here. It's passed into do_fetch. When\nwe realloc it, the old pointer value will be invalid. But when we return\nfrom do_fetch, the caller (fetch_one) will still have that old value,\nand will call free() on it.\n\nInstead, you have to make a whole new list, copy the old values in, add\nyour new one, and then free the result.\n\n-Peff\n"},{"id":"177162","messageId":"1318005433.4579.5.camel@centaur.lab.cmartin.tk","threadId":"28616","inReplyTo":"20111007162625.GB4399@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-07T16:37:13Z","receivedAt":"2011-10-07T16:37:13Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Fri, 2011-10-07 at 12:26 -0400, Jeff King wrote:\n> On Thu, Oct 06, 2011 at 11:21:46PM +0200, Carlos Martín Nieto wrote:\n> \n> > If the user gave us refspecs on the command line, we should use those\n> > when deciding whether to prune a ref instead of relying on the\n> > refspecs in the config.\n> > \n> > Previously, running\n> > \n> > git fetch --prune origin refs/heads/master:refs/remotes/origin/master\n> > \n> > would delete every other tag under the origin namespace because we\n> \n> I assume you mean s/tag/branch/ in the last line?\n\nYeah, maybe ref would be better?\n\n> \n> > ---\n> > builtin/fetch.c | 6 ++--\n> > builtin/remote.c | 2 +-\n> > remote.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-----\n> > remote.h | 3 +-\n> > 4 files changed, 77 insertions(+), 12 deletions(-)\n> \n> Tests?\n\nGood point.\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> > +\tint ret;\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> > +\n> > +\t/*\n> > +\t * If the user speicified refspecs on the command line, we\n> > +\t * should only use those to check. Otherwise, look in the\n> > +\t * remote's configuration for the branch.\n> > +\t */\n> > +\tif (info->ref_count)\n> > +\t\tret = find_in_refs(info->refs, info->ref_count, &refspec);\n> > +\telse\n> > +\t\tret = remote_find_tracking(info->remote, &refspec);\n> \n> Minor typo in the comment. But more importantly, this feels like a very\n> low-level place to be thinking about things like what the user gave us\n> on the command line.\n> \n> Shouldn't get_stale_heads not get a remote at all, and just get a set of\n> refspecs? Those should be the minimal information it needs to get its\n> answer, right?\n\nOK, so take a step back and figure out what we want the rules to be\nbefore we call get_stale_heads. It does sound like a more elegant\napproach. I was trying to disrupt the callers as little as possible, but\nthen again, there's only two. Will change.\n\n cmn\n\n"},{"id":"177163","messageId":"20111007163936.GA5212@sigill.intra.peff.net","threadId":"28616","inReplyTo":"1318005433.4579.5.camel@centaur.lab.cmartin.tk","subject":"Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-07T16:39:37Z","receivedAt":"2011-10-07T16:39:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 07, 2011 at 06:37:13PM +0200, Carlos Martín Nieto wrote:\n\n> > I assume you mean s/tag/branch/ in the last line?\n> \n> Yeah, maybe ref would be better?\n\nYeah, agreed.\n\n> > Tests?\n> \n> Good point.\n\nIt sounds like you already have a reproduction recipe for this, and for\nthe --tags thing in the next commit.\n\n> OK, so take a step back and figure out what we want the rules to be\n> before we call get_stale_heads. It does sound like a more elegant\n> approach. I was trying to disrupt the callers as little as possible, but\n> then again, there's only two. Will change.\n\nYeah. Sometimes we try hard to make a minimal patch, because it makes it\neasier to review. At the same time, I think it's more important to make\nthe code clean if it needs it. Especially when there aren't many callers\nto disrupt.\n\n-Peff\n"},{"id":"177164","messageId":"1318005601.4579.7.camel@centaur.lab.cmartin.tk","threadId":"28616","inReplyTo":"20111007163319.GC4399@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-10-07T16:40:01Z","receivedAt":"2011-10-07T16:40:01Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Fri, 2011-10-07 at 12:33 -0400, Jeff King wrote:\n> On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:\n> \n> > diff --git a/builtin/fetch.c b/builtin/fetch.c\n> > index b937d71..94b2bd3 100644\n> > --- a/builtin/fetch.c\n> > +++ b/builtin/fetch.c\n> > @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,\n> > \t\tfree_refs(ref_map);\n> > \t\treturn 1;\n> > \t}\n> > -\tif (prune)\n> > +\tif (prune) {\n> > +\t\t/* If --tags was specified, we need to tell prune_refs\n> > +\t\t * that we're filtering the refs from the remote */\n> > +\t\tif (tags == TAGS_SET) {\n> > +\t\t\tconst char * tags_refspec = \"refs/tags/*:refs/tags/*\";\n> > +\t\t\trefs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));\n> > +\t\t\trefs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);\n> > +\t\t\tref_count++;\n> > +\t\t}\n> > \t\tprune_refs(transport, refs, ref_count, ref_map);\n> > +\t}\n> \n> I don't think we can realloc refs here. It's passed into do_fetch. When\n> we realloc it, the old pointer value will be invalid. But when we return\n> from do_fetch, the caller (fetch_one) will still have that old value,\n> and will call free() on it.\n\nYes, you're right. I guess it's been working by luck and generous amount\nof memory.\n\n> \n> Instead, you have to make a whole new list, copy the old values in, add\n> your new one, and then free the result.\n\nWill do.\n\n cmn\n\n"}]}