{"thread":{"id":"40635","subject":"[PATCH] fetch: only show \"Fetching remote\" when verbose mode is enabled","startedAt":"2015-10-25T02:49:44Z","lastAt":"2015-10-26T02:21:45Z","messageCount":4,"participants":["Paul Wise","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"272182","messageId":"1445741384-30828-1-git-send-email-pabs3@bonedaddy.net","threadId":"40635","inReplyTo":null,"subject":"[PATCH] fetch: only show \"Fetching remote\" when verbose mode is enabled","fromName":"Paul Wise","fromEmail":"pabs3@bonedaddy.net","sentAt":"2015-10-25T02:49:44Z","receivedAt":"2015-10-25T02:49:44Z","isPatch":true,"sender":{"key":"pabs3@bonedaddy.net","avatar":"https://avatars.githubusercontent.com/u/328602?v=4"},"body":"By default when fetching one remote nothing is printed unless there\nare changes that need fetching. This makes fetching multiple remotes\nbe just as verbose as fetching a single remote.\n\nThis is needed when fetching multiple repositories using the myrepos\ntool in minimal output mode, where myrepos only prints the repository\nnames when git fetch prints some output. For example in the output below\nthe cgit and git-remote-* lines would be hidden if git fetch were\nsilent by default when fetching multiple remotes, since the default\nfor myrepos is to fetch all remotes for git repositories.\n\npabs@chianamo ~ $ mr -m fetch\nmr fetch: /home/pabs/cgit\nFetching origin\n\nmr fetch: /home/pabs/git\nFetching origin\nFrom https://github.com/git/git\n - [tag update]      junio-gpg-pub -> junio-gpg-pub\nFetching hg\nFrom https://github.com/SRabbelier/git\n - [tag update]      junio-gpg-pub -> junio-gpg-pub\n\nmr fetch: /home/pabs/git-remote-bzr\nFetching origin\n\nmr fetch: /home/pabs/git-remote-hg\nFetching origin\n\nSigned-off-by: Paul Wise <pabs3@bonedaddy.net>\n---\n builtin/fetch.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 9a3869f..fc33667 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1067,7 +1067,7 @@ static int fetch_multiple(struct string_list *list)\n \tfor (i = 0; i < list->nr; i++) {\n \t\tconst char *name = list->items[i].string;\n \t\targv_array_push(&argv, name);\n-\t\tif (verbosity >= 0)\n+\t\tif (verbosity >= 1)\n \t\t\tprintf(_(\"Fetching %s\\n\"), name);\n \t\tif (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n \t\t\terror(_(\"Could not fetch %s\"), name);\n-- \n2.6.1\n"},{"id":"272207","messageId":"xmqqwpuakd9y.fsf@gitster.mtv.corp.google.com","threadId":"40635","inReplyTo":"1445741384-30828-1-git-send-email-pabs3@bonedaddy.net","subject":"Re: [PATCH] fetch: only show \"Fetching remote\" when verbose mode is enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-25T18:34:49Z","receivedAt":"2015-10-25T18:34:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Wise <pabs3@bonedaddy.net> writes:\n\nI ignored your patch when you sent it the first time the last week,\ndue to the same issues I had with this round (see below) and forgot\nabout it.\n\n> By default when fetching one remote nothing is printed unless there\n> are changes that need fetching. This makes fetching multiple remotes\n> be just as verbose as fetching a single remote.\n\nI read this several times over a few days and I still cannot tell\nwhat you are trying to say.  Let me disect.\n\n> By default when fetching one remote nothing is printed unless there\n> are changes that need fetching.\n\nThis is the description of the current behaviour, and a correct one\nat that.  We are silent when nothing needs to be done here.\n\nI cannot tell if you are trying to say if that is problematic, or if\nyou are trying to say if it is a good thing.\n\n> This makes fetching multiple remotes\n> be just as verbose as fetching a single remote.\n\nI cannot tell what \"This\" refers to.\n\nYour earlier sentence was about the behaviour of fetching from one\nremote, e.g. \"git fetch this\".  And this second sentence makes it\nsound as if that behaviour has some influence on how verbosely \"git\nfetch group\" (where 'this' and 'that' are members of 'remotes.group')\nto fetch from multiple remotes behaves.\n\nAlso (and this is the more important part of my complaint), I cannot\ntell if you are saying that it is *bad* for fetching multiple to be\njust as verbose, or if it is *good*, or what.\n\nIf you are fetching from two places, and only one of them has\nsomething new, you would see\n\n    $ git fetch subs\n    Fetching paulus\n    Fetching l10n\n    From git://one.of.the.places.xz/\n       aadb70a..74301d6  master     -> this/master\n\nso that you can see how remote.subs group expanded to.\n   \n> This is needed when fetching multiple repositories using the myrepos\n> tool in minimal output mode, where myrepos only prints the repository\n> names when git fetch prints some output.\n\nThat does not sound like a valid excuse to change the behaviour of\nthe command everybody, not just \"myrepos tool\" (whatever it is),\nuses.  Your explanation does not seem to give us enough information\nto answer this question intelligently: shouldn't you be fixing\nmyrepos instead, perhaps by making it run 'git fetch' with more\nverbose mode, if it wants to see more information, or running 'git\nfetch' and parsing different parts of its output?\n\nHaving said all that, this time I read the change and the change\nitself feels 40% sensible, even for those who do not care about\n\"myrepos\" at all.\n\nI'd sell it like the attached, if I were doing this patch.  The last\nparagraph is where the remaining 60% went ;-)\n\n-- sample --\nSubject: fetch: do not show remote group component without change\n\nWhen fetching from a remote group, \"Fetching X\" is shown, followed\nby the URL and ref-update summary, for each component of the remote\ngroup.  With the default verbosity, the URL and ref-update summary\nare not shown for repositories without anything new, but \"Fetching\nX\" is always shown, which results in an output like this:\n\n    $ git fetch group\n    Fetching X\n    From git://the.url.for.X.repo/\n       aadb70a..74301d6  master     -> X/master\n    Fetching Y\n    Fetching Z\n\nif the 'remotes.group' is configured to refer to three remotes, X, Y\nand Z.  Given that \"git fetch Y\" would have produced no output with\nthe same verbosity, having the last two lines look inconsistent.\n\nTweak the verbosity so that \"Fetching X\" lines are given only when a\nverbose output was requested with \"git fetch -v\".\n\nNote that the current output was deliberately designed like this to\ngive an easy reminder for the user what the components of 'group'\nare.  With this change, we are selfishly and unilaterally breaking a\nfeature that was designed to help them, but if they strongly care,\nthey can complain and revert this change.\n\n-- end sample --\n"},{"id":"272212","messageId":"1445817847.23160.31.camel@bonedaddy.net","threadId":"40635","inReplyTo":"xmqqwpuakd9y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] fetch: only show \"Fetching remote\" when verbose mode is enabled","fromName":"Paul Wise","fromEmail":"pabs3@bonedaddy.net","sentAt":"2015-10-26T00:04:07Z","receivedAt":"2015-10-26T00:04:07Z","isPatch":true,"sender":{"key":"pabs3@bonedaddy.net","avatar":"https://avatars.githubusercontent.com/u/328602?v=4"},"body":"On Sun, 2015-10-25 at 11:34 -0700, Junio C Hamano wrote:\n\n> I ignored your patch when you sent it the first time the last week,\n> due to the same issues I had with this round (see below) and forgot\n> about it.\n\nThanks for the feedback this round.\n\n\n> I cannot tell if you are trying to say if that is problematic, or if\n> you are trying to say if it is a good thing.\n\nIt is just a description of the current behaviour when fetching a\nsingle remote and is used as a justification for changing the behaviour\nwhen fetching multiple remotes to match the behaviour when fetching a\nsingle remote, because that suits my myrepos use-case better.\n\n> I cannot tell what \"This\" refers to.\n\nReplace \"This\" with \"This patch\".\n\n> Your earlier sentence was about the behaviour of fetching from one\n> remote, e.g. \"git fetch this\".  And this second sentence makes it\n> sound as if that behaviour has some influence on how verbosely \"git\n> fetch group\" (where 'this' and 'that' are members of 'remotes.group')\n> to fetch from multiple remotes behaves.\n\nThere is no correlation between the verbosity of fetching one remote\nand fetching multiple remotes but I would like them to have the same\nverbosity level by default. I would like the default verbosity level to\nbe to not print anything when nothing was fetched. This is the default\nverbosity level for fetching a single remote but not multiple remotes.\n\n> Also (and this is the more important part of my complaint), I cannot\n> tell if you are saying that it is *bad* for fetching multiple to be\n> just as verbose, or if it is *good*, or what.\n\nI think it is good, as long as they both default to printing nothing\nwhen no commits/tags/etc were fetched.\n\n> If you are fetching from two places, and only one of them has\n> something new, you would see\n\nI am not interested in that case, only in the case where multiple\nremotes are being fetched and no commits/tags/etc were fetched,\nessentially I want to turn this output:\n\n$ git fetch --all\nFetching origin\nFetching mirror\n\nInto this output:\n\n$ git fetch --all\n\n> That does not sound like a valid excuse to change the behaviour of\n> the command everybody, not just \"myrepos tool\" (whatever it is),\n> uses.  Your explanation does not seem to give us enough information\n> to answer this question intelligently: shouldn't you be fixing\n> myrepos instead, perhaps by making it run 'git fetch' with more\n> verbose mode, if it wants to see more information, or running 'git\n> fetch' and parsing different parts of its output?\n\nmyrepos is a tool for managing multiple repos of different types:\n\nhttps://myrepos.branchable.com/\n\nIt definitely wouldn't be appropriate to add screen scraping and\nparsing of different version control systems to myrepos.\n\nmyrepos doesn't care about the output of repository tools beyond\nwhether there was any output or not. myrepos will pass --verbose and\nother flags on to git fetch if the user passes --verbose to it.\n\n> Having said all that, this time I read the change and the change\n> itself feels 40% sensible, even for those who do not care about\n> \"myrepos\" at all.\nAll I want is for \"Fetching remote\" to not be printed when there are no\nchanges fetched and I haven't used the --verbose option.\n\nI realise now that my patch is actually incorrect in that it also\nsuppresses \"Fetching remote\" messages when some changes were fetched.\n\nI will come back with a correct patch that is better explained.\n\n> I'd sell it like the attached, if I were doing this patch.  The last\n> paragraph is where the remaining 60% went ;-)\n\nThanks, I will try to re-use that for the next patch.\n\n> Note that the current output was deliberately designed like this to\n> give an easy reminder for the user what the components of 'group'\n> are.  With this change, we are selfishly and unilaterally breaking a\n> feature that was designed to help them, but if they strongly care,\n> they can complain and revert this change.\n\nTo be honest I didn't know this group feature existed and I am\nsurprised that anyone would want anything other than --all.\n\nI am not sure what the solution here is but perhaps the behaviour\ndemonstrated below is acceptable to users of this feature:\n\n# When no changes were fetched\n$ git fetch group\n\n# When some changes were fetched\n$ git fetch group\nFetched origin\nFrom\ngit://one.of.the.places.xz/\n       aadb70a..74301d6  master     ->\nthis/master\nNo changes from someone someoneelse otherperson\n\n# When no changes were fetched in verbose mode\n$ git fetch --verbose group\nFetching origin\nFetching someone\nFetching someoneelse\nFetching otherperson\n\n# When some changes were fetched in verbose mode\n$ git fetch --verbose group\nFetching origin\nFrom git://one.of.the.places.xz/\n       aadb70a..74301d6  master     -> this/master\nFetching someone\nFetching someoneelse\nFetching otherperson\n\n-- \nbye,\npabs\n\nhttp://bonedaddy.net/pabs3/\n\n"},{"id":"272215","messageId":"xmqqio5ujrnq.fsf@gitster.mtv.corp.google.com","threadId":"40635","inReplyTo":"1445817847.23160.31.camel@bonedaddy.net","subject":"Re: [PATCH] fetch: only show \"Fetching remote\" when verbose mode is enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-26T02:21:45Z","receivedAt":"2015-10-26T02:21:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Wise <pabs3@bonedaddy.net> writes:\n\n> It definitely wouldn't be appropriate to add screen scraping and\n> parsing of different version control systems to myrepos.\n\nHuh?  If the tool claims to support VCS X and Y and Z, it needs to\nknow how to cause X and Y and Z to \"fetch\" in their own ways, and it\nneeds to know how X and Y and Z report what they have done.\n\nThe tool can choose to be lazy and not understand the outcome.  But\nthen if the tool misbehaves because it does not know and care what\nhappened, don't come here blaming the underlying VCS.  It is the\nlaziness of the tool that causes such a bug in the tool.\n\n>> Having said all that, this time I read the change and the change\n>> itself feels 40% sensible, even for those who do not care about\n>> \"myrepos\" at all.\n> All I want is for \"Fetching remote\" to not be printed when there are no\n> changes fetched and I haven't used the --verbose option.\n>\n> I realise now that my patch is actually incorrect in that it also\n> suppresses \"Fetching remote\" messages when some changes were fetched.\n\nI actually think it makes things even worse.  I can come up with a\njustification for the change in your patch as posted (but with\nreservations, aka \"the last paragraph\"), but not with such a\nbehaviour.\n\n> I will come back with a correct patch that is better explained.\n>\n>> I'd sell it like the attached, if I were doing this patch.  The last\n>> paragraph is where the remaining 60% went ;-)\n>\n> Thanks, I will try to re-use that for the next patch.\n>\n>> Note that the current output was deliberately designed like this to\n>> give an easy reminder for the user what the components of 'group'\n>> are.  With this change, we are selfishly and unilaterally breaking a\n>> feature that was designed to help them, but if they strongly care,\n>> they can complain and revert this change.\n>\n> To be honest I didn't know this group feature existed and I am\n> surprised that anyone would want anything other than --all.\n\nI admit that I do not care too deeply myself, but I do care about\nexisting users (and possibly tools) that expect the current output,\nso if you are going to argue for discarding some information from\nthe output, I'd want to make sure that you understand what you are\ndiscarding and how they are supposed to be used and how they are\nuseful.\n\nUnfortunately I am not getting that sense from the above \"I'll show\nthe message only for the ones that were updated\".\n\nThe thing is, the story does not change an iota between --all and a\nnamed group.  Imagine you have three remotes X, Y and Z, and you\nhave a named group that contains all of them called G.  These are\nthe same:\n\n    $ git fetch G\n    $ git fetch --all\n\nFurther imagine that only X has something new.  Both of the above\nwould give you\n\n    $ git fetch --all ;# or G\n    Fetching X\n    From git://...\n        xxxxx..yyyyy master -> X/master\n    Fetching Y\n    Fetching Z\n\nRemember that the user never said \"X Y Z\" from the command line; it\nis because \"--all\" (and named group) is \"once I configure, I do not\nhave to remember each and every one every time\" mechanism.  That is\nthe crucial point.\n\nThe above output shows that the command tried to fetch from X and Y\nand Z, it got something interesting from X, *AND* it also shows that\nY and Z were unchanged.  And this last part \"Y and Z were unchanged\"\nis a useful information.  If you do not give the last two lines, the\nuser _must_ remember that --all (or G) consists of X, Y and Z, in\norder to infer that.  That defeats the whole point of --all/group to\nrelieve the user from having to remember the set of remotes and\nhaving to type them from the command line every time.\n\nContrast the following command sequence with the above:\n\n    $ git fetch X\n    From git://...\n        xxxxx..yyyyy master -> X/master\n    $ git fetch Y\n    $ git fetch Z\n\nThe responses from the command in the above sequence do not say\nanything about Y and Z and that is totally sensible.  The user\nexplicitly said Y (and Z), and the command did not say anything for\nthem--it is clear enough for the user to know Y (and Z) did not have\nanything new.\n\nSo fetching from multiple repositories with \"--all\" or group cannot\nbe fundamentally \"consistent\" with the single remote case, if you do\nnot want to lose information.\n\nOne tangent worth noting is this.  You can fetch from multiple\nrepositories that are explicitly named from the command line:\n\n    $ git fetch --multiple X Y\n    Fetching X\n    From git://...\n        xxxxx..yyyyy master -> X/master\n    Fetching Y\n\nI do think that we do not need \"Fetching Y\" for this case, and I\nactually think we shouldn't even say \"Fetching X\".\n\n> I am not sure what the solution here is but perhaps the behaviour\n> demonstrated below is acceptable to users of this feature:\n\nI do not think so.  It does not address the fact that it loses\ninformation to throw away \"Fetching ...\" message for repositories\nwith nothing new at all.  What gives us the right to break existing\nusers' expectations and force them to add --verbose on their command\nline?\n"}]}