{"thread":{"id":"35548","subject":"[PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()","startedAt":"2013-12-18T21:22:54Z","lastAt":"2013-12-19T22:57:12Z","messageCount":13,"participants":["Tom Miller","Junio C Hamano","Jeff King","Thomas Miller"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"232200","messageId":"1387401776-30994-1-git-send-email-jackerran@gmail.com","threadId":"35548","inReplyTo":null,"subject":"[PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-18T21:22:54Z","receivedAt":"2013-12-18T21:22:54Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"In order to fix branchname DF conflicts during `fetch --prune`, the way\nthe header is output to the screen needs to be refactored. Here is an\nexmaple of the output with the line in question denoted by '>':\n\n\t$ git fetch --prune --dry-run upstream\n>\tFrom https://github.com/git/git\n\t   a155a5f..5512ac5  maint      -> upstream/maint\n\t   d7aced9..7794a68  master     -> upstream/master\n\t   523f7c4..3e57c29  next       -> upstream/next\n\t + 462f102...0937cdf pu         -> upstream/pu  (forced update)\n\t   e24105a..5d352bc  todo       -> upstream/todo\n\t * [new tag]         v1.8.5.2   -> v1.8.5.2\n\t * [new tag]         v1.8.5.2   -> v1.8.5.2\n\npretty_url():\nThis function when passed a transport url will anonymize the transport\nof the url. It will strip a trailing '/'. It will also strip a trailing\n'.git'. It will return the newly formated url for use. I do not believe\nthere is a need for stripping the trailing '/' and '.git' from a url,\nbut it was already there and I wanted to make as little changes as\npossible.\n\nprint_url():\nThis function will convert a transport url to a pretty url using\npretty_url(). Then it will print out the pretty url to stderr as\nindicated above in the example output. It uses a global variable\nnamed \"gshown_url' to prevent this header for being printed twice.\n\nSigned-off-by: Tom Miller <jackerran@gmail.com>\n---\n builtin/fetch.c | 60 ++++++++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 42 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 3d978eb..b3145f6 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -44,6 +44,42 @@ 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 gshown_url = 0;\n+\n+static char *pretty_url(const char *raw_url) {\n+\tif (raw_url) {\n+\t\tint url_len, i;\n+\t\tchar *pretty_url, *url;\n+\n+\t\turl = transport_anonymize_url(raw_url);\n+\n+\t\turl_len = strlen(url);\n+\t\tfor (i = url_len - 1; url[i] == '/' && 0 <= i; i--)\n+\t\t\t;\n+\t\turl_len = i + 1;\n+\t\tif (4 < i && !strncmp(\".git\", url + i - 3, 4))\n+\t\t\turl_len = i - 3;\n+\n+\t\tpretty_url = xcalloc(1, 1 + url_len);\n+\t\tmemcpy(pretty_url, url, url_len);\n+\n+\t\tfree(url);\n+\t\treturn pretty_url;\n+\t}\n+\treturn xstrdup(\"foreign\");\n+}\n+\n+static void print_url(const char *raw_url) {\n+\tif (!gshown_url) {\n+\t\tchar *url = pretty_url(raw_url);\n+\n+\t\tfprintf(stderr, _(\"From %s\\n\"), url);\n+\n+\t\tgshown_url = 1;\n+\t\tfree(url);\n+\t}\n+}\n+\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 +571,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@@ -546,10 +582,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \tif (!fp)\n \t\treturn error(_(\"cannot open %s: %s\\n\"), filename, strerror(errno));\n \n-\tif (raw_url)\n-\t\turl = transport_anonymize_url(raw_url);\n-\telse\n-\t\turl = xstrdup(\"foreign\");\n+\turl = pretty_url(raw_url);\n+\turl_len = strlen(url);\n \n \trm = ref_map;\n \tif (check_everything_connected(iterate_ref_map, 0, &rm)) {\n@@ -606,13 +640,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t\twhat = rm->name;\n \t\t\t}\n \n-\t\t\turl_len = strlen(url);\n-\t\t\tfor (i = url_len - 1; url[i] == '/' && 0 <= i; i--)\n-\t\t\t\t;\n-\t\t\turl_len = i + 1;\n-\t\t\tif (4 < i && !strncmp(\".git\", url + i - 3, 4))\n-\t\t\t\turl_len = i - 3;\n-\n \t\t\tstrbuf_reset(&note);\n \t\t\tif (*what) {\n \t\t\t\tif (*kind)\n@@ -651,13 +678,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t\t\t    REFCOL_WIDTH,\n \t\t\t\t\t    *what ? what : \"HEAD\");\n \t\t\tif (note.len) {\n-\t\t\t\tif (verbosity >= 0 && !shown_url) {\n-\t\t\t\t\tfprintf(stderr, _(\"From %.*s\\n\"),\n-\t\t\t\t\t\t\turl_len, url);\n-\t\t\t\t\tshown_url = 1;\n-\t\t\t\t}\n-\t\t\t\tif (verbosity >= 0)\n+\t\t\t\tif (verbosity >= 0) {\n+\t\t\t\t\tprint_url(raw_url);\n \t\t\t\t\tfprintf(stderr, \" %s\\n\", note.buf);\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n \t}\n-- \n1.8.5.1.163.gd7aced9\n"},{"id":"232201","messageId":"1387401776-30994-2-git-send-email-jackerran@gmail.com","threadId":"35548","inReplyTo":"1387401776-30994-1-git-send-email-jackerran@gmail.com","subject":"[PATCH 2/3] fetch --prune: Always print header url","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-18T21:22:55Z","receivedAt":"2013-12-18T21:22:55Z","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. the following is example output showing this\nbehavior:\n\n$ git fetch --prune --dry-run origin\n x [deleted]         (none)     -> origin/world\n\nAfter this patch the output of fetch --prune should look like this:\n\n$ git fetch --prune --dry-run origin\nFrom https://github.com/git/git\n x [deleted]         (none)     -> origin/test\n\nSigned-off-by: Tom Miller <jackerran@gmail.com>\n---\n builtin/fetch.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex b3145f6..e50b697 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -732,7 +732,8 @@ 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\t\tconst char *raw_url)\n {\n \tint result = 0;\n \tstruct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);\n@@ -744,6 +745,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n \t\tif (!dry_run)\n \t\t\tresult |= delete_ref(ref->name, NULL, 0);\n \t\tif (verbosity >= 0) {\n+\t\t\tprint_url(raw_url);\n \t\t\tfprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n \t\t\t\tTRANSPORT_SUMMARY(_(\"[deleted]\")),\n \t\t\t\tREFCOL_WIDTH, _(\"(none)\"), prettify_refname(ref->name));\n@@ -878,11 +880,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-- \n1.8.5.1.163.gd7aced9\n"},{"id":"232202","messageId":"1387401776-30994-3-git-send-email-jackerran@gmail.com","threadId":"35548","inReplyTo":"1387401776-30994-1-git-send-email-jackerran@gmail.com","subject":"[PATCH 3/3] fetch --prune: Repair branchname DF conflicts","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-18T21:22:56Z","receivedAt":"2013-12-18T21:22:56Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"When a branchname DF conflict occurs during a fetch, --prune should\nbe able to fix it. When fetching with --prune, the fetching process\nhappens before pruning causing the branchname DF conflict to persist\nand report an error. This patch prunes before fetching, thus\ncorrecting DF conflicts during a fetch.\n\nSigned-off-by: Tom Miller <jackerran@gmail.com>\nTested-by: Thomas Rast <tr@thomasrast.ch>\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 e50b697..845c687 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -868,11 +868,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@@ -888,6 +883,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 5d4581d..a981125 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -614,4 +614,18 @@ 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 '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.1.163.gd7aced9\n"},{"id":"232206","messageId":"xmqq8uvhhlwg.fsf@gitster.dls.corp.google.com","threadId":"35548","inReplyTo":"1387401776-30994-1-git-send-email-jackerran@gmail.com","subject":"Re: [PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-18T21:47:11Z","receivedAt":"2013-12-18T21:47:11Z","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> In order to fix branchname DF conflicts during `fetch --prune`, the way\n> the header is output to the screen needs to be refactored. Here is an\n> exmaple of the output with the line in question denoted by '>':\n>\n> \t$ git fetch --prune --dry-run upstream\n>>\tFrom https://github.com/git/git\n> \t   a155a5f..5512ac5  maint      -> upstream/maint\n> \t   d7aced9..7794a68  master     -> upstream/master\n> \t   523f7c4..3e57c29  next       -> upstream/next\n> \t + 462f102...0937cdf pu         -> upstream/pu  (forced update)\n> \t   e24105a..5d352bc  todo       -> upstream/todo\n> \t * [new tag]         v1.8.5.2   -> v1.8.5.2\n> \t * [new tag]         v1.8.5.2   -> v1.8.5.2\n>\n> pretty_url():\n> This function when passed a transport url will anonymize the transport\n> of the url. It will strip a trailing '/'. It will also strip a trailing\n> '.git'. It will return the newly formated url for use. I do not believe\n> there is a need for stripping the trailing '/' and '.git' from a url,\n> but it was already there and I wanted to make as little changes as\n> possible.\n\nOK.  I tend to agree that stripping the trailing part is probably\nnot a good idea and we would want to remove that but that definitely\nshould be done as a separate step, or even as a separate series on\ntop of this one.\n\n> print_url():\n> This function will convert a transport url to a pretty url using\n> pretty_url(). Then it will print out the pretty url to stderr as\n> indicated above in the example output. It uses a global variable\n> named \"gshown_url' to prevent this header for being printed twice.\n\nGaah.  What is that 'g' doing there?  Please don't do that\nmeaningless naming.\n\nI do not think the change to introduce such a global variable\nbelongs to this refactoring step.  The current caller can decide\nitself if it called that function, and if you are going to introduce\nnew callers in later steps, they can coordinate among themselves,\nno?\n"},{"id":"232207","messageId":"xmqq4n65hlko.fsf@gitster.dls.corp.google.com","threadId":"35548","inReplyTo":"1387401776-30994-3-git-send-email-jackerran@gmail.com","subject":"Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-18T21:54:15Z","receivedAt":"2013-12-18T21:54:15Z","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> When a branchname DF conflict occurs during a fetch,\n\nYou may have started with a specific case in which you want to\nchange the behaviour of current Git, so it may be clear what you\nmeant by \"branchname DF conflict\", but that is true for nobody other\nthan you who will read this log message.  Introducing new lingo is\nOK as long as it is necessary, but in a case like this, where you\nhave to describe what situation you are trying to address anyway,\nI do not think you need to add a new word to our vocabulary.\n\n\tWhen we have a remote-tracking branch frotz/nitfol from a\n\tprevious fetch, and the upstream now has branch frotz, we\n\tused to fail to remove frotz/nitfol and recreate frotz with\n\t\"git fetch --prune\" from the upstream.\n\nor something like that?\n\nBut what should happen when we do not give --prune to \"git fetch\" in\nsuch a situation?  Should it fail, because we still have frotz/nitfol\nand we cannot create frotz without losing it?\n\n> --prune should\n> be able to fix it. When fetching with --prune, the fetching process\n> happens before pruning causing the branchname DF conflict to persist\n> and report an error. This patch prunes before fetching, thus\n> correcting DF conflicts during a fetch.\n>\n> Signed-off-by: Tom Miller <jackerran@gmail.com>\n> Tested-by: Thomas Rast <tr@thomasrast.ch>\n\nI wasn't following previous threads closely (was there a previous\nthread???); has this iteration been already tested by trast?\n\n> ---\n>  builtin/fetch.c  | 10 +++++-----\n>  t/t5510-fetch.sh | 14 ++++++++++++++\n>  2 files changed, 19 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index e50b697..845c687 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -868,11 +868,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> @@ -888,6 +883,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\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 5d4581d..a981125 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -614,4 +614,18 @@ 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 '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"},{"id":"232225","messageId":"20131219011817.GA31924@gmail.com","threadId":"35548","inReplyTo":"xmqq8uvhhlwg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-19T01:18:17Z","receivedAt":"2013-12-19T01:18:17Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"On Wed, Dec 18, 2013 at 3:47 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tom Miller <jackerran@gmail.com> writes:\n>\n>> In order to fix branchname DF conflicts during `fetch --prune`, the way\n>> the header is output to the screen needs to be refactored. Here is an\n>> exmaple of the output with the line in question denoted by '>':\n>>\n>>       $ git fetch --prune --dry-run upstream\n>>>      From https://github.com/git/git\n>>          a155a5f..5512ac5  maint      -> upstream/maint\n>>          d7aced9..7794a68  master     -> upstream/master\n>>          523f7c4..3e57c29  next       -> upstream/next\n>>        + 462f102...0937cdf pu         -> upstream/pu  (forced update)\n>>          e24105a..5d352bc  todo       -> upstream/todo\n>>        * [new tag]         v1.8.5.2   -> v1.8.5.2\n>>        * [new tag]         v1.8.5.2   -> v1.8.5.2\n>>\n>> pretty_url():\n>> This function when passed a transport url will anonymize the transport\n>> of the url. It will strip a trailing '/'. It will also strip a trailing\n>> '.git'. It will return the newly formated url for use. I do not believe\n>> there is a need for stripping the trailing '/' and '.git' from a url,\n>> but it was already there and I wanted to make as little changes as\n>> possible.\n>\n> OK.  I tend to agree that stripping the trailing part is probably\n> not a good idea and we would want to remove that but that definitely\n> should be done as a separate step, or even as a separate series on\n> top of this one.\n\nI think that removing the trailing part will greatly reduce the complexity\nto the point were it is unnecessary to have pretty_url().  My goal with\nextracting this function is to isolate the complexity of formatting the\nurl to a single spot. I am thinking along the lines of the following\ncommit order:\n\n1. Remove the \"remove trailing part\"\n2. Add print_url()\n3. Always print url when pruning\n4. Reverse order of prune and fetch\n\n>> print_url():\n>> This function will convert a transport url to a pretty url using\n>> pretty_url(). Then it will print out the pretty url to stderr as\n>> indicated above in the example output. It uses a global variable\n>> named \"gshown_url' to prevent this header for being printed twice.\n>\n> Gaah.  What is that 'g' doing there?  Please don't do that\n> meaningless naming.\n\nI am not familiar with C conventions and I was trying to stay consistent.\nI saw other global variables starting with 'g' and made an assumption.\nIt will use the original name in the upcoming patches.\n\n> I do not think the change to introduce such a global variable\n> belongs to this refactoring step.  The current caller can decide\n> itself if it called that function, and if you are going to introduce\n> new callers in later steps, they can coordinate among themselves,\n> no?\n\nI agree, there is no reason for introducing it in this step. Thanks for\npointing that out.\n"},{"id":"232226","messageId":"20131219014859.GA32240@gmail.com","threadId":"35548","inReplyTo":"xmqq4n65hlko.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-19T01:48:59Z","receivedAt":"2013-12-19T01:48:59Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"On Wed, Dec 18, 2013 at 3:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tom Miller <jackerran@gmail.com> writes:\n>\n>> When a branchname DF conflict occurs during a fetch,\n>\n> You may have started with a specific case in which you want to\n> change the behaviour of current Git, so it may be clear what you\n> meant by \"branchname DF conflict\", but that is true for nobody other\n> than you who will read this log message.  Introducing new lingo is\n> OK as long as it is necessary, but in a case like this, where you\n> have to describe what situation you are trying to address anyway,\n> I do not think you need to add a new word to our vocabulary.\n>\n>         When we have a remote-tracking branch frotz/nitfol from a\n>         previous fetch, and the upstream now has branch frotz, we\n>         used to fail to remove frotz/nitfol and recreate frotz with\n>         \"git fetch --prune\" from the upstream.\n>\n> or something like that?\n\nI did not intend to introduce new lingo. I did some searching through\nhistory to see if something like this had been worked on before and\nI found a commit by Jeff King that introduced me the the idea of\n\"DF conflicts\"\n\n\t> commit fa250759794ab98e6edfbbf2f6aa2cb912e535eb\n\t> Author: Jeff King <peff@peff.net>\n\t> Date:   Mon May 25 06:40:54 2009 -0400\n\t>\n\t>     fetch: report ref storage DF errors more accurately\n\t>\n\t>     When we fail to store a fetched ref, we recommend that the\n\t>     user try running \"git prune\" to remove up any old refs that\n\t>     have been deleted by the remote, which would clear up any DF\n\t>     conflicts. However, ref storage might fail for other\n\t>     reasons (e.g., permissions problems) in which case the\n\t>     advice is useless and misleading.\n\t>\n\t>     This patch detects when there is an actual DF situation and\n\t>     only issues the advice when one is found.\n\t>\n\t>     Signed-off-by: Jeff King <peff@peff.net>\n\t>     Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nI have no issue with rewording the it to be more clear and to try to\nremove any new lingo.\n\n> But what should happen when we do not give --prune to \"git fetch\" in\n> such a situation?  Should it fail, because we still have frotz/nitfol\n> and we cannot create frotz without losing it?\n\nYou talk about this to some extent in an email from 2009. I have linked\nit below for your review.\nhttp://article.gmane.org/gmane.comp.version-control.git/132276\n\nIn my opinion, if I supply \"--prune\" to \"fetch\" I expect it to be\ndestructive. It should be noted that the reflog can *not* be used\nto recover pruned branches from a remote.\n\n>> --prune should\n>> be able to fix it. When fetching with --prune, the fetching process\n>> happens before pruning causing the branchname DF conflict to persist\n>> and report an error. This patch prunes before fetching, thus\n>> correcting DF conflicts during a fetch.\n>>\n>> Signed-off-by: Tom Miller <jackerran@gmail.com>\n>> Tested-by: Thomas Rast <tr@thomasrast.ch>\n>\n> I wasn't following previous threads closely (was there a previous\n> thread???); has this iteration been already tested by trast?\n\nThere was a previous thread, but I was just looking for feed back on this\nas a WIP. Should I have replied to it with this patchset?\n\nHere is a link to the previous thread.\nhttp://thread.gmane.org/gmane.comp.version-control.git/238530\n\nThe commit below should be the same patch he tested. The test was added\nby him, and I made it part of this commit. Did I do this wrong?\n\n>> ---\n>>  builtin/fetch.c  | 10 +++++-----\n>>  t/t5510-fetch.sh | 14 ++++++++++++++\n>>  2 files changed, 19 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/builtin/fetch.c b/builtin/fetch.c\n>> index e50b697..845c687 100644\n>> --- a/builtin/fetch.c\n>> +++ b/builtin/fetch.c\n>> @@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,\n>>\n>>       if (tags == TAGS_DEFAULT && autotags)\n>>               transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n>> -     if (fetch_refs(transport, ref_map)) {\n>> -             free_refs(ref_map);\n>> -             retcode = 1;\n>> -             goto cleanup;\n>> -     }\n>>       if (prune) {\n>>               /*\n>>                * We only prune based on refspecs specified\n>> @@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,\n>>                                  transport->url);\n>>               }\n>>       }\n>> +     if (fetch_refs(transport, ref_map)) {\n>> +             free_refs(ref_map);\n>> +             retcode = 1;\n>> +             goto cleanup;\n>> +     }\n>>       free_refs(ref_map);\n>>\n>>       /* if neither --no-tags nor --tags was specified, do automated tag\n>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n>> index 5d4581d..a981125 100755\n>> --- a/t/t5510-fetch.sh\n>> +++ b/t/t5510-fetch.sh\n>> @@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded' '\n>>       test_bundle_object_count .git/objects/pack/pack-${pack##pack    }.pack 3\n>>  '\n>>\n>> +test_expect_success 'branchname D/F conflict resolved by --prune' '\n>> +     git branch dir/file &&\n>> +     git clone . prune-df-conflict &&\n>> +     git branch -D dir/file &&\n>> +     git branch dir &&\n>> +     (\n>> +             cd prune-df-conflict &&\n>> +             git fetch --prune &&\n>> +             git rev-parse origin/dir >../actual\n>> +     ) &&\n>> +     git rev-parse dir >expect &&\n>> +     test_cmp expect actual\n>> +'\n>> +\n>>  test_done\n"},{"id":"232229","messageId":"7vzjnxjqwp.fsf@alter.siamese.dyndns.org","threadId":"35548","inReplyTo":"20131219014859.GA32240@gmail.com","subject":"Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-19T06:28:22Z","receivedAt":"2013-12-19T06:28:22Z","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> The commit below should be the same patch he tested. The test was added\n> by him, and I made it part of this commit. Did I do this wrong?\n\nNo, no, no.  All my questions were true questions, not complaints\nveiled as rhetorical questions.  Thanks for many pointers for\nclarification.\n\n>>>> ---\n>>>  builtin/fetch.c  | 10 +++++-----\n>>>  t/t5510-fetch.sh | 14 ++++++++++++++\n>>>  2 files changed, 19 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/builtin/fetch.c b/builtin/fetch.c\n>>> index e50b697..845c687 100644\n>>> --- a/builtin/fetch.c\n>>> +++ b/builtin/fetch.c\n>>> @@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,\n>>>\n>>>       if (tags == TAGS_DEFAULT && autotags)\n>>>               transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n>>> -     if (fetch_refs(transport, ref_map)) {\n>>> -             free_refs(ref_map);\n>>> -             retcode = 1;\n>>> -             goto cleanup;\n>>> -     }\n>>>       if (prune) {\n>>>               /*\n>>>                * We only prune based on refspecs specified\n>>> @@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,\n>>>                                  transport->url);\n>>>               }\n>>>       }\n>>> +     if (fetch_refs(transport, ref_map)) {\n>>> +             free_refs(ref_map);\n>>> +             retcode = 1;\n>>> +             goto cleanup;\n>>> +     }\n>>>       free_refs(ref_map);\n>>>\n>>>       /* if neither --no-tags nor --tags was specified, do automated tag\n>>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n>>> index 5d4581d..a981125 100755\n>>> --- a/t/t5510-fetch.sh\n>>> +++ b/t/t5510-fetch.sh\n>>> @@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded' '\n>>>       test_bundle_object_count .git/objects/pack/pack-${pack##pack    }.pack 3\n>>>  '\n>>>\n>>> +test_expect_success 'branchname D/F conflict resolved by --prune' '\n>>> +     git branch dir/file &&\n>>> +     git clone . prune-df-conflict &&\n>>> +     git branch -D dir/file &&\n>>> +     git branch dir &&\n>>> +     (\n>>> +             cd prune-df-conflict &&\n>>> +             git fetch --prune &&\n>>> +             git rev-parse origin/dir >../actual\n>>> +     ) &&\n>>> +     git rev-parse dir >expect &&\n>>> +     test_cmp expect actual\n>>> +'\n>>> +\n>>>  test_done\n"},{"id":"232233","messageId":"20131219114413.GA23298@sigill.intra.peff.net","threadId":"35548","inReplyTo":"20131219014859.GA32240@gmail.com","subject":"Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-12-19T11:44:14Z","receivedAt":"2013-12-19T11:44:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 18, 2013 at 07:48:59PM -0600, Tom Miller wrote:\n\n> I did not intend to introduce new lingo. I did some searching through\n> history to see if something like this had been worked on before and\n> I found a commit by Jeff King that introduced me the the idea of\n> \"DF conflicts\"\n\nI take all the blame. :)\n\nAs for the patch itself:\n\n> >> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> >> index e50b697..845c687 100644\n> >> --- a/builtin/fetch.c\n> >> +++ b/builtin/fetch.c\n> >> @@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,\n> >>\n> >>       if (tags == TAGS_DEFAULT && autotags)\n> >>               transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n> >> -     if (fetch_refs(transport, ref_map)) {\n> >> -             free_refs(ref_map);\n> >> -             retcode = 1;\n> >> -             goto cleanup;\n> >> -     }\n> >>       if (prune) {\n> >>               /*\n> >>                * We only prune based on refspecs specified\n> >> @@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,\n> >>                                  transport->url);\n> >>               }\n> >>       }\n> >> +     if (fetch_refs(transport, ref_map)) {\n> >> +             free_refs(ref_map);\n> >> +             retcode = 1;\n> >> +             goto cleanup;\n> >> +     }\n\nI think this is _probably_ a good thing to do, but it does have an\ninteresting side effect for concurrent operations, and I haven't seen\nthat mentioned so far in the discussion.\n\nReaders of the ref namespace don't have any sort of transactionally\nconsistent view of all of the refs. So if a remote has moved a branch\n\"foo\" to \"bar\" and we \"fetch --prune\", there will be a moment where a\nsimultaneous reader will see one of two states that never existed on the\nremote (depending on the order the fetch chooses): either both refs\nexist, or neither exists.\n\nRight now fetch creates first and deletes after, so a simultaneous\nreader may see both refs. After your change, it may see no refs at all.\nEven though both are technically wrong, the current behavior is safer.\nIf the reader is calculating reachability (e.g., for a repack or \"git\nprune), it is better to have too many references than too few.\n\nI'm not sure to what degree we want to care. This is a race, but it's a\nreasonably unlikely one, and the D/F thing bites people in the real\nworld.\n\nAnd further confounding this is the fact that even if the writer does\neverything correctly, the way we read refs can still cause an odd view\nof the whole namespace. For example, consider moving \"refs/heads/z/foo\"\nto \"refs/heads/a/foo\", while somebody else reads simultaneously. Even\nwith create-before-delete, we can get the sequence:\n\n  1. Reader reads \"refs/heads/a/\" and sees it does not contain \"foo\".\n\n  2. Writer writes \"refs/heads/a/foo\".\n\n  3. Writer deletes \"refs/heads/z/foo\".\n\n  4. Reader reads \"refs/heads/z\", which does not contain \"foo\".\n\nThat race can be closed with a double-read of the ref namespaces, but\nthat has poor performance. A more reasonable fix, IMHO, would be to have\nan alternate ref store that represents transactions atomically (keeping\nin mind that this really only matters for busy repos with simultaneous\nreaders and writers, so it would not even need to be the default ref\nstore). And once you have such a store, that solves the other problem,\ntoo: you can just treat the delete-create as a transaction anyway. It\nalso solves a similar problem with refs that rewind.\n\nSo even leaving it as-is does not make the problem go away, though the\nproposed change does exacerbate it somewhat. I wonder how hard it would\nbe to do the safer thing in the common case that there is no D/F\nconflict. That is, do multiple passes at updating the refs:\n\n  1. Create/update any refs we can. Those with D/F conflicts are put\n     aside for the moment.\n\n  2. Delete any refs according to the --prune rules.\n\n  3. Come back to any D/F conflicts and try them again.\n\nI dunno. As far as I know, this is not a race that people see often in\nreal life (I do not have any confirmed cases of it yet). So it may\nsimply not be worth worrying about.\n\n-Peff\n"},{"id":"232242","messageId":"CAECD2aGPbJEUD5Vr0XH23eP_353hKwzRs7Q3sJ65LRjFKMr6Yw@mail.gmail.com","threadId":"35548","inReplyTo":"20131219011817.GA31924@gmail.com","subject":"Re: [PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()","fromName":"Thomas Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-19T17:41:09Z","receivedAt":"2013-12-19T17:41:09Z","isPatch":true,"sender":{"key":"jackerran@gmail.com","avatar":null},"body":"On Wed, Dec 18, 2013 at 7:18 PM, Tom Miller <jackerran@gmail.com> wrote:\n> On Wed, Dec 18, 2013 at 3:47 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Tom Miller <jackerran@gmail.com> writes:\n>>\n>>> In order to fix branchname DF conflicts during `fetch --prune`, the way\n>>> the header is output to the screen needs to be refactored. Here is an\n>>> exmaple of the output with the line in question denoted by '>':\n>>>\n>>>       $ git fetch --prune --dry-run upstream\n>>>>      From https://github.com/git/git\n>>>          a155a5f..5512ac5  maint      -> upstream/maint\n>>>          d7aced9..7794a68  master     -> upstream/master\n>>>          523f7c4..3e57c29  next       -> upstream/next\n>>>        + 462f102...0937cdf pu         -> upstream/pu  (forced update)\n>>>          e24105a..5d352bc  todo       -> upstream/todo\n>>>        * [new tag]         v1.8.5.2   -> v1.8.5.2\n>>>        * [new tag]         v1.8.5.2   -> v1.8.5.2\n>>>\n>>> pretty_url():\n>>> This function when passed a transport url will anonymize the transport\n>>> of the url. It will strip a trailing '/'. It will also strip a trailing\n>>> '.git'. It will return the newly formated url for use. I do not believe\n>>> there is a need for stripping the trailing '/' and '.git' from a url,\n>>> but it was already there and I wanted to make as little changes as\n>>> possible.\n>>\n>> OK.  I tend to agree that stripping the trailing part is probably\n>> not a good idea and we would want to remove that but that definitely\n>> should be done as a separate step, or even as a separate series on\n>> top of this one.\n>\n> I think that removing the trailing part will greatly reduce the complexity\n> to the point were it is unnecessary to have pretty_url().  My goal with\n> extracting this function is to isolate the complexity of formatting the\n> url to a single spot. I am thinking along the lines of the following\n> commit order:\n>\n> 1. Remove the \"remove trailing part\"\n> 2. Add print_url()\n> 3. Always print url when pruning\n> 4. Reverse order of prune and fetch\n>\n>>> print_url():\n>>> This function will convert a transport url to a pretty url using\n>>> pretty_url(). Then it will print out the pretty url to stderr as\n>>> indicated above in the example output. It uses a global variable\n>>> named \"gshown_url' to prevent this header for being printed twice.\n>>\n>> Gaah.  What is that 'g' doing there?  Please don't do that\n>> meaningless naming.\n>\n> I am not familiar with C conventions and I was trying to stay consistent.\n> I saw other global variables starting with 'g' and made an assumption.\n> It will use the original name in the upcoming patches.\n>\n>> I do not think the change to introduce such a global variable\n>> belongs to this refactoring step.  The current caller can decide\n>> itself if it called that function, and if you are going to introduce\n>> new callers in later steps, they can coordinate among themselves,\n>> no?\n>\n> I agree, there is no reason for introducing it in this step. Thanks for\n> pointing that out.\n\nAfter working on this some more and realizing there is more work to\nbe done on the \"fetch --prune should prune before fetching\" issue. Also,\nseeing Jeff's response opened my eyes even more. I believe you are\ncorrect. The \"trailing parts\" piece should be split off into another patch set.\nI think it would make sense to add the \"fetch --prune should print the header\nurl\" to that patch set. Should I submit those patches as a separate thread\nor reply to this thread with just those patches?\n\n-- \nTom Miller\njackerran@gmail.com\n"},{"id":"232250","messageId":"xmqq7gb0fxd0.fsf@gitster.dls.corp.google.com","threadId":"35548","inReplyTo":"20131219014859.GA32240@gmail.com","subject":"Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-19T19:34:51Z","receivedAt":"2013-12-19T19:34:51Z","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>> But what should happen when we do not give --prune to \"git fetch\" in\n>> such a situation?  Should it fail, because we still have frotz/nitfol\n>> and we cannot create frotz without losing it?\n>\n> You talk about this to some extent in an email from 2009. I have linked\n> it below for your review.\n> http://article.gmane.org/gmane.comp.version-control.git/132276\n\nI do not think the old discussion talks about the case.  It was\nabout \"we have remotes/origin/{frotz,nitfol} from the origin from an\nearlier fetch, the origin now has updated its frotz and deleted its\nnitfol.  'git remote prune' removes our remotes/origin/nitfol\nwithout updating our copy of remotes/origin/frotz, but I do not\nthink it is sensible.  'git fetch --prune origin' would update both\nand make our remote-tracking branches for 'origin' in line with the\nreality\".  It was not about what 'git fetch' without '--prune'\nshould do.\n\nYour \"'git fetch' without '--prune' should be less destrictive\" is a\ngood guiding principle.  If we have a copy of the 'frotz/nitfol'\nbranch from the 'origin', removing it so that we can have a new copy\nof the 'frotz' branch the 'origin' now has (after it removed\n'frotz/nitfol' to make room) is indeed an operation that loses\ninformation.  And it probably is the right thing to do to fail such\na fetch. 'git fetch --prune' on the other hand really means \"I do\nnot care about the branches' histories my 'origin' discarded; bring\nme up to date and give me the same view as my 'origin' has in my\nremote-tracking branches\", so losing 'frotz/nitfol', which the\n'origin' already decided to discard, is what the user wants.\n\nThe atomicity issue Peff brings up is an interesting and important\none, but I think that is an orthogonal issue.\n\nWith the background information from the previous thread between you\nand trast, the patch [3/3] looks good to me.\n\nThanks.\n"},{"id":"232265","messageId":"1387493832-13588-1-git-send-email-jackerran@gmail.com","threadId":"35548","inReplyTo":"1387401776-30994-1-git-send-email-jackerran@gmail.com","subject":"[PATCH V2 1/2] fetch --prune: Always print header url","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-19T22:57:11Z","receivedAt":"2013-12-19T22:57:11Z","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.git\n\t x [deleted]         (none)     -> origin/test\n\nSigned-off-by: Tom Miller <jackerran@gmail.com>\n---\n builtin/fetch.c  | 22 ++++++++++++++++++----\n t/t5510-fetch.sh | 12 ++++++++++++\n 2 files changed, 30 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 1e7d617..e6dc2d6 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,28 @@ 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 \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 \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);\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 +738,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 +867,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..08a4841 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 full 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.194.g00457d4\n"},{"id":"232266","messageId":"1387493832-13588-2-git-send-email-jackerran@gmail.com","threadId":"35548","inReplyTo":"1387493832-13588-1-git-send-email-jackerran@gmail.com","subject":"[PATCH V2 2/2] fetch --prune: Run prune before fetching","fromName":"Tom Miller","fromEmail":"jackerran@gmail.com","sentAt":"2013-12-19T22:57:12Z","receivedAt":"2013-12-19T22:57:12Z","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>\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 e6dc2d6..ef3f0bb 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -855,11 +855,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@@ -875,6 +870,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 08a4841..3e64af4 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 full 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.194.g00457d4\n"}]}