{"thread":{"id":"29193","subject":"[PATCH] make \"git push -v\" actually verbose","startedAt":"2011-12-17T09:37:15Z","lastAt":"2011-12-17T19:50:24Z","messageCount":4,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"181378","messageId":"20111217093713.GA2073@sigill.intra.peff.net","threadId":"29193","inReplyTo":null,"subject":"[PATCH] make \"git push -v\" actually verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-17T09:37:15Z","receivedAt":"2011-12-17T09:37:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Providing a single \"-v\" to \"git push\" currently does\nnothing. Giving two flags (\"git push -v -v\") turns on the\nfirst level of verbosity.\n\nThis is caused by a regression introduced in 8afd8dc (push:\nsupport multiple levels of verbosity, 2010-02-24). Before\nthe series containing 8afd8dc, the verbosity handling for\nfetching and pushing was completely separate. Commit bde873c\nrefactored the verbosity handling out of the fetch side, and\nthen 8afd8dc converted push to use the refactored code.\n\nHowever, the fetch and push sides numbered and passed along\ntheir verbosity levels differently. For both, a verbosity\nlevel of \"-1\" meant \"quiet\", and \"0\" meant \"default output\".\nBut from there they differed.\n\nFor fetch, a verbosity level of \"1\" indicated to the \"fetch\"\nprogram that it should make the status table slightly more\nverbose, showing up-to-date entries. A verbosity level of\n\"2\" meant that we should pass a verbose flag to the\ntransport; in the case of fetch-pack, this displays protocol\ndebugging information.\n\nAs a result, the refactored code in bde873c checks for\n\"verbosity >= 2\", and only then passes it on to the\ntransport. From the transport code's perspective, a\nverbosity of 0 or 1 both meant \"0\".\n\nPush, on the other hand, does not show its own status table;\nthat is always handled by the transport layer or below\n(originally send-pack itself, but these days it is done by\nthe transport code). So a verbosity level of 1 meant that we\nshould pass the verbose flag to send-pack, so that it knows\nwe want a verbose status table. However, once 8afd8dc\nswitched it to the refactored fetch code, a verbosity level\nof 1 was now being ignored.  Thus, you needed to\nartificially bump the verbosity to 2 (via \"-v -v\") to have\nany effect.\n\nWe can fix this by letting the transport code know about the\ntrue verbosity level (i.e., let it distinguish level 0 or\n1).\n\nWe then have to also make an adjustment to any transport\nmethods that assumed \"verbose > 0\" meant they could spew\nlots of debugging information. Before, they could only get\n\"0\" or \"2\", but now they will also receive \"1\". They need to\nadjust their condition for turning on such spew from\n\"verbose > 0\" to \"verbose > 1\".\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis is an old bug, obviously, but it has been bugging me\noff and on for the past year, so I finally decided to track\nit down.\n\nSorry for the epic-length commit message. It was a subtle\nbug, and the actual patch makes it very difficult to\nunderstand why it works and doesn't regress the fetch case.\nHopefully it made sense. :)\n\n transport.c |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 51814b5..48002b9 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -215,7 +215,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)\n \trsync.argv = args;\n \trsync.stdout_to_stderr = 1;\n \targs[0] = \"rsync\";\n-\targs[1] = (transport->verbose > 0) ? \"-rv\" : \"-r\";\n+\targs[1] = (transport->verbose > 1) ? \"-rv\" : \"-r\";\n \targs[2] = buf.buf;\n \targs[3] = temp_dir.buf;\n \targs[4] = NULL;\n@@ -268,7 +268,7 @@ static int fetch_objs_via_rsync(struct transport *transport,\n \trsync.argv = args;\n \trsync.stdout_to_stderr = 1;\n \targs[0] = \"rsync\";\n-\targs[1] = (transport->verbose > 0) ? \"-rv\" : \"-r\";\n+\targs[1] = (transport->verbose > 1) ? \"-rv\" : \"-r\";\n \targs[2] = \"--ignore-existing\";\n \targs[3] = \"--exclude\";\n \targs[4] = \"info\";\n@@ -351,7 +351,7 @@ static int rsync_transport_push(struct transport *transport,\n \targs[i++] = \"-a\";\n \tif (flags & TRANSPORT_PUSH_DRY_RUN)\n \t\targs[i++] = \"--dry-run\";\n-\tif (transport->verbose > 0)\n+\tif (transport->verbose > 1)\n \t\targs[i++] = \"-v\";\n \targs[i++] = \"--ignore-existing\";\n \targs[i++] = \"--exclude\";\n@@ -527,7 +527,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \targs.lock_pack = 1;\n \targs.use_thin_pack = data->options.thin;\n \targs.include_tag = data->options.followtags;\n-\targs.verbose = (transport->verbose > 0);\n+\targs.verbose = (transport->verbose > 1);\n \targs.quiet = (transport->verbose < 0);\n \targs.no_progress = !transport->progress;\n \targs.depth = data->options.depth;\n@@ -981,7 +981,7 @@ int transport_set_option(struct transport *transport,\n void transport_set_verbosity(struct transport *transport, int verbosity,\n \tint force_progress)\n {\n-\tif (verbosity >= 2)\n+\tif (verbosity >= 1)\n \t\ttransport->verbose = verbosity <= 3 ? verbosity : 3;\n \tif (verbosity < 0)\n \t\ttransport->verbose = -1;\n-- \n1.7.7.4.13.g57bf4\n"},{"id":"181379","messageId":"20111217094142.GA10451@sigill.intra.peff.net","threadId":"29193","inReplyTo":"20111217093713.GA2073@sigill.intra.peff.net","subject":"Re: [PATCH] make \"git push -v\" actually verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-17T09:41:42Z","receivedAt":"2011-12-17T09:41:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 17, 2011 at 04:37:15AM -0500, Jeff King wrote:\n\n> Providing a single \"-v\" to \"git push\" currently does\n> nothing. Giving two flags (\"git push -v -v\") turns on the\n> first level of verbosity.\n\nOne minor clarification: it is not technically true that \"git push -v\"\ndoes nothing. It just does not do the interesting \"show a verbose status\ntable\" operation, which is almost certainly what the user wants (and\nwhat happened before the commits I mentioned). It does print \"Pushing to\n$url\", since that happens above the transport layer. But I'm pretty sure\nthat is not what users of \"-v\" are interested in. :)\n\n-Peff\n"},{"id":"181418","messageId":"7vobv7dmak.fsf@alter.siamese.dyndns.org","threadId":"29193","inReplyTo":"20111217094142.GA10451@sigill.intra.peff.net","subject":"Re: [PATCH] make \"git push -v\" actually verbose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-17T19:34:59Z","receivedAt":"2011-12-17T19:34:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Dec 17, 2011 at 04:37:15AM -0500, Jeff King wrote:\n>\n>> Providing a single \"-v\" to \"git push\" currently does\n>> nothing. Giving two flags (\"git push -v -v\") turns on the\n>> first level of verbosity.\n>\n> One minor clarification: it is not technically true that \"git push -v\"\n> does nothing. It just does not do the interesting \"show a verbose status\n> table\" operation, which is almost certainly what the user wants (and\n> what happened before the commits I mentioned). It does print \"Pushing to\n> $url\", since that happens above the transport layer. But I'm pretty sure\n> that is not what users of \"-v\" are interested in. :)\n\nThanks, but don't be so sure about that.\n\nWhen you are so used to say \"git push ko\", from time to time you want to\ncheck which one of your kernel.org machine you are pushing into; that\n\"pushing to this exact url\" is actually quite useful.\n"},{"id":"181421","messageId":"20111217195024.GA32441@sigill.intra.peff.net","threadId":"29193","inReplyTo":"7vobv7dmak.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] make \"git push -v\" actually verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-17T19:50:24Z","receivedAt":"2011-12-17T19:50:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 17, 2011 at 11:34:59AM -0800, Junio C Hamano wrote:\n\n> > One minor clarification: it is not technically true that \"git push -v\"\n> > does nothing. It just does not do the interesting \"show a verbose status\n> > table\" operation, which is almost certainly what the user wants (and\n> > what happened before the commits I mentioned). It does print \"Pushing to\n> > $url\", since that happens above the transport layer. But I'm pretty sure\n> > that is not what users of \"-v\" are interested in. :)\n> \n> Thanks, but don't be so sure about that.\n> \n> When you are so used to say \"git push ko\", from time to time you want to\n> check which one of your kernel.org machine you are pushing into; that\n> \"pushing to this exact url\" is actually quite useful.\n\nBut we will also print that as part of the status table, so it really is\nredundant. The only difference is that it comes before the object\ntransfer phase, so if the whole thing barfs before you even make it to\nthe status table, you get a little more debugging output (although\ntypically the URL is mentioned in the die() message, anyway).\n\nE.g.:\n\n  $ git push github\n  Counting objects: 214, done.\n  Compressing objects: 100% (131/131), done.\n  Writing objects: 100% (135/135), 33.73 KiB, done.\n  Total 135 (delta 102), reused 9 (delta 4)\n  To https://github.com/peff/git.git\n   + 710a44a...0fa8107 private -> private (forced update)\n\nBefore my patch, adding \"-v\" would just put the \"Pushing to https://...\"\nbefore the object phase.\n\nAnyway, regardless of the utility of that message, I think the fix makes\nsense. It really looks like an unintended regression in 8afd8dc, and\ncertainly the original intent of the code was to match fetch's use of\n\"-v\" to show up-to-date entries in the status table (which I know\nbecause I wrote it).\n\n-Peff\n"}]}