{"thread":{"id":"13858","subject":"remote show/prune: strange -n(--dry-run) option.","startedAt":"2008-06-08T00:54:43Z","lastAt":"2008-06-12T11:07:46Z","messageCount":36,"participants":["Olivier Marin","dkr+ml.git@free.fr","Junio C Hamano","Johannes Schindelin","Shawn O. Pearce","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"79079","messageId":"484B2DD3.8050307@free.fr","threadId":"13858","inReplyTo":null,"subject":"remote show/prune: strange -n(--dry-run) option.","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-08T00:54:43Z","receivedAt":"2008-06-08T00:54:43Z","isPatch":false,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Hello,\n\nThe git-remote documentation talks about a mysterious -n option for show\nand prune that comes from the old git-remote.perl script. This flag was\nused to prevent the script from calling ls-remote more than once, FWIU.\nToday, the builtin accept an (un)?related -n(--dry-run) flag that does\nnothing, actually. It seems broken.\n\nSo, is it safe to drop it entirely or is it better to just remove it\nfrom the documentation for compatibility? In the second case, how long\nshould we wait before using --dry-run for something different?\n\nI would like to make \"git remote prune\" more verbose and use --dry-run\nto really prevent it from deleting stale tracking branches.\n\n$ git remote prune origin -n\nPruning origin\nFrom: git://.../myproject.git\n * [stale branch] bla\n * [stale branch] bli\n * [stale branch] blu\n\nWhat about something like that ?\n\nOlivier.\n"},{"id":"79103","messageId":"484BBC8C.80700@free.fr","threadId":"13858","inReplyTo":"484B2DD3.8050307@free.fr","subject":"[PATCH] Documentation/git-remote.txt: remove description for useless -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-08T11:03:40Z","receivedAt":"2008-06-08T11:03:40Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThis option comes from the original git-remote.perl script and is not\nused nor needed in the current builtin.\n\nSo, remove it from the documentation so that we can reuse it later for\nsomething else.\n\nSigned-off-by: Olivier Marin \n---\n Documentation/git-remote.txt | 7 -------\n 1 files changed, 0 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex e97dc09..e51d232 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -61,9 +61,6 @@ configuration settings for the remote are removed.\n 'show'::\n\n Gives some information about the remote .\n-+\n-With `-n` option, the remote heads are not queried first with\n-`git ls-remote `; cached information is used instead.\n\n 'prune'::\n\n@@ -71,10 +68,6 @@ Deletes all stale tracking branches under .\n These stale branches have already been removed from the remote repository\n referenced by , but are still locally available in\n \"remotes/\".\n-+\n-With `-n` option, the remote heads are not confirmed first with `git\n-ls-remote `; cached information is used instead. Use with\n-caution.\n\n 'update'::\n\n-- 1.5.6.rc2.160.ga44ac\n"},{"id":"79105","messageId":"1212927772-10006-1-git-send-email-dkr+ml.git@free.fr","threadId":"13858","inReplyTo":"484B2DD3.8050307@free.fr","subject":"[PATCH] Documentation/git-remote.txt: remove description for useless -n option","fromName":"","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-08T12:22:52Z","receivedAt":"2008-06-08T12:22:52Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThis option comes from the original git-remote.perl script and is not\nused nor needed in the current builtin.\n\nSo, remove it from the documentation so that we can reuse it later for\nsomething else.\n\nSigned-off-by: Olivier Marin \n---\n\nMy MUA destroyed the previous patch! Sorry.\n\n Documentation/git-remote.txt | 7 -------\n 1 files changed, 0 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex e97dc09..e51d232 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -61,9 +61,6 @@ configuration settings for the remote are removed.\n 'show'::\n \n Gives some information about the remote .\n-+\n-With `-n` option, the remote heads are not queried first with\n-`git ls-remote `; cached information is used instead.\n \n 'prune'::\n \n@@ -71,10 +68,6 @@ Deletes all stale tracking branches under .\n These stale branches have already been removed from the remote repository\n referenced by , but are still locally available in\n \"remotes/\".\n-+\n-With `-n` option, the remote heads are not confirmed first with `git\n-ls-remote `; cached information is used instead. Use with\n-caution.\n \n 'update'::\n \n-- \n1.5.6.rc2.160.ga44ac\n"},{"id":"79155","messageId":"7v63sjk6yo.fsf@gitster.siamese.dyndns.org","threadId":"13858","inReplyTo":"1212927772-10006-1-git-send-email-dkr+ml.git@free.fr","subject":"Re: [PATCH] Documentation/git-remote.txt: remove description for useless -n option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-08T20:27:43Z","receivedAt":"2008-06-08T20:27:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"dkr+ml.git@free.fr writes:\n\n> From: Olivier Marin \n>\n> This option comes from the original git-remote.perl script and is not\n> used nor needed in the current builtin.\n\nIs this something we would want to document as a new feature, or just a\nregression that makes the existing feature unusable when disconnected from\nthe network that needs to be fixed in the code?\n"},{"id":"79192","messageId":"484C7CBE.4070700@free.fr","threadId":"13858","inReplyTo":"7v63sjk6yo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Documentation/git-remote.txt: remove description for useless -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T00:43:42Z","receivedAt":"2008-06-09T00:43:42Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Junio C Hamano a écrit :\n>> From: Olivier Marin \n>>\n>> This option comes from the original git-remote.perl script and is not\n>> used nor needed in the current builtin.\n> \n> Is this something we would want to document as a new feature, or just a\n> regression that makes the existing feature unusable when disconnected from\n> the network that needs to be fixed in the code?\n\nOK, I restored the original behaviour for \"git remote show\", patch will follow.\n\nBut for \"git remote prune\" I don't no what to do. The perl script behaviour was\nto delete all refs for the remote when called with -n. It seems really dangerous\nto me, especially if I have no connection to restore them. The current builtin\ndoesn't honor the flag and with my patch it just does nothing.\n\nSo, do you prefere that I remove the documentation for git remote prune -n or\nrestore the old behaviour that was probably never used (but maybe I'm wrong)?\n\nOlivier.\n"},{"id":"79195","messageId":"484C7DCC.6080303@free.fr","threadId":"13858","inReplyTo":"484C7CBE.4070700@free.fr","subject":"[PATCH] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T00:48:12Z","receivedAt":"2008-06-09T00:48:12Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThe perl version accepted a -n flag, to show local informations only\nwithout querying remote heads, that seems to have been lost in the C\nrewrite.\n\nThis restores the older behaviour and add a test case.\n\nSigned-off-by: Olivier Marin \n---\n builtin-remote.c | 48 ++++++++++++++++++++++++++----------------------\n t/t5505-remote.sh | 17 +++++++++++++++++\n 2 files changed, 43 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex c49f00f..cb9e282 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n \n static int show_or_prune(int argc, const char **argv, int prune)\n {\n-\tint dry_run = 0, result = 0;\n+\tint no_query = 0, result = 0;\n \tstruct option options[] = {\n \t\tOPT_GROUP(\"show specific options\"),\n-\t\tOPT__DRY_RUN(&dry_run),\n+\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n \t\tOPT_END()\n \t};\n \tstruct ref_states states;\n@@ -442,21 +442,25 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\tstruct transport *transport;\n \t\tconst struct ref *ref;\n \t\tstruct strbuf buf;\n-\t\tint i, got_states;\n+\t\tint i, got_states = 1;\n \n \t\tstates.remote = remote_get(*argv);\n \t\tif (!states.remote)\n \t\t\treturn error(\"No such remote: %s\", *argv);\n-\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n-\t\t\tstates.remote->url[0] : NULL);\n-\t\tref = transport_get_remote_refs(transport);\n-\t\ttransport_disconnect(transport);\n \n \t\tread_branches();\n-\t\tgot_states = get_ref_states(ref, &states);\n-\t\tif (got_states)\n-\t\t\tresult = error(\"Error getting local info for '%s'\",\n-\t\t\t\t\tstates.remote->name);\n+\n+\t\tif (!no_query) {\n+\t\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n+\t\t\t\tstates.remote->url[0] : NULL);\n+\t\t\tref = transport_get_remote_refs(transport);\n+\t\t\ttransport_disconnect(transport);\n+\n+\t\t\tgot_states = get_ref_states(ref, &states);\n+\t\t\tif (got_states)\n+\t\t\t\tresult = error(\"Error getting local info for '%s'\",\n+\t\t\t\t\t\tstates.remote->name);\n+\t\t}\n \n \t\tif (prune) {\n \t\t\tfor (i = 0; i < states.stale.nr; i++) {\n@@ -486,17 +490,17 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\t\tprintf(\"\\n\");\n \t\t}\n \n-\t\tif (got_states)\n-\t\t\tcontinue;\n-\t\tstrbuf_init(&buf, 0);\n-\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch will \"\n-\t\t\t\"store in remotes/%s)\", states.remote->name);\n-\t\tshow_list(buf.buf, &states.new);\n-\t\tstrbuf_release(&buf);\n-\t\tshow_list(\" Stale tracking branch%s (use 'git remote prune')\",\n-\t\t\t\t&states.stale);\n-\t\tshow_list(\" Tracked remote branch%s\",\n-\t\t\t\t&states.tracked);\n+\t\tif (!got_states) {\n+\t\t\tstrbuf_init(&buf, 0);\n+\t\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch will \"\n+\t\t\t\t\"store in remotes/%s)\", states.remote->name);\n+\t\t\tshow_list(buf.buf, &states.new);\n+\t\t\tstrbuf_release(&buf);\n+\t\t\tshow_list(\" Stale tracking branch%s (use 'git remote prune')\",\n+\t\t\t\t\t&states.stale);\n+\t\t\tshow_list(\" Tracked remote branch%s\",\n+\t\t\t\t\t&states.tracked);\n+\t\t}\n \n \t\tif (states.remote->push_refspec_nr) {\n \t\t\tprintf(\" Local branch%s pushed with 'git push'\\n \",\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 0d7ed1f..c6a7bfb 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -138,6 +138,23 @@ test_expect_success 'show' '\n \t test_cmp expect output)\n '\n \n+cat > test/expect << EOF\n+* remote origin\n+ URL: $(pwd)/one/.git\n+ Remote branch merged with 'git pull' while on branch master\n+ master\n+ Local branches pushed with 'git push'\n+ master:upstream +refs/tags/lastbackup\n+EOF\n+\n+test_expect_success 'show -n' '\n+\t(mv one one.unreachable &&\n+\t cd test &&\n+\t git remote show -n origin > output &&\n+\t mv ../one.unreachable ../one &&\n+\t test_cmp expect output)\n+'\n+\n test_expect_success 'prune' '\n \t(cd one &&\n \t git branch -m side side2) &&\n-- 1.5.6.rc2.121.gaeb64.dirty\n"},{"id":"79197","messageId":"alpine.DEB.1.00.0806090212270.1783@racer","threadId":"13858","inReplyTo":"484C7DCC.6080303@free.fr","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T01:16:52Z","receivedAt":"2008-06-09T01:16:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n> The perl version accepted a -n flag, to show local informations only \n> without querying remote heads, that seems to have been lost in the C \n> rewrite.\n\nWould have been nice to Cc: the author of the C rewrite.\n\n> diff --git a/builtin-remote.c b/builtin-remote.c\n> index c49f00f..cb9e282 100644\n> --- a/builtin-remote.c\n> +++ b/builtin-remote.c\n> @@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n> \n> static int show_or_prune(int argc, const char **argv, int prune)\n> {\n> -\tint dry_run = 0, result = 0;\n> +\tint no_query = 0, result = 0;\n\nWhy?\n\n> \tstruct option options[] = {\n> \t\tOPT_GROUP(\"show specific options\"),\n> -\t\tOPT__DRY_RUN(&dry_run),\n> +\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n\nWhy?\n\n\n> +\t\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n\nPlease rewrap.\n\n> @@ -486,17 +490,17 @@ static int show_or_prune(int argc, const char **argv, int prune)\n> \t\t\tprintf(\"\\n\");\n> \t\t}\n> \n> -\t\tif (got_states)\n> -\t\t\tcontinue;\n> -\t\tstrbuf_init(&buf, 0);\n> -\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch will \"\n> -\t\t\t\"store in remotes/%s)\", states.remote->name);\n> -\t\tshow_list(buf.buf, &states.new);\n> -\t\tstrbuf_release(&buf);\n> -\t\tshow_list(\" Stale tracking branch%s (use 'git remote prune')\",\n> -\t\t\t\t&states.stale);\n> -\t\tshow_list(\" Tracked remote branch%s\",\n> -\t\t\t\t&states.tracked);\n> +\t\tif (!got_states) {\n> +\t\t\tstrbuf_init(&buf, 0);\n> +\t\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch will \"\n> +\t\t\t\t\"store in remotes/%s)\", states.remote->name);\n> +\t\t\tshow_list(buf.buf, &states.new);\n> +\t\t\tstrbuf_release(&buf);\n> +\t\t\tshow_list(\" Stale tracking branch%s (use 'git remote prune')\",\n> +\t\t\t\t\t&states.stale);\n> +\t\t\tshow_list(\" Tracked remote branch%s\",\n> +\t\t\t\t\t&states.tracked);\n> +\t\t}\n> \n> \t\tif (states.remote->push_refspec_nr) {\n> \t\t\tprintf(\" Local branch%s pushed with 'git push'\\n \",\n\nMaybe we need two different values of got_states; not calling ls-remote \nand then showing things is okay, but calling ls-remote, getting an error \nand _then_ showing stuff is not okay, IMO.\n\nThanks,\nDscho\n"},{"id":"79198","messageId":"484C901B.6000401@free.fr","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806090212270.1783@racer","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T02:06:19Z","receivedAt":"2008-06-09T02:06:19Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> Would have been nice to Cc: the author of the C rewrite.\n\nSorry for that, will do it next time.\n\n>> \tstruct option options[] = {\n>> \t\tOPT_GROUP(\"show specific options\"),\n>> -\t\tOPT__DRY_RUN(&dry_run),\n>> +\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n> \n> Why?\n\nBecause I think it's something different. It's more like in \"route -n\" than --dry-run\nin \"patch --dry-run\". Don't you think ?\n\n>> +\t\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n> \n> Please rewrap.\n\nI'm not sure what you are talking about. Should I wrap after \"NULL,\" instead of \"?\"?\n\n> Maybe we need two different values of got_states; not calling ls-remote \n> and then showing things is okay, but calling ls-remote, getting an error \n> and _then_ showing stuff is not okay, IMO.\n\nIn fact, it seems that get_ref_states() always return 0 or just die when an error\noccur. And that transport_get_remote_refs() never return if something goes wrong.\n\nSo, what about removing got_states and use !no_query instead ?\n\nOlivier.\n"},{"id":"79199","messageId":"alpine.DEB.1.00.0806090330490.1783@racer","threadId":"13858","inReplyTo":"484C901B.6000401@free.fr","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T02:35:28Z","receivedAt":"2008-06-09T02:35:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n> Johannes Schindelin a écrit :\n> \n> >> \tstruct option options[] = {\n> >> \t\tOPT_GROUP(\"show specific options\"),\n> >> -\t\tOPT__DRY_RUN(&dry_run),\n> >> +\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n> > \n> > Why?\n> \n> Because I think it's something different. It's more like in \"route -n\" \n> than --dry-run in \"patch --dry-run\". Don't you think ?\n\nNo, I think that the information about stale branches and if the branches \nare up-to-date is missing. In that sense, it is not like \"route -n\" at \nall, which just skips one convenience step, but really a dry run, because \nthe result is different (as opposed to differently displayed).\n\n> >> +\t\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n> > \n> > Please rewrap.\n> \n> I'm not sure what you are talking about. Should I wrap after \"NULL,\" \n> instead of \"?\"?\n\nIt is a too long line (way over 80 characters). So yes, you should wrap \nafter the NULL here.\n\n> > Maybe we need two different values of got_states; not calling \n> > ls-remote and then showing things is okay, but calling ls-remote, \n> > getting an error and _then_ showing stuff is not okay, IMO.\n> \n> In fact, it seems that get_ref_states() always return 0 or just die when \n> an error occur. And that transport_get_remote_refs() never return if \n> something goes wrong.\n> \n> So, what about removing got_states and use !no_query instead ?\n\nHrmpf. I did not mean to die() there...\n\nCiao,\nDscho\n"},{"id":"79200","messageId":"484CAE95.3020008@free.fr","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806090330490.1783@racer","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T04:16:21Z","receivedAt":"2008-06-09T04:16:21Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> No, I think that the information about stale branches and if the branches \n> are up-to-date is missing. In that sense, it is not like \"route -n\" at \n> all, which just skips one convenience step, but really a dry run, because \n> the result is different (as opposed to differently displayed).\n\nAm I wrong if I say that dry run is for commands that modify something? For\nexample there is no \"diff --dry-run\" probably because diff does not change\nanything. A dry run has no real meaning for diff.\n\nThis the same for \"git remote show\": it's a read-only command, it just display\na summary of the remote and does not modify anything. With -n, it just skips\nthe ls-remote (read-only) step and yes the result can be different, some parts\ncan be missing. Exactly like \"route -n\", we skip the dns resolution, the host\nnames are missing.\n\nNow, if we talk about \"prune\", I completely agree. A --dry-run flag make sens.\nBut it's not the same thing than the \"show -n\" one. For what reason would I\nwant to ask \"prune\" to skip the ls-remote step? What I would find more useful\nis to make \"prune\" show what it is doing (like \"update\") and add a --dry-run\noption to say \"just show me, but do not touch anything\". And we can even add a\n-p flag to \"update\" to say \"prune at the same time\".\n\n> It is a too long line (way over 80 characters). So yes, you should wrap \n> after the NULL here.\n\nWill fix. (my tabs were only 4 spaces long)\n\n>> In fact, it seems that get_ref_states() always return 0 or just die when \n>> an error occur. And that transport_get_remote_refs() never return if \n>> something goes wrong.\n>>\n>> So, what about removing got_states and use !no_query instead ?\n> \n> Hrmpf. I did not mean to die() there...\n\nI don't understand. Is it ok or not?\n\nThanks for your comments,\nOlivier.\n"},{"id":"79202","messageId":"alpine.DEB.1.00.0806090551070.1783@racer","threadId":"13858","inReplyTo":"484CAE95.3020008@free.fr","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T04:53:10Z","receivedAt":"2008-06-09T04:53:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n> Johannes Schindelin a écrit :\n> > \n> > No, I think that the information about stale branches and if the \n> > branches are up-to-date is missing. In that sense, it is not like \n> > \"route -n\" at all, which just skips one convenience step, but really a \n> > dry run, because the result is different (as opposed to differently \n> > displayed).\n> \n> Am I wrong if I say that dry run is for commands that modify something? \n> For example there is no \"diff --dry-run\" probably because diff does not \n> change anything. A dry run has no real meaning for diff.\n\nFor me, a dry run is something that avoids the high-cost operations.\n\nSomething like, uhm, a dry run of a ship.\n\n> >> In fact, it seems that get_ref_states() always return 0 or just die \n> >> when an error occur. And that transport_get_remote_refs() never \n> >> return if something goes wrong.\n> >>\n> >> So, what about removing got_states and use !no_query instead ?\n> > \n> > Hrmpf. I did not mean to die() there...\n> \n> I don't understand. Is it ok or not?\n\nI would not like to remove the got_states. I think this is the wrong \ndirection. Rather change the die() into a return error().\n\nCiao,\nDscho\n"},{"id":"79227","messageId":"484D3C90.2050009@free.fr","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806090551070.1783@racer","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T14:22:08Z","receivedAt":"2008-06-09T14:22:08Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> For me, a dry run is something that avoids the high-cost operations.\n\nSee:\n http://en.wikipedia.org/wiki/Dry_Run_(testing)\n http://www.askoxford.com/concise_oed/dryrun?view=uk\n http://encarta.msn.com/dictionary_1861689507/dry_run.html\n\nIt's more \"do something as it was for real but it's not\". It has nothing\nto do with high-cost operations or something like that.\n\nYes?\n\n> I would not like to remove the got_states. I think this is the wrong \n> direction. Rather change the die() into a return error().\n\nOK, I will try that.\n\n-- \nOlivier.\n"},{"id":"79236","messageId":"484D4FB3.2090309@freesurf.fr","threadId":"13858","inReplyTo":"484D3C90.2050009@free.fr","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr@freesurf.fr","sentAt":"2008-06-09T15:43:47Z","receivedAt":"2008-06-09T15:43:47Z","isPatch":true,"sender":{"key":"dkr@freesurf.fr","avatar":null},"body":"Olivier Marin a écrit :\n> Johannes Schindelin a écrit :\n> \n>> I would not like to remove the got_states. I think this is the wrong \n>> direction. Rather change the die() into a return error().\n> \n> OK, I will try that.\n> \n\nAfter reading some more code, I can say that changing die() in return\nerror() won't change anything here because, in get_ref_states() we only\ndie() if get_fetch_map() return an error. But guess what, get_fetch_map()\nnever return an error. It just die() or return 0. And I can't change it\nwithout breaking \"clone\" and \"fetch\".\n\nSo, what I think is:\n\n Those changes are not in the scope of my patch. I can provide an other\n one for that, if you really care about. But IMHO it's not a problem to\n die(). Maybe we can simply remove the if () die().\n\n I will send a v2 patch with the changes we both agree.\n\nOlivier.\n"},{"id":"79237","messageId":"484D5322.6050309@free.fr","threadId":"13858","inReplyTo":"484C7DCC.6080303@free.fr","subject":"[PATCH v2] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T15:58:26Z","receivedAt":"2008-06-09T15:58:26Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThe perl version accepted a -n flag, to show local informations only\nwithout querying remote heads, that seems to have been lost in the C\nrevrite.\n\nThis restores the older behaviour and add a test case.\n\nSigned-off-by: Olivier Marin \n---\n builtin-remote.c | 44 +++++++++++++++++++++++---------------------\n t/t5505-remote.sh | 17 +++++++++++++++++\n 2 files changed, 40 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex c49f00f..efe74c7 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n \n static int show_or_prune(int argc, const char **argv, int prune)\n {\n-\tint dry_run = 0, result = 0;\n+\tint no_query = 0, result = 0;\n \tstruct option options[] = {\n \t\tOPT_GROUP(\"show specific options\"),\n-\t\tOPT__DRY_RUN(&dry_run),\n+\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n \t\tOPT_END()\n \t};\n \tstruct ref_states states;\n@@ -442,21 +442,23 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\tstruct transport *transport;\n \t\tconst struct ref *ref;\n \t\tstruct strbuf buf;\n-\t\tint i, got_states;\n+\t\tint i;\n \n \t\tstates.remote = remote_get(*argv);\n \t\tif (!states.remote)\n \t\t\treturn error(\"No such remote: %s\", *argv);\n-\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n-\t\t\tstates.remote->url[0] : NULL);\n-\t\tref = transport_get_remote_refs(transport);\n-\t\ttransport_disconnect(transport);\n \n \t\tread_branches();\n-\t\tgot_states = get_ref_states(ref, &states);\n-\t\tif (got_states)\n-\t\t\tresult = error(\"Error getting local info for '%s'\",\n-\t\t\t\t\tstates.remote->name);\n+\n+\t\tif (!no_query) {\n+\t\t\ttransport = transport_get(NULL,\n+\t\t\t\tstates.remote->url_nr > 0 ?\n+\t\t\t\tstates.remote->url[0] : NULL);\n+\t\t\tref = transport_get_remote_refs(transport);\n+\t\t\ttransport_disconnect(transport);\n+\n+\t\t\tget_ref_states(ref, &states);\n+\t\t}\n \n \t\tif (prune) {\n \t\t\tfor (i = 0; i < states.stale.nr; i++) {\n@@ -486,17 +488,17 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\t\tprintf(\"\\n\");\n \t\t}\n \n-\t\tif (got_states)\n-\t\t\tcontinue;\n-\t\tstrbuf_init(&buf, 0);\n-\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch will \"\n-\t\t\t\"store in remotes/%s)\", states.remote->name);\n-\t\tshow_list(buf.buf, &states.new);\n-\t\tstrbuf_release(&buf);\n-\t\tshow_list(\" Stale tracking branch%s (use 'git remote prune')\",\n-\t\t\t\t&states.stale);\n-\t\tshow_list(\" Tracked remote branch%s\",\n+\t\tif (!no_query) {\n+\t\t\tstrbuf_init(&buf, 0);\n+\t\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch \"\n+\t\t\t\t\"will store in remotes/%s)\", states.remote->name);\n+\t\t\tshow_list(buf.buf, &states.new);\n+\t\t\tstrbuf_release(&buf);\n+\t\t\tshow_list(\" Stale tracking branch%s (use 'git remote \"\n+\t\t\t\t\"prune')\", &states.stale);\n+\t\t\tshow_list(\" Tracked remote branch%s\",\n \t\t\t\t&states.tracked);\n+\t\t}\n \n \t\tif (states.remote->push_refspec_nr) {\n \t\t\tprintf(\" Local branch%s pushed with 'git push'\\n \",\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 0d7ed1f..c6a7bfb 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -138,6 +138,23 @@ test_expect_success 'show' '\n \t test_cmp expect output)\n '\n \n+cat > test/expect << EOF\n+* remote origin\n+ URL: $(pwd)/one/.git\n+ Remote branch merged with 'git pull' while on branch master\n+ master\n+ Local branches pushed with 'git push'\n+ master:upstream +refs/tags/lastbackup\n+EOF\n+\n+test_expect_success 'show -n' '\n+\t(mv one one.unreachable &&\n+\t cd test &&\n+\t git remote show -n origin > output &&\n+\t mv ../one.unreachable ../one &&\n+\t test_cmp expect output)\n+'\n+\n test_expect_success 'prune' '\n \t(cd one &&\n \t git branch -m side side2) &&\n"},{"id":"79240","messageId":"alpine.DEB.1.00.0806091729020.1783@racer","threadId":"13858","inReplyTo":"484D4FB3.2090309@freesurf.fr","subject":"Re: [PATCH] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T16:31:31Z","receivedAt":"2008-06-09T16:31:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n> Olivier Marin a écrit :\n> > Johannes Schindelin a écrit :\n> > \n> >> I would not like to remove the got_states. I think this is the wrong \n> >> direction. Rather change the die() into a return error().\n> > \n> > OK, I will try that.\n> > \n> \n> After reading some more code, I can say that changing die() in return \n> error() won't change anything here because, in get_ref_states() we only \n> die() if get_fetch_map() return an error. But guess what, \n> get_fetch_map() never return an error. It just die() or return 0. And I \n> can't change it without breaking \"clone\" and \"fetch\".\n\nSo you think it is okay, because the result is the same? I think not. I \nthink this is exactly the way of thinking that makes reusing unlibified \nparts of Git's source code hard. I think that this is exactly the style \nof programming I try to avoid, because it messes up clean concepts.\n\nAnd I am utterly embarassed that we are talking about my code here.\n\nCiao,\nDscho\n"},{"id":"79241","messageId":"alpine.DEB.1.00.0806091733230.1783@racer","threadId":"13858","inReplyTo":"484D5322.6050309@free.fr","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T16:35:13Z","receivedAt":"2008-06-09T16:35:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n> diff --git a/builtin-remote.c b/builtin-remote.c\n> index c49f00f..efe74c7 100644\n> --- a/builtin-remote.c\n> +++ b/builtin-remote.c\n> @@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n> \n> static int show_or_prune(int argc, const char **argv, int prune)\n> {\n> -\tint dry_run = 0, result = 0;\n> +\tint no_query = 0, result = 0;\n\nJust for the record (not that I think anybody will care): I do not like \nthis change.\n\n> @@ -442,21 +442,23 @@ static int show_or_prune(int argc, const char **argv, int prune)\n> \t\tstruct transport *transport;\n> \t\tconst struct ref *ref;\n> \t\tstruct strbuf buf;\n> -\t\tint i, got_states;\n> +\t\tint i;\n> \n> \t\tstates.remote = remote_get(*argv);\n> \t\tif (!states.remote)\n> \t\t\treturn error(\"No such remote: %s\", *argv);\n> -\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n> -\t\t\tstates.remote->url[0] : NULL);\n> -\t\tref = transport_get_remote_refs(transport);\n> -\t\ttransport_disconnect(transport);\n> \n> \t\tread_branches();\n> -\t\tgot_states = get_ref_states(ref, &states);\n> -\t\tif (got_states)\n> -\t\t\tresult = error(\"Error getting local info for '%s'\",\n> -\t\t\t\t\tstates.remote->name);\n\nAnd I do not like this change either. It proliferates the \"we just die() \nand do not care about reusing the code where die()ing is not desired\" \nparadigm.\n\nSad,\nDscho\n"},{"id":"79244","messageId":"484D6128.1010800@freesurf.fr","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806091733230.1783@racer","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr@freesurf.fr","sentAt":"2008-06-09T16:58:16Z","receivedAt":"2008-06-09T16:58:16Z","isPatch":true,"sender":{"key":"dkr@freesurf.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> And I do not like this change either. It proliferates the \"we just die() \n> and do not care about reusing the code where die()ing is not desired\" \n> paradigm.\n\nI agree and I'm OK to try to do something about that. But not in that patch.\n\nThis patch is just to fix a regression.\n\nOlivier.\n"},{"id":"79248","messageId":"alpine.DEB.1.00.0806091856180.1783@racer","threadId":"13858","inReplyTo":"484D6128.1010800@freesurf.fr","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T17:56:56Z","receivedAt":"2008-06-09T17:56:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n> Johannes Schindelin a écrit :\n> > \n> > And I do not like this change either. It proliferates the \"we just \n> > die() and do not care about reusing the code where die()ing is not \n> > desired\" paradigm.\n> \n> I agree and I'm OK to try to do something about that. But not in that patch.\n> \n> This patch is just to fix a regression.\n\nBut did you not now make it harder to fix \"that\"? By relying on the die() \nbehaviour in your regression fix?\n\nWhatever,\nDscho\n"},{"id":"79250","messageId":"484D7860.6050301@free.fr","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806091856180.1783@racer","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T18:37:20Z","receivedAt":"2008-06-09T18:37:20Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> But did you not now make it harder to fix \"that\"? By relying on the die() \n> behaviour in your regression fix?\n\nIf I change return path for some functions, I will have to check all the\ncallers anyway. So, no I don't think it make things harder to fix. Also\nI don't like to add dead code.\n\nPlease, let me do this fix so that I can post my next patches. After that\nI will be happy to work on what you asked.\n\nOlivier.\n"},{"id":"79258","messageId":"alpine.DEB.1.00.0806092110020.1783@racer","threadId":"13858","inReplyTo":"484D7860.6050301@free.fr","subject":"[PATCH] builtin-remote: make reuse of code easier by not die()ing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T20:11:37Z","receivedAt":"2008-06-09T20:11:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nBy mistake, this programmer used a die() call when an error() was much\nmore appropriate. Code reuse was not possible, hence this fix.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Mon, 9 Jun 2008, Olivier Marin wrote:\n\n\t> Johannes Schindelin a écrit :\n\t> > \n\t> > But did you not now make it harder to fix \"that\"? By relying \n\t> > on the die() behaviour in your regression fix?\n\t> \n\t> If I change return path for some functions, I will have to check \n\t> all the callers anyway. So, no I don't think it make things harder to \n\t> fix. Also I don't like to add dead code.\n\t> \n\t> Please, let me do this fix so that I can post my next patches. \n\t> After that I will be happy to work on what you asked.\n\n\tWow, that patch was hard ;-)\n\n\tBTW this thread shows -- again -- how hard it is to push toward \n\tlibification. People seem to actively block it.\n\n builtin-remote.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 2641e20..9939c96 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -243,7 +243,7 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)\n \n \tfor (i = 0; i < states->remote->fetch_refspec_nr; i++)\n \t\tif (get_fetch_map(ref, states->remote->fetch + i, &tail, 1))\n-\t\t\tdie(\"Could not get fetch map for refspec %s\",\n+\t\t\treturn error(\"Could not get fetch map for refspec %s\",\n \t\t\t\tstates->remote->fetch_refspec[i]);\n \n \tstates->new.strdup_paths = states->tracked.strdup_paths = 1;\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"79265","messageId":"484D95E4.4090903@free.fr","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806092110020.1783@racer","subject":"Re: [PATCH] builtin-remote: make reuse of code easier by not die()ing","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-09T20:43:16Z","receivedAt":"2008-06-09T20:43:16Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> \tWow, that patch was hard ;-)\n\nThis is just what you wanted? I'm a little disappointed. ;-)\n\n> \tBTW this thread shows -- again -- how hard it is to push toward \n> \tlibification. People seem to actively block it.\n\nI don't see how this patch will solve the real problem. You just hide the die()\nbecause get_fetch_map() still can die() and you add dead code. Now, the next\nperson that will use get_ref_states() will think it always return. Seems worse\nto me.\n\nIMHO, if you really want to libify you have to really analyze what should be\ndone. Split the work in coherent steps, write some specs to explain where you\nwant to go, how you planed to do it and why?\n\nIf you want to go this way, I'm ready to help you to do the hard work.\n\nOlivier.\n"},{"id":"79307","messageId":"7vd4mqdrhi.fsf@gitster.siamese.dyndns.org","threadId":"13858","inReplyTo":"alpine.DEB.1.00.0806091733230.1783@racer","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-10T01:10:49Z","receivedAt":"2008-06-10T01:10:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Mon, 9 Jun 2008, Olivier Marin wrote:\n>\n>> diff --git a/builtin-remote.c b/builtin-remote.c\n>> index c49f00f..efe74c7 100644\n>> --- a/builtin-remote.c\n>> +++ b/builtin-remote.c\n>> @@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n>> \n>> static int show_or_prune(int argc, const char **argv, int prune)\n>> {\n>> -\tint dry_run = 0, result = 0;\n>> +\tint no_query = 0, result = 0;\n>\n> Just for the record (not that I think anybody will care): I do not like \n> this change.\n\nI do not think nobody cares ;-).\n\nAt least I care enough to point out that I think you are wrong in this\ncase. \"show -n\" in the scripted version was never about \"dry-run\" but\nwas about \"no-query\".\n\nThe problem with the area of the code this patch touches is that compared\nto the scripted version, show and prune now share their codepaths a bit\nmore, and it is less easy to keep -n disabled for prune (I think it is a\nnonsense because you cannot \"prune\" sensibly without looking at what the\nremote has. It was a bug in the scripted version and losing it in C\nrewrite was a \"silent bugfix\") while resurrecting -n for show (which is a\nquick way to view where the URL points at without bothering to see what\nremote branches there are).\n\nI think a sensible thing to do would be to:\n\n - Agree that \"-n\" in the sense that \"do not query\" and \"--dry-run\" in the\n sense that \"do not do anything but report what you would do\" are\n different options.\n\n - Resurrect \"show -n\" as a quick way to view URLs without bothering to\n contact the remote end that is needed to show \"the tracked branches\"\n information.\n\n - Forbid \"prune -n\", which is nonsense.\n\n - Make \"prune --dry-run\" truly useful --- contact the other end, and\n report what will be pruned without really pruning them.\n\n - Perhaps as an enhancement, \"show -n\" could show what tracking branches\n we have from the remote, even though the information may be stale.\n The scripted version did not do this, I think, and it would be an\n improvement.\n\nI am CC'ing Shawn who authored 859607d (Teach 'git remote' how to cleanup\nstale tracking branches., 2007-02-02) to give him a chance to point out\nwhy I am wrong in saying \"prune -n\" is nonsense. Maybe there is a valid\nuse case for that option, even though I do not see one.\n"},{"id":"79309","messageId":"20080610011913.GA11793@spearce.org","threadId":"13858","inReplyTo":"7vd4mqdrhi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-10T01:19:13Z","receivedAt":"2008-06-10T01:19:13Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> At least I care enough to point out that I think you are wrong in this\n> case. \"show -n\" in the scripted version was never about \"dry-run\" but\n> was about \"no-query\".\n...\n> I am CC'ing Shawn who authored 859607d (Teach 'git remote' how to cleanup\n> stale tracking branches., 2007-02-02) to give him a chance to point out\n> why I am wrong in saying \"prune -n\" is nonsense. Maybe there is a valid\n> use case for that option, even though I do not see one.\n\nI agree with you Junio. \"prune -n\" is nonsense. You cannot know\nwhat to remove locally that the remote no longer advertises without\nquerying the remote.\n\nSo \"prune -n\" is nonsense and should issue an error. \"prune --dry-run\"\nis different and means \"query, show what you would delete, but don't\nactually delete\".\n\nLikewise \"show --dry-run\" is nonsense. What does it mean to show\nwhat would show without showing it? Just show it. :)\n\n-- \nShawn.\n"},{"id":"79311","messageId":"alpine.DEB.1.00.0806100338530.1783@racer","threadId":"13858","inReplyTo":"20080610011913.GA11793@spearce.org","subject":"Re: [PATCH v2] remote show: fix the -n option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-10T02:39:25Z","receivedAt":"2008-06-10T02:39:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Shawn O. Pearce wrote:\n\n> Likewise \"show --dry-run\" is nonsense. What does it mean to show what \n> would show without showing it? Just show it. :)\n\nAh, that clarifies it.\n\nCiao,\nDscho\n"},{"id":"79348","messageId":"1213109413-6842-1-git-send-email-dkr+ml.git@free.fr","threadId":"13858","inReplyTo":"7vd4mqdrhi.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/4] remote show/prune improvement","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T14:50:13Z","receivedAt":"2008-06-10T14:50:13Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nJunio C Hamano a écrit :\n> Johannes Schindelin writes:\n> \n>> On Mon, 9 Jun 2008, Olivier Marin wrote:\n>>\n>>> diff --git a/builtin-remote.c b/builtin-remote.c\n>>> index c49f00f..efe74c7 100644\n>>> --- a/builtin-remote.c\n>>> +++ b/builtin-remote.c\n>>> @@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n>>> \n>>> static int show_or_prune(int argc, const char **argv, int prune)\n>>> {\n>>> -\tint dry_run = 0, result = 0;\n>>> +\tint no_query = 0, result = 0;\n>> Just for the record (not that I think anybody will care): I do not like \n>> this change.\n> \n> I do not think nobody cares ;-).\n> \n> At least I care enough to point out that I think you are wrong in this\n> case. \"show -n\" in the scripted version was never about \"dry-run\" but\n> was about \"no-query\".\n> \n> The problem with the area of the code this patch touches is that compared\n> to the scripted version, show and prune now share their codepaths a bit\n> more, and it is less easy to keep -n disabled for prune (I think it is a\n> nonsense because you cannot \"prune\" sensibly without looking at what the\n> remote has. It was a bug in the scripted version and losing it in C\n> rewrite was a \"silent bugfix\") while resurrecting -n for show (which is a\n> quick way to view where the URL points at without bothering to see what\n> remote branches there are).\n> \n> I think a sensible thing to do would be to:\n> \n> - Agree that \"-n\" in the sense that \"do not query\" and \"--dry-run\" in the\n> sense that \"do not do anything but report what you would do\" are\n> different options.\n> \n> - Resurrect \"show -n\" as a quick way to view URLs without bothering to\n> contact the remote end that is needed to show \"the tracked branches\"\n> information.\n> \n> - Forbid \"prune -n\", which is nonsense.\n> \n> - Make \"prune --dry-run\" truly useful --- contact the other end, and\n> report what will be pruned without really pruning them.\n> \n> - Perhaps as an enhancement, \"show -n\" could show what tracking branches\n> we have from the remote, even though the information may be stale.\n> The scripted version did not do this, I think, and it would be an\n> improvement.\n\n [1/4] remote show: fix the -n option\n [2/4] builtin-remote: split show_or_prune() in two separate functions.\n [3/4] remote prune: print the list of pruned branches\n [4/4] remote show: list tracked remote branches with -n.\n\n Documentation/git-remote.txt | 9 +--\n builtin-remote.c | 160 ++++++++++++++++++++++++++++++------------\n t/t5505-remote.sh | 36 ++++++++++\n 3 files changed, 154 insertions(+), 51 deletions(-)\n\nOlivier.\n"},{"id":"79352","messageId":"1213109468-6906-1-git-send-email-dkr+ml.git@free.fr","threadId":"13858","inReplyTo":"1213109413-6842-1-git-send-email-dkr+ml.git@free.fr","subject":"[PATCH 1/4] remote show: fix the -n option","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T14:51:08Z","receivedAt":"2008-06-10T14:51:08Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThe perl version accepted a -n flag, to show local informations only\nwithout querying remote heads, that seems to have been lost in the C\nrevrite.\n\nThis restores the older behaviour and add a test case.\n\nSigned-off-by: Olivier Marin \n---\n Documentation/git-remote.txt | 2 +-\n builtin-remote.c | 44 +++++++++++++++++++++--------------------\n t/t5505-remote.sh | 17 ++++++++++++++++\n 3 files changed, 41 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex 782b055..7bd024e 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git-remote' [-v | --verbose]\n 'git-remote' add [-t ] [-m ] [-f] [--mirror] \n 'git-remote' rm \n-'git-remote' show \n+'git-remote' show [-n] \n 'git-remote' prune \n 'git-remote' update [group]\n \ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex c49f00f..efe74c7 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -421,10 +421,10 @@ static void show_list(const char *title, struct path_list *list)\n \n static int show_or_prune(int argc, const char **argv, int prune)\n {\n-\tint dry_run = 0, result = 0;\n+\tint no_query = 0, result = 0;\n \tstruct option options[] = {\n \t\tOPT_GROUP(\"show specific options\"),\n-\t\tOPT__DRY_RUN(&dry_run),\n+\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n \t\tOPT_END()\n \t};\n \tstruct ref_states states;\n@@ -442,21 +442,23 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\tstruct transport *transport;\n \t\tconst struct ref *ref;\n \t\tstruct strbuf buf;\n-\t\tint i, got_states;\n+\t\tint i;\n \n \t\tstates.remote = remote_get(*argv);\n \t\tif (!states.remote)\n \t\t\treturn error(\"No such remote: %s\", *argv);\n-\t\ttransport = transport_get(NULL, states.remote->url_nr > 0 ?\n-\t\t\tstates.remote->url[0] : NULL);\n-\t\tref = transport_get_remote_refs(transport);\n-\t\ttransport_disconnect(transport);\n \n \t\tread_branches();\n-\t\tgot_states = get_ref_states(ref, &states);\n-\t\tif (got_states)\n-\t\t\tresult = error(\"Error getting local info for '%s'\",\n-\t\t\t\t\tstates.remote->name);\n+\n+\t\tif (!no_query) {\n+\t\t\ttransport = transport_get(NULL,\n+\t\t\t\tstates.remote->url_nr > 0 ?\n+\t\t\t\tstates.remote->url[0] : NULL);\n+\t\t\tref = transport_get_remote_refs(transport);\n+\t\t\ttransport_disconnect(transport);\n+\n+\t\t\tget_ref_states(ref, &states);\n+\t\t}\n \n \t\tif (prune) {\n \t\t\tfor (i = 0; i < states.stale.nr; i++) {\n@@ -486,17 +488,17 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\t\tprintf(\"\\n\");\n \t\t}\n \n-\t\tif (got_states)\n-\t\t\tcontinue;\n-\t\tstrbuf_init(&buf, 0);\n-\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch will \"\n-\t\t\t\"store in remotes/%s)\", states.remote->name);\n-\t\tshow_list(buf.buf, &states.new);\n-\t\tstrbuf_release(&buf);\n-\t\tshow_list(\" Stale tracking branch%s (use 'git remote prune')\",\n-\t\t\t\t&states.stale);\n-\t\tshow_list(\" Tracked remote branch%s\",\n+\t\tif (!no_query) {\n+\t\t\tstrbuf_init(&buf, 0);\n+\t\t\tstrbuf_addf(&buf, \" New remote branch%%s (next fetch \"\n+\t\t\t\t\"will store in remotes/%s)\", states.remote->name);\n+\t\t\tshow_list(buf.buf, &states.new);\n+\t\t\tstrbuf_release(&buf);\n+\t\t\tshow_list(\" Stale tracking branch%s (use 'git remote \"\n+\t\t\t\t\"prune')\", &states.stale);\n+\t\t\tshow_list(\" Tracked remote branch%s\",\n \t\t\t\t&states.tracked);\n+\t\t}\n \n \t\tif (states.remote->push_refspec_nr) {\n \t\t\tprintf(\" Local branch%s pushed with 'git push'\\n \",\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 0d7ed1f..c6a7bfb 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -138,6 +138,23 @@ test_expect_success 'show' '\n \t test_cmp expect output)\n '\n \n+cat > test/expect << EOF\n+* remote origin\n+ URL: $(pwd)/one/.git\n+ Remote branch merged with 'git pull' while on branch master\n+ master\n+ Local branches pushed with 'git push'\n+ master:upstream +refs/tags/lastbackup\n+EOF\n+\n+test_expect_success 'show -n' '\n+\t(mv one one.unreachable &&\n+\t cd test &&\n+\t git remote show -n origin > output &&\n+\t mv ../one.unreachable ../one &&\n+\t test_cmp expect output)\n+'\n+\n test_expect_success 'prune' '\n \t(cd one &&\n \t git branch -m side side2) &&\n-- \n1.5.6.rc2.160.gd660c\n"},{"id":"79350","messageId":"1213109481-6939-1-git-send-email-dkr+ml.git@free.fr","threadId":"13858","inReplyTo":"1213109413-6842-1-git-send-email-dkr+ml.git@free.fr","subject":"[PATCH 2/4] builtin-remote: split show_or_prune() in two separate functions","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T14:51:21Z","receivedAt":"2008-06-10T14:51:21Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThis allow us to add different features to each of them and keep the\ncode simple at the same time. Also create a get_remote_ref_states()\nto avoid duplicated code.\n\nSigned-off-by: Olivier Marin \n---\n builtin-remote.c | 101 +++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 67 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex efe74c7..745a4ee 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -419,7 +419,32 @@ static void show_list(const char *title, struct path_list *list)\n \tprintf(\"\\n\");\n }\n \n-static int show_or_prune(int argc, const char **argv, int prune)\n+static int get_remote_ref_states(const char *name,\n+\t\t\t\t struct ref_states *states,\n+\t\t\t\t int query)\n+{\n+\tstruct transport *transport;\n+\tconst struct ref *ref;\n+\n+\tstates->remote = remote_get(name);\n+\tif (!states->remote)\n+\t\treturn error(\"No such remote: %s\", name);\n+\n+\tread_branches();\n+\n+\tif (query) {\n+\t\ttransport = transport_get(NULL, states->remote->url_nr > 0 ?\n+\t\t\tstates->remote->url[0] : NULL);\n+\t\tref = transport_get_remote_refs(transport);\n+\t\ttransport_disconnect(transport);\n+\n+\t\tget_ref_states(ref, states);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int show(int argc, const char **argv)\n {\n \tint no_query = 0, result = 0;\n \tstruct option options[] = {\n@@ -431,42 +456,15 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \n \targc = parse_options(argc, argv, options, builtin_remote_usage, 0);\n \n-\tif (argc < 1) {\n-\t\tif (!prune)\n-\t\t\treturn show_all();\n-\t\tusage_with_options(builtin_remote_usage, options);\n-\t}\n+\tif (argc < 1)\n+\t\treturn show_all();\n \n \tmemset(&states, 0, sizeof(states));\n \tfor (; argc; argc--, argv++) {\n-\t\tstruct transport *transport;\n-\t\tconst struct ref *ref;\n \t\tstruct strbuf buf;\n \t\tint i;\n \n-\t\tstates.remote = remote_get(*argv);\n-\t\tif (!states.remote)\n-\t\t\treturn error(\"No such remote: %s\", *argv);\n-\n-\t\tread_branches();\n-\n-\t\tif (!no_query) {\n-\t\t\ttransport = transport_get(NULL,\n-\t\t\t\tstates.remote->url_nr > 0 ?\n-\t\t\t\tstates.remote->url[0] : NULL);\n-\t\t\tref = transport_get_remote_refs(transport);\n-\t\t\ttransport_disconnect(transport);\n-\n-\t\t\tget_ref_states(ref, &states);\n-\t\t}\n-\n-\t\tif (prune) {\n-\t\t\tfor (i = 0; i < states.stale.nr; i++) {\n-\t\t\t\tconst char *refname = states.stale.items[i].util;\n-\t\t\t\tresult |= delete_ref(refname, NULL);\n-\t\t\t}\n-\t\t\tgoto cleanup_states;\n-\t\t}\n+\t\tget_remote_ref_states(*argv, &states, !no_query);\n \n \t\tprintf(\"* remote %s\\n URL: %s\\n\", *argv,\n \t\t\tstates.remote->url_nr > 0 ?\n@@ -513,7 +511,42 @@ static int show_or_prune(int argc, const char **argv, int prune)\n \t\t\t}\n \t\t\tprintf(\"\\n\");\n \t\t}\n-cleanup_states:\n+\n+\t\t/* NEEDSWORK: free remote */\n+\t\tpath_list_clear(&states.new, 0);\n+\t\tpath_list_clear(&states.stale, 0);\n+\t\tpath_list_clear(&states.tracked, 0);\n+\t}\n+\n+\treturn result;\n+}\n+\n+static int prune(int argc, const char **argv)\n+{\n+\tint no_query = 0, result = 0;\n+\tstruct option options[] = {\n+\t\tOPT_GROUP(\"prune specific options\"),\n+\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n+\t\tOPT_END()\n+\t};\n+\tstruct ref_states states;\n+\n+\targc = parse_options(argc, argv, options, builtin_remote_usage, 0);\n+\n+\tif (argc < 1)\n+\t\tusage_with_options(builtin_remote_usage, options);\n+\n+\tmemset(&states, 0, sizeof(states));\n+\tfor (; argc; argc--, argv++) {\n+\t\tint i;\n+\n+\t\tget_remote_ref_states(*argv, &states, !no_query);\n+\n+\t\tfor (i = 0; i < states.stale.nr; i++) {\n+\t\t\tconst char *refname = states.stale.items[i].util;\n+\t\t\tresult |= delete_ref(refname, NULL);\n+\t\t}\n+\n \t\t/* NEEDSWORK: free remote */\n \t\tpath_list_clear(&states.new, 0);\n \t\tpath_list_clear(&states.stale, 0);\n@@ -634,9 +667,9 @@ int cmd_remote(int argc, const char **argv, const char *prefix)\n \telse if (!strcmp(argv[0], \"rm\"))\n \t\tresult = rm(argc, argv);\n \telse if (!strcmp(argv[0], \"show\"))\n-\t\tresult = show_or_prune(argc, argv, 0);\n+\t\tresult = show(argc, argv);\n \telse if (!strcmp(argv[0], \"prune\"))\n-\t\tresult = show_or_prune(argc, argv, 1);\n+\t\tresult = prune(argc, argv);\n \telse if (!strcmp(argv[0], \"update\"))\n \t\tresult = update(argc, argv);\n \telse {\n-- \n1.5.6.rc2.160.gd660c\n"},{"id":"79351","messageId":"1213109495-6974-1-git-send-email-dkr+ml.git@free.fr","threadId":"13858","inReplyTo":"1213109413-6842-1-git-send-email-dkr+ml.git@free.fr","subject":"[PATCH 3/4] remote prune: print the list of pruned branches","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T14:51:35Z","receivedAt":"2008-06-10T14:51:35Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nThis command is really too quiet which make it unconfortable to use.\n\nAlso implement a --dry-run option, in place of the original -n one, to\nlist stale tracking branches that will be pruned, but do not actually\nprune them.\n\nAdd a test case for --dry-run.\n\nSigned-off-by: Olivier Marin \n---\n Documentation/git-remote.txt | 7 +++----\n builtin-remote.c | 20 ++++++++++++++++----\n t/t5505-remote.sh | 18 ++++++++++++++++++\n 3 files changed, 37 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex 7bd024e..345943a 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n 'git-remote' add [-t ] [-m ] [-f] [--mirror] \n 'git-remote' rm \n 'git-remote' show [-n] \n-'git-remote' prune \n+'git-remote' prune [-n | --dry-run] \n 'git-remote' update [group]\n \n DESCRIPTION\n@@ -80,9 +80,8 @@ These stale branches have already been removed from the remote repository\n referenced by , but are still locally available in\n \"remotes/\".\n +\n-With `-n` option, the remote heads are not confirmed first with `git\n-ls-remote `; cached information is used instead. Use with\n-caution.\n+With `--dry-run` option, report what branches will be pruned, but do no\n+actually prune them.\n \n 'update'::\n \ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 745a4ee..851bdde 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -521,12 +521,14 @@ static int show(int argc, const char **argv)\n \treturn result;\n }\n \n+#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n+\n static int prune(int argc, const char **argv)\n {\n-\tint no_query = 0, result = 0;\n+\tint dry_run = 0, result = 0;\n \tstruct option options[] = {\n \t\tOPT_GROUP(\"prune specific options\"),\n-\t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n+\t\tOPT__DRY_RUN(&dry_run),\n \t\tOPT_END()\n \t};\n \tstruct ref_states states;\n@@ -540,11 +542,21 @@ static int prune(int argc, const char **argv)\n \tfor (; argc; argc--, argv++) {\n \t\tint i;\n \n-\t\tget_remote_ref_states(*argv, &states, !no_query);\n+\t\tget_remote_ref_states(*argv, &states, 1);\n+\n+\t\tprintf(\"Pruning %s\\n\", *argv);\n+\t\tif (states.stale.nr)\n+\t\t\tprintf(\"From: %s\\n\", states.remote->url[0]);\n \n \t\tfor (i = 0; i < states.stale.nr; i++) {\n \t\t\tconst char *refname = states.stale.items[i].util;\n-\t\t\tresult |= delete_ref(refname, NULL);\n+\n+\t\t\tif (!dry_run)\n+\t\t\t\tresult |= delete_ref(refname, NULL);\n+\n+\t\t\tprintf(\" * %-*s %s\\n\", SUMMARY_WIDTH, \"[stale branch]\",\n+\t\t\t\trefname + strlen(\"refs/remotes/\")\n+\t\t\t\t+ strlen(*argv) + 1);\n \t\t}\n \n \t\t/* NEEDSWORK: free remote */\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex c6a7bfb..c27cfad 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -165,6 +165,24 @@ test_expect_success 'prune' '\n \t ! git rev-parse refs/remotes/origin/side)\n '\n \n+cat > test/expect << EOF\n+Pruning origin\n+From: $(pwd)/one/.git\n+ * [stale branch] side2\n+EOF\n+\n+test_expect_success 'prune --dry-run' '\n+\t(cd one &&\n+\t git branch -m side2 side) &&\n+\t(cd test &&\n+\t git remote prune --dry-run origin > output &&\n+\t git rev-parse refs/remotes/origin/side2 &&\n+\t ! git rev-parse refs/remotes/origin/side &&\n+\t(cd ../one &&\n+\t git branch -m side side2) &&\n+\t test_cmp expect output)\n+'\n+\n test_expect_success 'add --mirror && prune' '\n \t(mkdir mirror &&\n \t cd mirror &&\n-- \n1.5.6.rc2.160.gd660c\n"},{"id":"79349","messageId":"1213109509-7013-1-git-send-email-dkr+ml.git@free.fr","threadId":"13858","inReplyTo":"1213109413-6842-1-git-send-email-dkr+ml.git@free.fr","subject":"[PATCH 4/4] remote show: list tracked remote branches with -n","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T14:51:49Z","receivedAt":"2008-06-10T14:51:49Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nSigned-off-by: Olivier Marin \n---\n builtin-remote.c | 25 +++++++++++++++++++++++--\n t/t5505-remote.sh | 3 ++-\n 2 files changed, 25 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 851bdde..de4a4f2 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -444,6 +444,25 @@ static int get_remote_ref_states(const char *name,\n \treturn 0;\n }\n \n+static int append_ref_to_tracked_list(const char *refname,\n+\tconst unsigned char *sha1, int flags, void *cb_data)\n+{\n+\tstruct ref_states *states = cb_data;\n+\tstruct strbuf buf;\n+\n+\tstrbuf_init(&buf, 0);\n+\tstrbuf_addf(&buf, \"%s/\", states->remote->name);\n+\tif (strncmp(buf.buf, refname, buf.len)) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn 0;\n+\t}\n+\n+\tpath_list_append(skip_prefix(refname, strbuf_detach(&buf, NULL)),\n+\t\t&states->tracked);\n+\n+\treturn 0;\n+}\n+\n static int show(int argc, const char **argv)\n {\n \tint no_query = 0, result = 0;\n@@ -494,10 +513,12 @@ static int show(int argc, const char **argv)\n \t\t\tstrbuf_release(&buf);\n \t\t\tshow_list(\" Stale tracking branch%s (use 'git remote \"\n \t\t\t\t\"prune')\", &states.stale);\n-\t\t\tshow_list(\" Tracked remote branch%s\",\n-\t\t\t\t&states.tracked);\n \t\t}\n \n+\t\tif (no_query)\n+\t\t\tfor_each_remote_ref(append_ref_to_tracked_list, &states);\n+\t\tshow_list(\" Tracked remote branch%s\", &states.tracked);\n+\n \t\tif (states.remote->push_refspec_nr) {\n \t\t\tprintf(\" Local branch%s pushed with 'git push'\\n \",\n \t\t\t\tstates.remote->push_refspec_nr > 1 ?\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex c27cfad..ec5ea54 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -137,12 +137,13 @@ test_expect_success 'show' '\n \t git remote show origin > output &&\n \t test_cmp expect output)\n '\n-\n cat > test/expect << EOF\n * remote origin\n URL: $(pwd)/one/.git\n Remote branch merged with 'git pull' while on branch master\n master\n+ Tracked remote branch\n+ side\n Local branches pushed with 'git push'\n master:upstream +refs/tags/lastbackup\n EOF\n-- \n1.5.6.rc2.160.gd660c\n"},{"id":"79356","messageId":"m3ej75pbrw.fsf@localhost.localdomain","threadId":"13858","inReplyTo":"1213109413-6842-1-git-send-email-dkr+ml.git@free.fr","subject":"Re: [PATCH 0/4] remote show/prune improvement","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-10T15:09:31Z","receivedAt":"2008-06-10T15:09:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Olivier Marin writes:\n\n> \n> [1/4] remote show: fix the -n option\n> [2/4] builtin-remote: split show_or_prune() in two separate functions.\n> [3/4] remote prune: print the list of pruned branches\n> [4/4] remote show: list tracked remote branches with -n.\n> \n> Documentation/git-remote.txt | 9 +--\n> builtin-remote.c | 160 ++++++++++++++++++++++++++++++------------\n\nI like this series... but the [4/4] lacks documentation (all other\npatches update documentation).\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"79363","messageId":"484EA77D.7040003@free.fr","threadId":"13858","inReplyTo":"m3ej75pbrw.fsf@localhost.localdomain","subject":"Re: [PATCH 0/4] remote show/prune improvement","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T16:10:37Z","receivedAt":"2008-06-10T16:10:37Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Jakub Narebski a écrit :\n> Olivier Marin writes:\n> \n>> [1/4] remote show: fix the -n option\n>> [2/4] builtin-remote: split show_or_prune() in two separate functions.\n>> [3/4] remote prune: print the list of pruned branches\n>> [4/4] remote show: list tracked remote branches with -n.\n>>\n>> Documentation/git-remote.txt | 9 +--\n>> builtin-remote.c | 160 ++++++++++++++++++++++++++++++------------\n> \n> I like this series... but the [4/4] lacks documentation (all other\n> patches update documentation).\n> \n\nI'm not sure, it's a minor change. Perhaps, I can squashed it in 1/4 instead.\n\nWhat do you think?\n\nOlivier.\n"},{"id":"79366","messageId":"200806101911.02625.jnareb@gmail.com","threadId":"13858","inReplyTo":"484EA77D.7040003@free.fr","subject":"Re: [PATCH 0/4] remote show/prune improvement","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-10T17:11:01Z","receivedAt":"2008-06-10T17:11:01Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia wtorek 10. czerwca 2008 18:10, Olivier Marin napisał:\n> Jakub Narebski a écrit :\n>> Olivier Marin writes:\n>> \n>>> [1/4] remote show: fix the -n option\n>>> [2/4] builtin-remote: split show_or_prune() in two separate\n>>> functions. \n>>> [3/4] remote prune: print the list of pruned branches\n>>> [4/4] remote show: list tracked remote branches with -n.\n>> \n>> I like this series... but the [4/4] lacks documentation (all other\n>> patches update documentation).\n\nAh, sorry, my mistake. It looks like [4/4] is just improvement\nto [1/4], which is documented. \n \n> I'm not sure, it's a minor change. Perhaps, I can squashed it in\n> 1/4 instead. \n> \n> What do you think?\n\nPerhaps it could be, but this is not strictly necessary.\n\nAfter reading patches a bit more carefully, I think that the features\nare documented well enough, and any Documentation (and patches) \nimprovements are not necessary, and further changes can happen \"in \ntree\".\n\n\nIn \"[PATCH 1/4] remote show: fix the -n option\" you have:\n> --- a/Documentation/git-remote.txt\n> +++ b/Documentation/git-remote.txt\n[...]\n> -'git-remote' show \n> +'git-remote' show [-n] \n\nwhile in Documentation/git-remote.txt there is remainder of Perl\nimplementation\n\n 'show'::\n\n Gives some information about the remote .\n +\n With `-n` option, the remote heads are not queried first with\n `git ls-remote `; cached information is used instead.\n\nThe information about using `git ls-remote ` is no longer fully\naccurate in builtin version, and perhaps could be removed.\n\n\nIn \"[PATCH 3/4] remote prune: print the list of pruned branches\":\n> --- a/Documentation/git-remote.txt\n> +++ b/Documentation/git-remote.txt\n[...]\n> -'git-remote' prune \n> +'git-remote' prune [-n | --dry-run] \n[...]\n> -With `-n` option, the remote heads are not confirmed first with `git\n> -ls-remote `; cached information is used instead. Use with\n> -caution.\n> +With `--dry-run` option, report what branches will be pruned, but do\n> +no actually prune them.\n\nNo `git ls-remote` is mentioned there, as it should be.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"79376","messageId":"7vod69cder.fsf@gitster.siamese.dyndns.org","threadId":"13858","inReplyTo":"1213109509-7013-1-git-send-email-dkr+ml.git@free.fr","subject":"Re: [PATCH 4/4] remote show: list tracked remote branches with -n","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-10T19:12:28Z","receivedAt":"2008-06-10T19:12:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olivier Marin writes:\n\n> From: Olivier Marin \n>\n> Signed-off-by: Olivier Marin \n> ---\n> builtin-remote.c | 25 +++++++++++++++++++++++--\n> t/t5505-remote.sh | 3 ++-\n> 2 files changed, 25 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin-remote.c b/builtin-remote.c\n> index 851bdde..de4a4f2 100644\n> --- a/builtin-remote.c\n> +++ b/builtin-remote.c\n> @@ -444,6 +444,25 @@ static int get_remote_ref_states(const char *name,\n> \treturn 0;\n> }\n> \n> +static int append_ref_to_tracked_list(const char *refname,\n> +\tconst unsigned char *sha1, int flags, void *cb_data)\n> +{\n> +\tstruct ref_states *states = cb_data;\n> +\tstruct strbuf buf;\n> +\n> +\tstrbuf_init(&buf, 0);\n> +\tstrbuf_addf(&buf, \"%s/\", states->remote->name);\n> +\tif (strncmp(buf.buf, refname, buf.len)) {\n> +\t\tstrbuf_release(&buf);\n> +\t\treturn 0;\n> +\t}\n\nDoesn't this have the same issue Shawn fixed in 7ad2458 (Make \"git-remote\nrm\" delete refs acccording to fetch specs, 2008-06-01)?\n"},{"id":"79400","messageId":"484F0639.4060307@free.fr","threadId":"13858","inReplyTo":"7vod69cder.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/4] remote show: list tracked remote branches with -n","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-10T22:54:49Z","receivedAt":"2008-06-10T22:54:49Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"From: Olivier Marin \n\nSigned-off-by: Olivier Marin \n---\n\nJunio C Hamano a écrit :\n> Olivier Marin writes:\n> \n>> +static int append_ref_to_tracked_list(const char *refname,\n>> +\tconst unsigned char *sha1, int flags, void *cb_data)\n>> +{\n>> +\tstruct ref_states *states = cb_data;\n>> +\tstruct strbuf buf;\n>> +\n>> +\tstrbuf_init(&buf, 0);\n>> +\tstrbuf_addf(&buf, \"%s/\", states->remote->name);\n>> +\tif (strncmp(buf.buf, refname, buf.len)) {\n>> +\t\tstrbuf_release(&buf);\n>> +\t\treturn 0;\n>> +\t}\n> \n> Doesn't this have the same issue Shawn fixed in 7ad2458 (Make \"git-remote\n> rm\" delete refs acccording to fetch specs, 2008-06-01)?\n\nYou are right. This version should fix this.\n\n builtin-remote.c | 22 ++++++++++++++++++++--\n t/t5505-remote.sh | 2 ++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 851bdde..d55d320 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -444,6 +444,22 @@ static int get_remote_ref_states(const char *name,\n \treturn 0;\n }\n \n+static int append_ref_to_tracked_list(const char *refname,\n+\tconst unsigned char *sha1, int flags, void *cb_data)\n+{\n+\tstruct ref_states *states = cb_data;\n+\tstruct refspec refspec;\n+\n+\tmemset(&refspec, 0, sizeof(refspec));\n+\trefspec.dst = (char *)refname;\n+\tif (!remote_find_tracking(states->remote, &refspec)) {\n+\t\tpath_list_append(skip_prefix(refspec.src, \"refs/heads/\"),\n+\t\t\t&states->tracked);\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int show(int argc, const char **argv)\n {\n \tint no_query = 0, result = 0;\n@@ -494,10 +510,12 @@ static int show(int argc, const char **argv)\n \t\t\tstrbuf_release(&buf);\n \t\t\tshow_list(\" Stale tracking branch%s (use 'git remote \"\n \t\t\t\t\"prune')\", &states.stale);\n-\t\t\tshow_list(\" Tracked remote branch%s\",\n-\t\t\t\t&states.tracked);\n \t\t}\n \n+\t\tif (no_query)\n+\t\t\tfor_each_ref(append_ref_to_tracked_list, &states);\n+\t\tshow_list(\" Tracked remote branch%s\", &states.tracked);\n+\n \t\tif (states.remote->push_refspec_nr) {\n \t\t\tprintf(\" Local branch%s pushed with 'git push'\\n \",\n \t\t\t\tstates.remote->push_refspec_nr > 1 ?\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex c27cfad..fbf0d30 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -143,6 +143,8 @@ cat > test/expect << EOF\n URL: $(pwd)/one/.git\n Remote branch merged with 'git pull' while on branch master\n master\n+ Tracked remote branches\n+ master side\n Local branches pushed with 'git push'\n master:upstream +refs/tags/lastbackup\n EOF\n"},{"id":"79567","messageId":"7v63sf9lye.fsf@gitster.siamese.dyndns.org","threadId":"13858","inReplyTo":"1213109495-6974-1-git-send-email-dkr+ml.git@free.fr","subject":"Re: [PATCH 3/4] remote prune: print the list of pruned branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-12T07:00:41Z","receivedAt":"2008-06-12T07:00:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olivier Marin writes:\n\n> diff --git a/builtin-remote.c b/builtin-remote.c\n> index 745a4ee..851bdde 100644\n> --- a/builtin-remote.c\n> +++ b/builtin-remote.c\n> ... \n> +\t\tprintf(\"Pruning %s\\n\", *argv);\n> +\t\tif (states.stale.nr)\n> +\t\t\tprintf(\"From: %s\\n\", states.remote->url[0]);\n\nThanks. I've queued the series (with minor fixups and rewording) to\n'next' already, hoping that we can merge this fix to 'master' before\n1.5.6.\n\nBut I am very tempted to also apply the following on top. Thoughts?\n\n-- >8 --\n[PATCH] \"remote prune\": be quiet when there is nothing to prune\n\nThe previous commit made it always say \"Pruning $remote\" but reported the\nURL only when there is something to prune. Make it consistent by not\nsaying anything at all when there is nothing to prune.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-remote.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 4b00cf9..145dd85 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -560,12 +560,13 @@ static int prune(int argc, const char **argv)\n \n \t\tget_remote_ref_states(*argv, &states, 1);\n \n-\t\tprintf(\"Pruning %s\\n\", *argv);\n-\t\tif (states.stale.nr)\n+\t\tif (states.stale.nr) {\n+\t\t\tprintf(\"Pruning %s\\n\", *argv);\n \t\t\tprintf(\"URL: %s\\n\",\n \t\t\t states.remote->url_nr\n \t\t\t ? states.remote->url[0]\n \t\t\t : \"(no URL)\");\n+\t\t}\n \n \t\tfor (i = 0; i < states.stale.nr; i++) {\n \t\t\tconst char *refname = states.stale.items[i].util;\n-- \n1.5.6.rc2.26.g8c37\n"},{"id":"79583","messageId":"48510382.9000302@free.fr","threadId":"13858","inReplyTo":"7v63sf9lye.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] remote prune: print the list of pruned branches","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-12T11:07:46Z","receivedAt":"2008-06-12T11:07:46Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Junio C Hamano a écrit :\n> \n> Thanks. I've queued the series (with minor fixups and rewording) to\n> 'next' already, hoping that we can merge this fix to 'master' before\n> 1.5.6.\n\nThanks. I find your \"would prune/pruned\" better.\n\n> But I am very tempted to also apply the following on top. Thoughts?\n\nActually, I did that to stay consistent with \"git remote update\" and, as\na user, I prefer to see something. That said, I not opposed to your patch.\n\nOlivier.\n"}]}