{"thread":{"id":"35599","subject":"[PATCH V3 2/2] fetch --prune: Run prune before fetching","startedAt":"2014-01-03T02:28:51Z","lastAt":"2014-01-03T18:17:46Z","messageCount":3,"participants":["Tom Miller","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"232605","messageId":"1388716132-71437-1-git-send-email-jackerran@gmail.com","threadId":"35599","inReplyTo":null,"subject":"[PATCH V3 1/2] fetch --prune: Always print header url","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2014-01-03T02:28:51Z","receivedAt":"2014-01-03T02:28:51Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"If \"fetch --prune\" is run with no new refs to fetch, but it has refs\nto prune. Then, the header url is not printed as it would if there were\nnew refs to fetch.\n\nOutput before this patch:\n\n\t$ git fetch --prune remote-with-no-new-refs\n\t x [deleted]         (none)     -> origin/world\n\nOutput after this patch:\n\n\t$ git fetch --prune remote-with-no-new-refs\n\tFrom https://github.com/git/git\n\t x [deleted]         (none)     -> origin/test\n\nSigned-off-by: Tom Miller <jackerran@gmail.com>\n---\n\nI decided it is not worth writing a function to format the header url\nthat is printed by fetch. Instead, I will duplicate the formatting logic\nfor now because I plan on following up this patch set with another patch\nto stop striping the trailing \"/\" and \".git\" from the url. When that\npatch is finished it will remove the majority of the duplicated logic.\n\n builtin/fetch.c  | 32 +++++++++++++++++++++++++++-----\n t/t5510-fetch.sh | 12 ++++++++++++\n 2 files changed, 39 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 1e7d617..1b81cf9 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -44,6 +44,7 @@ static struct transport *gtransport;\n static struct transport *gsecondary;\n static const char *submodule_prefix = \"\";\n static const char *recurse_submodules_default;\n+static int shown_url = 0;\n \n static int option_parse_recurse_submodules(const struct option *opt,\n \t\t\t\t   const char *arg, int unset)\n@@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n {\n \tFILE *fp;\n \tstruct commit *commit;\n-\tint url_len, i, shown_url = 0, rc = 0;\n+\tint url_len, i, rc = 0;\n \tstruct strbuf note = STRBUF_INIT;\n \tconst char *what, *kind;\n \tstruct ref *rm;\n@@ -708,17 +709,36 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \treturn ret;\n }\n \n-static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,\n+\t\tconst char *raw_url)\n {\n-\tint result = 0;\n+\tint url_len, i, result = 0;\n \tstruct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);\n+\tchar *url;\n \tconst char *dangling_msg = dry_run\n \t\t? _(\"   (%s will become dangling)\")\n \t\t: _(\"   (%s has become dangling)\");\n \n+\tif (raw_url)\n+\t\turl = transport_anonymize_url(raw_url);\n+\telse\n+\t\turl = xstrdup(\"foreign\");\n+\n+\turl_len = strlen(url);\n+\tfor (i = url_len - 1; url[i] == '/' && 0 <= i; i--)\n+\t\t;\n+\n+\turl_len = i + 1;\n+\tif (4 < i && !strncmp(\".git\", url + i - 3, 4))\n+\t\turl_len = i - 3;\n+\n \tfor (ref = stale_refs; ref; ref = ref->next) {\n \t\tif (!dry_run)\n \t\t\tresult |= delete_ref(ref->name, NULL, 0);\n+\t\tif (verbosity >= 0 && !shown_url) {\n+\t\t\tfprintf(stderr, _(\"From %.*s\\n\"), url_len, url);\n+\t\t\tshown_url = 1;\n+\t\t}\n \t\tif (verbosity >= 0) {\n \t\t\tfprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n \t\t\t\tTRANSPORT_SUMMARY(_(\"[deleted]\")),\n@@ -726,6 +746,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n \t\t\twarn_dangling_symref(stderr, dangling_msg, ref->name);\n \t\t}\n \t}\n+\tfree(url);\n \tfree_refs(stale_refs);\n \treturn result;\n }\n@@ -854,11 +875,12 @@ static int do_fetch(struct transport *transport,\n \t\t * don't care whether --tags was specified.\n \t\t */\n \t\tif (ref_count) {\n-\t\t\tprune_refs(refs, ref_count, ref_map);\n+\t\t\tprune_refs(refs, ref_count, ref_map, transport->url);\n \t\t} else {\n \t\t\tprune_refs(transport->remote->fetch,\n \t\t\t\t   transport->remote->fetch_refspec_nr,\n-\t\t\t\t   ref_map);\n+\t\t\t\t   ref_map,\n+\t\t\t\t   transport->url);\n \t\t}\n \t}\n \tfree_refs(ref_map);\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 5d4581d..87e896d 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '\n \ttest_bundle_object_count .git/objects/pack/pack-${pack##pack\t}.pack 3\n '\n \n+test_expect_success 'fetch --prune prints the remotes url' '\n+\tgit branch goodbye &&\n+\tgit clone . only-prunes &&\n+\tgit branch -D goodbye &&\n+\t(\n+\t\tcd only-prunes &&\n+\t\tgit fetch --prune origin 2>&1 | head -n1 >../actual\n+\t) &&\n+\techo \"From ${D}/.\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.5.2.231.g4834e63.dirty\n"},{"id":"232604","messageId":"1388716132-71437-2-git-send-email-jackerran@gmail.com","threadId":"35599","inReplyTo":"1388716132-71437-1-git-send-email-jackerran@gmail.com","subject":"[PATCH V3 2/2] fetch --prune: Run prune before fetching","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2014-01-03T02:28:52Z","receivedAt":"2014-01-03T02:28:52Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"When we have a remote-tracking branch named \"frotz/nitfol\" from a\nprevious fetch, and the upstream now has a branch named \"frotz\". Prior\nto this patch fetch would fail to remove \"frotz/nitfol\" with a \"git\nfetch --prune\" from the upstream. git would inform the user to use \"git\nremote prune\" to fix the problem.\n\nThis patch changes the way \"fetch --prune\" works by moving the pruning\noperation before the fetching operation. Instead of warning the user of\na conflict, it autmatically fixes it.\n\nSigned-off-by: Tom Miller <jackerran@gmail.com>\nTested-by: Thomas Rast <tr@thomasrast.ch>\nAcked-by: Junio C Hamano <gitster@pobox.com>\n---\n\nI did change the commit message according to Junio's suggestion in the\nfirst patch.\n\n builtin/fetch.c  | 10 +++++-----\n t/t5510-fetch.sh | 14 ++++++++++++++\n 2 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 1b81cf9..09825c8 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -863,11 +863,6 @@ static int do_fetch(struct transport *transport,\n \n \tif (tags == TAGS_DEFAULT && autotags)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n-\tif (fetch_refs(transport, ref_map)) {\n-\t\tfree_refs(ref_map);\n-\t\tretcode = 1;\n-\t\tgoto cleanup;\n-\t}\n \tif (prune) {\n \t\t/*\n \t\t * We only prune based on refspecs specified\n@@ -883,6 +878,11 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t   transport->url);\n \t\t}\n \t}\n+\tif (fetch_refs(transport, ref_map)) {\n+\t\tfree_refs(ref_map);\n+\t\tretcode = 1;\n+\t\tgoto cleanup;\n+\t}\n \tfree_refs(ref_map);\n \n \t/* if neither --no-tags nor --tags was specified, do automated tag\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 87e896d..12674ac 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -626,4 +626,18 @@ test_expect_success 'fetch --prune prints the remotes url' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'branchname D/F conflict resolved by --prune' '\n+\tgit branch dir/file &&\n+\tgit clone . prune-df-conflict &&\n+\tgit branch -D dir/file &&\n+\tgit branch dir &&\n+\t(\n+\t\tcd prune-df-conflict &&\n+\t\tgit fetch --prune &&\n+\t\tgit rev-parse origin/dir >../actual\n+\t) &&\n+\tgit rev-parse dir >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.5.2.231.g4834e63.dirty\n"},{"id":"232624","messageId":"xmqqk3ehlyk5.fsf@gitster.dls.corp.google.com","threadId":"35599","inReplyTo":"1388716132-71437-1-git-send-email-jackerran@gmail.com","subject":"Re: [PATCH V3 1/2] fetch --prune: Always print header url","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-03T18:17:46Z","receivedAt":"2014-01-03T18:17:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Miller <jackerran@gmail.com> writes:\n\n> If \"fetch --prune\" is run with no new refs to fetch, but it has refs\n> to prune. Then, the header url is not printed as it would if there were\n> new refs to fetch.\n>\n> Output before this patch:\n>\n> \t$ git fetch --prune remote-with-no-new-refs\n> \t x [deleted]         (none)     -> origin/world\n>\n> Output after this patch:\n>\n> \t$ git fetch --prune remote-with-no-new-refs\n> \tFrom https://github.com/git/git\n> \t x [deleted]         (none)     -> origin/test\n>\n> Signed-off-by: Tom Miller <jackerran@gmail.com>\n\nAnd (needless to say) when there are refs to be fetched and also\npruned, the \"shown_url\" logic prevents us from giving two identical\nheaders \"From <there>\".  Sounds sane.\n\n> ---\n>\n> I decided it is not worth writing a function to format the header url\n> that is printed by fetch. Instead, I will duplicate the formatting logic\n> for now because I plan on following up this patch set with another patch\n> to stop striping the trailing \"/\" and \".git\" from the url. When that\n> patch is finished it will remove the majority of the duplicated logic.\n\nOK, let's see how it goes.\n\nWill replace what has been queued on 'pu'.  The incremental change\nrelative to the old one looked sensible and I think this one is\nready for 'next'.\n\nThanks.\n\n\n>  builtin/fetch.c  | 32 +++++++++++++++++++++++++++-----\n>  t/t5510-fetch.sh | 12 ++++++++++++\n>  2 files changed, 39 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 1e7d617..1b81cf9 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -44,6 +44,7 @@ static struct transport *gtransport;\n>  static struct transport *gsecondary;\n>  static const char *submodule_prefix = \"\";\n>  static const char *recurse_submodules_default;\n> +static int shown_url = 0;\n>  \n>  static int option_parse_recurse_submodules(const struct option *opt,\n>  \t\t\t\t   const char *arg, int unset)\n> @@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n>  {\n>  \tFILE *fp;\n>  \tstruct commit *commit;\n> -\tint url_len, i, shown_url = 0, rc = 0;\n> +\tint url_len, i, rc = 0;\n>  \tstruct strbuf note = STRBUF_INIT;\n>  \tconst char *what, *kind;\n>  \tstruct ref *rm;\n> @@ -708,17 +709,36 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n>  \treturn ret;\n>  }\n>  \n> -static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n> +static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,\n> +\t\tconst char *raw_url)\n>  {\n> -\tint result = 0;\n> +\tint url_len, i, result = 0;\n>  \tstruct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);\n> +\tchar *url;\n>  \tconst char *dangling_msg = dry_run\n>  \t\t? _(\"   (%s will become dangling)\")\n>  \t\t: _(\"   (%s has become dangling)\");\n>  \n> +\tif (raw_url)\n> +\t\turl = transport_anonymize_url(raw_url);\n> +\telse\n> +\t\turl = xstrdup(\"foreign\");\n> +\n> +\turl_len = strlen(url);\n> +\tfor (i = url_len - 1; url[i] == '/' && 0 <= i; i--)\n> +\t\t;\n> +\n> +\turl_len = i + 1;\n> +\tif (4 < i && !strncmp(\".git\", url + i - 3, 4))\n> +\t\turl_len = i - 3;\n> +\n>  \tfor (ref = stale_refs; ref; ref = ref->next) {\n>  \t\tif (!dry_run)\n>  \t\t\tresult |= delete_ref(ref->name, NULL, 0);\n> +\t\tif (verbosity >= 0 && !shown_url) {\n> +\t\t\tfprintf(stderr, _(\"From %.*s\\n\"), url_len, url);\n> +\t\t\tshown_url = 1;\n> +\t\t}\n>  \t\tif (verbosity >= 0) {\n>  \t\t\tfprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n>  \t\t\t\tTRANSPORT_SUMMARY(_(\"[deleted]\")),\n> @@ -726,6 +746,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n>  \t\t\twarn_dangling_symref(stderr, dangling_msg, ref->name);\n>  \t\t}\n>  \t}\n> +\tfree(url);\n>  \tfree_refs(stale_refs);\n>  \treturn result;\n>  }\n> @@ -854,11 +875,12 @@ static int do_fetch(struct transport *transport,\n>  \t\t * don't care whether --tags was specified.\n>  \t\t */\n>  \t\tif (ref_count) {\n> -\t\t\tprune_refs(refs, ref_count, ref_map);\n> +\t\t\tprune_refs(refs, ref_count, ref_map, transport->url);\n>  \t\t} else {\n>  \t\t\tprune_refs(transport->remote->fetch,\n>  \t\t\t\t   transport->remote->fetch_refspec_nr,\n> -\t\t\t\t   ref_map);\n> +\t\t\t\t   ref_map,\n> +\t\t\t\t   transport->url);\n>  \t\t}\n>  \t}\n>  \tfree_refs(ref_map);\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 5d4581d..87e896d 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '\n>  \ttest_bundle_object_count .git/objects/pack/pack-${pack##pack\t}.pack 3\n>  '\n>  \n> +test_expect_success 'fetch --prune prints the remotes url' '\n> +\tgit branch goodbye &&\n> +\tgit clone . only-prunes &&\n> +\tgit branch -D goodbye &&\n> +\t(\n> +\t\tcd only-prunes &&\n> +\t\tgit fetch --prune origin 2>&1 | head -n1 >../actual\n> +\t) &&\n> +\techo \"From ${D}/.\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"}]}