{"thread":{"id":"39301","subject":"Bug: version 2.4 seems to have broken `git clone --progress`","startedAt":"2015-05-11T19:51:10Z","lastAt":"2015-05-12T04:30:16Z","messageCount":8,"participants":["Jack O'Connor","Junio C Hamano","Mike Hommey","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"261016","messageId":"CA+6di1=Qvp3oaw0moMg8XMFd5vgc0v1L0CC2ppEDOF8QA9N6Mg@mail.gmail.com","threadId":"39301","inReplyTo":null,"subject":"Bug: version 2.4 seems to have broken `git clone --progress`","fromName":"Jack O'Connor","fromEmail":"oconnor663@gmail.com","sentAt":"2015-05-11T19:51:10Z","receivedAt":"2015-05-11T19:51:10Z","isPatch":false,"sender":{"key":"oconnor663@gmail.com","avatar":"https://gravatar.com/avatar/221557916c71e91c6adf326e4fcc9c8bd71dd4b855a7a2ad670107ff7155330d?d=mp&s=160"},"body":"In git 2.3.7 I could run the following command and see progress in the\nterminal, despite the redirection of stdout and stderr:\n\n git clone https://github.com/oconnor663/dotfiles --progress 2>&1 | cat\n\nAs of 2.4, that command no longer shows progress. When I bisect, the\nresponsible commit is 2879bc3b0c3acc89f0415ac0d0e3946599d9fc88\n(\"transport-helper: ask the helper to set progress and verbosity\noptions after asking for its capabilities\"). Can anyone suggest a\nworkaround?\n\n-- Jack O'Connor\n"},{"id":"261019","messageId":"xmqq8ucu3jug.fsf@gitster.dls.corp.google.com","threadId":"39301","inReplyTo":"CA+6di1=Qvp3oaw0moMg8XMFd5vgc0v1L0CC2ppEDOF8QA9N6Mg@mail.gmail.com","subject":"Re: Bug: version 2.4 seems to have broken `git clone --progress`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-11T21:06:15Z","receivedAt":"2015-05-11T21:06:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jack O'Connor\" writes:\n\n> In git 2.3.7 I could run the following command and see progress in the\n> terminal, despite the redirection of stdout and stderr:\n>\n> git clone https://github.com/oconnor663/dotfiles --progress 2>&1 | cat\n>\n> As of 2.4, that command no longer shows progress. When I bisect, the\n> responsible commit is 2879bc3b0c3acc89f0415ac0d0e3946599d9fc88\n> (\"transport-helper: ask the helper to set progress and verbosity\n> options after asking for its capabilities\"). Can anyone suggest a\n> workaround?\n>\n> -- Jack O'Connor\n\nThat commit is by Mike Hommey so I'd imagine that\nCC'ing the author of the patch would be the first thing to do ;-)\n\nI am kind of surprised that the commit changes behaviour, though.\nIf I didn't hear that you had trouble in 2.4, I would have suspected\n85cb8906 (progress: no progress in background, 2015-04-13) instead.\n"},{"id":"261030","messageId":"20150512002242.GA8523@glandium.org","threadId":"39301","inReplyTo":"xmqq8ucu3jug.fsf@gitster.dls.corp.google.com","subject":"Re: Bug: version 2.4 seems to have broken `git clone --progress`","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2015-05-12T00:22:42Z","receivedAt":"2015-05-12T00:22:42Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Mon, May 11, 2015 at 02:06:15PM -0700, Junio C Hamano wrote:\n> \"Jack O'Connor\" writes:\n> \n> > In git 2.3.7 I could run the following command and see progress in the\n> > terminal, despite the redirection of stdout and stderr:\n> >\n> > git clone https://github.com/oconnor663/dotfiles --progress 2>&1 | cat\n> >\n> > As of 2.4, that command no longer shows progress. When I bisect, the\n> > responsible commit is 2879bc3b0c3acc89f0415ac0d0e3946599d9fc88\n> > (\"transport-helper: ask the helper to set progress and verbosity\n> > options after asking for its capabilities\"). Can anyone suggest a\n> > workaround?\n> >\n> > -- Jack O'Connor\n> \n> That commit is by Mike Hommey so I'd imagine that\n> CC'ing the author of the patch would be the first thing to do ;-)\n> \n> I am kind of surprised that the commit changes behaviour, though.\n> If I didn't hear that you had trouble in 2.4, I would have suspected\n> 85cb8906 (progress: no progress in background, 2015-04-13) instead.\n\nSo, the reason this is happening is that 2879bc3 moved sending the\nprogress helper option earlier, and for clone, it's early enough that\ntransport_set_verbosity happens afterwards. Since\ntransport_set_verbosity only sets the progress bit, and nothing re-emits\na helper option command when it changes, we're left with the default,\nwhich is that no progress is shown if the output file descripto is not\na tty.\n\nI can see two ways to fix this:\n- Make transport_set_verbosity call transport->set_option instead of\n defering to standard_options() in transport-helper.c.\n- Declare that transport_set_verbosity must be used before any other\n transport_set_option, and change clone to invoke it first. Note that\n fetch and push already do that, so this is only currently a problem\n for clone.\n\nJunio, what do you think?\n\nMike\n"},{"id":"261031","messageId":"xmqqfv721rh7.fsf@gitster.dls.corp.google.com","threadId":"39301","inReplyTo":"20150512002242.GA8523@glandium.org","subject":"Re: Bug: version 2.4 seems to have broken `git clone --progress`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T02:04:20Z","receivedAt":"2015-05-12T02:04:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n> So, the reason this is happening is that 2879bc3 moved sending the\n> progress helper option earlier, and for clone, it's early enough that\n> transport_set_verbosity happens afterwards. Since\n> transport_set_verbosity only sets the progress bit, and nothing re-emits\n> a helper option command when it changes, we're left with the default,\n> which is that no progress is shown if the output file descripto is not\n> a tty.\n>\n> I can see two ways to fix this:\n> - Make transport_set_verbosity call transport->set_option instead of\n> defering to standard_options() in transport-helper.c.\n> - Declare that transport_set_verbosity must be used before any other\n> transport_set_option, and change clone to invoke it first. Note that\n> fetch and push already do that, so this is only currently a problem\n> for clone.\n>\n> Junio, what do you think?\n\nThe latter sounds like more appropriate as a lower-impact short-term\nfix, so let's have that for now.\n\nI however wonder if there are other settings that can be flipped\nafter we started talking to the helper to cause a similar issue,\nand to prevent such breakages once and for all, we may have to\ntake the former route in the longer term. But I think that can be\ndone later after the dust settles.\n\nThanks for a quick diagnosis.\n"},{"id":"261035","messageId":"20150512025447.GA32735@glandium.org","threadId":"39301","inReplyTo":"xmqqfv721rh7.fsf@gitster.dls.corp.google.com","subject":"Re: Bug: version 2.4 seems to have broken `git clone --progress`","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2015-05-12T02:54:47Z","receivedAt":"2015-05-12T02:54:47Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Mon, May 11, 2015 at 07:04:20PM -0700, Junio C Hamano wrote:\n> Mike Hommey writes:\n> \n> > So, the reason this is happening is that 2879bc3 moved sending the\n> > progress helper option earlier, and for clone, it's early enough that\n> > transport_set_verbosity happens afterwards. Since\n> > transport_set_verbosity only sets the progress bit, and nothing re-emits\n> > a helper option command when it changes, we're left with the default,\n> > which is that no progress is shown if the output file descripto is not\n> > a tty.\n> >\n> > I can see two ways to fix this:\n> > - Make transport_set_verbosity call transport->set_option instead of\n> > defering to standard_options() in transport-helper.c.\n> > - Declare that transport_set_verbosity must be used before any other\n> > transport_set_option, and change clone to invoke it first. Note that\n> > fetch and push already do that, so this is only currently a problem\n> > for clone.\n> >\n> > Junio, what do you think?\n> \n> The latter sounds like more appropriate as a lower-impact short-term\n> fix, so let's have that for now.\n> \n> I however wonder if there are other settings that can be flipped\n> after we started talking to the helper to cause a similar issue,\n> and to prevent such breakages once and for all, we may have to\n> take the former route in the longer term. But I think that can be\n> done later after the dust settles.\n\nAFAICT, verbosity/progress is the only thing that is really treated\ndifferently. transport_set_options always sends options to the remote\nhelper wire.\n\nMike\n"},{"id":"261036","messageId":"1431400351-1590-1-git-send-email-mh@glandium.org","threadId":"39301","inReplyTo":"xmqqfv721rh7.fsf@gitster.dls.corp.google.com","subject":"[PATCH] clone: call transport_set_verbosity before anything else on the newly created transport","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2015-05-12T03:12:31Z","receivedAt":"2015-05-12T03:12:31Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Commit 2879bc3 made the progress and verbosity options sent to remote helper\nearlier than they previously were. But nothing else after that would send\nupdates if the value is changed later on with transport_set_verbosity.\n\nWhile for fetch and push, transport_set_verbosity is the first thing that\nis done after creating the transport, it was not the case for clone. So\ncommit 2879bc3 broke changing progress and verbosity for clone, for urls\nrequiring a remote helper only (so, not git:// urls, for instance).\n\nMoving transport_set_verbosity to just after the transport is created\nworks around the issue.\n---\n builtin/clone.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\nNote that another long term way to fix this would be to move setting\nverbosity and progress to the transport_get call itself.\n\nThe patch is against v2.4.0, if that matters.\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 53a2e5a..13030ee 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -906,6 +906,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tremote = remote_get(option_origin);\n \ttransport = transport_get(remote, remote->url[0]);\n+\ttransport_set_verbosity(transport, option_verbosity, option_progress);\n+\n \tpath = get_repo_path(remote->url[0], &is_bundle);\n \tis_local = option_local != 0 && path && !is_bundle;\n \tif (is_local) {\n@@ -932,8 +934,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_single_branch)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n \n-\ttransport_set_verbosity(transport, option_verbosity, option_progress);\n-\n \tif (option_upload_pack)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\n \t\t\t\t option_upload_pack);\n-- \n2.4.0.1.gde5e018\n"},{"id":"261037","messageId":"CAPig+cQhLKymUm2GgmFgJ+-i5O=44PJ4yWGpQFvPG_p9xa8R5g@mail.gmail.com","threadId":"39301","inReplyTo":"1431400351-1590-1-git-send-email-mh@glandium.org","subject":"Re: [PATCH] clone: call transport_set_verbosity before anything else on the newly created transport","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-12T03:33:49Z","receivedAt":"2015-05-12T03:33:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 11, 2015 at 11:12 PM, Mike Hommey wrote:\n> Commit 2879bc3 made the progress and verbosity options sent to remote helper\n> earlier than they previously were. But nothing else after that would send\n> updates if the value is changed later on with transport_set_verbosity.\n>\n> While for fetch and push, transport_set_verbosity is the first thing that\n> is done after creating the transport, it was not the case for clone. So\n> commit 2879bc3 broke changing progress and verbosity for clone, for urls\n> requiring a remote helper only (so, not git:// urls, for instance).\n>\n> Moving transport_set_verbosity to just after the transport is created\n> works around the issue.\n\nMissing sign-off.\n\n> ---\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 53a2e5a..13030ee 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -906,6 +906,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n>\n> remote = remote_get(option_origin);\n> transport = transport_get(remote, remote->url[0]);\n> + transport_set_verbosity(transport, option_verbosity, option_progress);\n> +\n> path = get_repo_path(remote->url[0], &is_bundle);\n> is_local = option_local != 0 && path && !is_bundle;\n> if (is_local) {\n> @@ -932,8 +934,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> if (option_single_branch)\n> transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n>\n> - transport_set_verbosity(transport, option_verbosity, option_progress);\n> -\n> if (option_upload_pack)\n> transport_set_option(transport, TRANS_OPT_UPLOADPACK,\n> option_upload_pack);\n> --\n> 2.4.0.1.gde5e018\n"},{"id":"261038","messageId":"1431405016-2520-1-git-send-email-mh@glandium.org","threadId":"39301","inReplyTo":"CAPig+cQhLKymUm2GgmFgJ+-i5O=44PJ4yWGpQFvPG_p9xa8R5g@mail.gmail.com","subject":"[PATCH] clone: call transport_set_verbosity before anything else on the newly created transport","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2015-05-12T04:30:16Z","receivedAt":"2015-05-12T04:30:16Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Commit 2879bc3 made the progress and verbosity options sent to remote helper\nearlier than they previously were. But nothing else after that would send\nupdates if the value is changed later on with transport_set_verbosity.\n\nWhile for fetch and push, transport_set_verbosity is the first thing that\nis done after creating the transport, it was not the case for clone. So\ncommit 2879bc3 broke changing progress and verbosity for clone, for urls\nrequiring a remote helper only (so, not git:// urls, for instance).\n\nMoving transport_set_verbosity to just after the transport is created\nworks around the issue.\n\nSigned-off-by: Mike Hommey \n---\n builtin/clone.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\nNote that another long term way to fix this would be to move setting\nverbosity and progress to the transport_get call itself.\n\nThe patch is against v2.4.0, if that matters.\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 53a2e5a..13030ee 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -906,6 +906,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tremote = remote_get(option_origin);\n \ttransport = transport_get(remote, remote->url[0]);\n+\ttransport_set_verbosity(transport, option_verbosity, option_progress);\n+\n \tpath = get_repo_path(remote->url[0], &is_bundle);\n \tis_local = option_local != 0 && path && !is_bundle;\n \tif (is_local) {\n@@ -932,8 +934,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_single_branch)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n \n-\ttransport_set_verbosity(transport, option_verbosity, option_progress);\n-\n \tif (option_upload_pack)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\n \t\t\t\t option_upload_pack);\n-- \n2.4.0.1.gde5e018\n"}]}