{"thread":{"id":"25431","subject":"Push not writing to standard error","startedAt":"2010-10-12T19:04:02Z","lastAt":"2010-10-18T16:39:38Z","messageCount":10,"participants":["Chase Brammer","Jonathan Nieder","Jeff King","Junio C Hamano","Scott R. Godin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"153335","messageId":"AANLkTim6j7cXj2-1JnKdNLb8KFJK86F02tzeByDBskEa@mail.gmail.com","threadId":"25431","inReplyTo":null,"subject":"Push not writing to standard error","fromName":"Chase Brammer","fromEmail":"cbrammer@gmail.com","sentAt":"2010-10-12T19:04:02Z","receivedAt":"2010-10-12T19:04:02Z","isPatch":false,"sender":{"key":"cbrammer@gmail.com","avatar":"https://gravatar.com/avatar/96536ec36bad2565256a6f6aaee25498608816ff1334a5dfefc404cf1cf2459e?d=mp&s=160"},"body":"First time on the mailing list, but I enjoy the IRC channel.  Excuse\nme if this is a logged bug, or if there is a known workaround.\n\nWhen using git outside of bash, or saving the standard error from bash\nto a file during a push doesn't seem to be working.  I am only able to\nget standard output, which doesn't give the progress of the push\n(counting, delta, compressing, and writing status).  This does however\nwork just fine with git fetch. For example:\n\ngit fetch origin master --progress > /fetch_error_ouput.txt 2>&1\n\nWorks just fine and writes a long file with the progress data.\nHowever, the following push doesn't write any data (even when pushing\nlarge data sets to verify progress output happens)\n\ngit push origin master --progress > ~/push_error_output.txt 2>&1\n\nAs far as I can tell this is a bug with push.  I am a bit biased\nbecause I really need this feature, but it seems to me that this is a\nfairly large bug because pushing is such a pillar to all things git.\n\nIdea's on work arounds or upcoming patches to fix this?\n\nThanks\nChase Brammer\n"},{"id":"153337","messageId":"20101012192117.GD16237@burratino","threadId":"25431","inReplyTo":"AANLkTim6j7cXj2-1JnKdNLb8KFJK86F02tzeByDBskEa@mail.gmail.com","subject":"Re: Push not writing to standard error","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-12T19:21:17Z","receivedAt":"2010-10-12T19:21:17Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Chase Brammer wrote:\n\n> saving the standard error from bash\n> to a file during a push doesn't seem to be working.  I am only able to\n> get standard output, which doesn't give the progress of the push\n> (counting, delta, compressing, and writing status).\n[...]\n> git push origin master --progress > ~/push_error_output.txt 2>&1\n[...]\n> Idea's on work arounds or upcoming patches to fix this?\n\nNone from me. But some hints for a patch:\n\n - As the man page says,\n\n --progress\n\n\tProgress status is reported on the standard error stream\n\tby default when it is attached to a terminal, unless -q is\n\tspecified. This flag forces progress status even if the\n\tstandard error stream is not directed to a terminal.\n\n It looks like this facility is not working.\n\n - Terminals are distinguished from nonterminals with isatty()\n\n - The \"Counting objects...\" output comes from pack-objects.\n Running with GIT_TRACE=1 reveals that the --progress option is\n not being passed to pack-objects as it should be.\n\n - Is this a regression? If so, narrowing the regression window\n with a few rounds of \"git bisect\" could be helpful.\n\nThanks for the report.\n"},{"id":"153338","messageId":"20101012193204.GA8620@sigill.intra.peff.net","threadId":"25431","inReplyTo":"20101012192117.GD16237@burratino","subject":"Re: Push not writing to standard error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-12T19:32:04Z","receivedAt":"2010-10-12T19:32:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2010 at 02:21:17PM -0500, Jonathan Nieder wrote:\n\n> Chase Brammer wrote:\n> \n> > saving the standard error from bash\n> > to a file during a push doesn't seem to be working.  I am only able to\n> > get standard output, which doesn't give the progress of the push\n> > (counting, delta, compressing, and writing status).\n> [...]\n> > git push origin master --progress > ~/push_error_output.txt 2>&1\n> [...]\n> > Idea's on work arounds or upcoming patches to fix this?\n> \n> None from me. But some hints for a patch:\n> \n> - As the man page says,\n> \n> --progress\n> \n> \tProgress status is reported on the standard error stream\n> \tby default when it is attached to a terminal, unless -q is\n> \tspecified. This flag forces progress status even if the\n> \tstandard error stream is not directed to a terminal.\n> \n> It looks like this facility is not working.\n> \n> - Terminals are distinguished from nonterminals with isatty()\n> \n> - The \"Counting objects...\" output comes from pack-objects.\n> Running with GIT_TRACE=1 reveals that the --progress option is\n> not being passed to pack-objects as it should be.\n> \n> - Is this a regression? If so, narrowing the regression window\n> with a few rounds of \"git bisect\" could be helpful.\n\nIt looks like transport_set_verbosity gets called correctly, and then\nsets the \"progress\" flag for the transport. But for the push side, I\ndon't see any transports actually looking at that flag. I think there\nneeds to be code in git_transport_push to handle the progress flag, and\nit just isn't there.\n\n-Peff\n"},{"id":"153339","messageId":"20101012193830.GB8620@sigill.intra.peff.net","threadId":"25431","inReplyTo":"20101012193204.GA8620@sigill.intra.peff.net","subject":"Re: Push not writing to standard error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-12T19:38:30Z","receivedAt":"2010-10-12T19:38:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2010 at 03:32:04PM -0400, Jeff King wrote:\n\n> It looks like transport_set_verbosity gets called correctly, and then\n> sets the \"progress\" flag for the transport. But for the push side, I\n> don't see any transports actually looking at that flag. I think there\n> needs to be code in git_transport_push to handle the progress flag, and\n> it just isn't there.\n\nHere's a quick 5-minute patch. It works on my test case:\n\n rm -rf parent child\n git init parent &&\n git clone parent child &&\n cd child &&\n echo content >file && git add file && git commit -m one &&\n git push --progress origin master:foo >foo.out 2>&1 &&\n cat foo.out\n\nbut I didn't even run the test suite. Maybe somebody more clueful in the\narea can pick it up?\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 481602d..efd9be6 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -48,6 +48,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n \t\tNULL,\n \t\tNULL,\n \t\tNULL,\n+\t\tNULL,\n \t};\n \tstruct child_process po;\n \tint i;\n@@ -59,6 +60,8 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n \t\targv[i++] = \"--delta-base-offset\";\n \tif (args->quiet)\n \t\targv[i++] = \"-q\";\n+\tif (args->progress)\n+\t\targv[i++] = \"--progress\";\n \tmemset(&po, 0, sizeof(po));\n \tpo.argv = argv;\n \tpo.in = -1;\ndiff --git a/send-pack.h b/send-pack.h\nindex 60b4ba6..fcf4707 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -4,6 +4,7 @@\n struct send_pack_args {\n \tunsigned verbose:1,\n \t\tquiet:1,\n+\t\tprogress:1,\n \t\tporcelain:1,\n \t\tsend_mirror:1,\n \t\tforce_update:1,\ndiff --git a/transport.c b/transport.c\nindex 4dba6f8..0078660 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -789,6 +789,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \targs.use_thin_pack = data->options.thin;\n \targs.verbose = (transport->verbose > 0);\n \targs.quiet = (transport->verbose < 0);\n+\targs.progress = transport->progress;\n \targs.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n \targs.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);\n \n"},{"id":"153345","messageId":"AANLkTim_pjJ76J0ctSQO=eYsVtkZAgq2nhm0fskjjo+g@mail.gmail.com","threadId":"25431","inReplyTo":"20101012193830.GB8620@sigill.intra.peff.net","subject":"Re: Push not writing to standard error","fromName":"Chase Brammer","fromEmail":"cbrammer@gmail.com","sentAt":"2010-10-12T20:37:50Z","receivedAt":"2010-10-12T20:37:50Z","isPatch":false,"sender":{"key":"cbrammer@gmail.com","avatar":"https://gravatar.com/avatar/96536ec36bad2565256a6f6aaee25498608816ff1334a5dfefc404cf1cf2459e?d=mp&s=160"},"body":"Wow, I am amazed at how quick you churned that out. I haven't\nparticipated in the git patch and release cycle, so forgive my\nignorance. Do you think that this will be released in the next\nrelease (1.7.3.2) ? If so, any expectations on release date?\n\nChase\n\n\nOn Tue, Oct 12, 2010 at 1:38 PM, Jeff King wrote:\n>\n> On Tue, Oct 12, 2010 at 03:32:04PM -0400, Jeff King wrote:\n>\n> > It looks like transport_set_verbosity gets called correctly, and then\n> > sets the \"progress\" flag for the transport. But for the push side, I\n> > don't see any transports actually looking at that flag. I think there\n> > needs to be code in git_transport_push to handle the progress flag, and\n> > it just isn't there.\n>\n> Here's a quick 5-minute patch. It works on my test case:\n>\n>  rm -rf parent child\n>  git init parent &&\n>  git clone parent child &&\n>  cd child &&\n>  echo content >file && git add file && git commit -m one &&\n>  git push --progress origin master:foo >foo.out 2>&1 &&\n>  cat foo.out\n>\n> but I didn't even run the test suite. Maybe somebody more clueful in the\n> area can pick it up?\n>\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index 481602d..efd9be6 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -48,6 +48,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n>                NULL,\n>                NULL,\n>                NULL,\n> +               NULL,\n>        };\n>        struct child_process po;\n>        int i;\n> @@ -59,6 +60,8 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n>                argv[i++] = \"--delta-base-offset\";\n>        if (args->quiet)\n>                argv[i++] = \"-q\";\n> +       if (args->progress)\n> +               argv[i++] = \"--progress\";\n>        memset(&po, 0, sizeof(po));\n>        po.argv = argv;\n>        po.in = -1;\n> diff --git a/send-pack.h b/send-pack.h\n> index 60b4ba6..fcf4707 100644\n> --- a/send-pack.h\n> +++ b/send-pack.h\n> @@ -4,6 +4,7 @@\n>  struct send_pack_args {\n>        unsigned verbose:1,\n>                quiet:1,\n> +               progress:1,\n>                porcelain:1,\n>                send_mirror:1,\n>                force_update:1,\n> diff --git a/transport.c b/transport.c\n> index 4dba6f8..0078660 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -789,6 +789,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n>        args.use_thin_pack = data->options.thin;\n>        args.verbose = (transport->verbose > 0);\n>        args.quiet = (transport->verbose < 0);\n> +       args.progress = transport->progress;\n>        args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n>        args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);\n>\n"},{"id":"153349","messageId":"20101012204845.GA12790@sigill.intra.peff.net","threadId":"25431","inReplyTo":"AANLkTim_pjJ76J0ctSQO=eYsVtkZAgq2nhm0fskjjo+g@mail.gmail.com","subject":"Re: Push not writing to standard error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-12T20:48:46Z","receivedAt":"2010-10-12T20:48:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2010 at 02:37:50PM -0600, Chase Brammer wrote:\n\n> Wow, I am amazed at how quick you churned that out. I haven't\n> participated in the git patch and release cycle, so forgive my\n> ignorance. Do you think that this will be released in the next\n> release (1.7.3.2) ? If so, any expectations on release date?\n\nWell, at 5 minutes it was really only 1 line of code per minute. ;)\n\nI'm hoping that somebody else on the list who has worked in the\ntransport code recently can comment on whether this is the right fix.\nDid you test it? Does it fix your issue?\n\nIf it seems OK, then somebody needs to submit a cleaned-up version with\ncommit message to Junio, who will probably cook it in \"next\" for at\nleast a few weeks, and then hopefully it would be in v1.7.3.2. He does\nmaintenance releases as-needed, which seems to generally be every few\nweeks.\n\n-Peff\n"},{"id":"153351","messageId":"AANLkTikVE_iQ8nMdq7_G9aX17M3hQ4vM=oe2o=qynR-U@mail.gmail.com","threadId":"25431","inReplyTo":"20101012204845.GA12790@sigill.intra.peff.net","subject":"Re: Push not writing to standard error","fromName":"Chase Brammer","fromEmail":"cbrammer@gmail.com","sentAt":"2010-10-12T22:18:58Z","receivedAt":"2010-10-12T22:18:58Z","isPatch":false,"sender":{"key":"cbrammer@gmail.com","avatar":"https://gravatar.com/avatar/96536ec36bad2565256a6f6aaee25498608816ff1334a5dfefc404cf1cf2459e?d=mp&s=160"},"body":"Peff\n\nThanks for all the help. It worked fantastic. I hope you don't mind\nme packing this into a commit and submitting it to Junio. It is\nsomething I really need in the next release. I don't know much about\nprotocol here, and I don't want to step on toes.\n\nChase\n\n\n\nOn Tue, Oct 12, 2010 at 2:48 PM, Jeff King wrote:\n> On Tue, Oct 12, 2010 at 02:37:50PM -0600, Chase Brammer wrote:\n>\n>> Wow, I am amazed at how quick you churned that out.  I haven't\n>> participated in the git patch and release cycle, so forgive my\n>> ignorance.  Do you think that this will be released in the next\n>> release (1.7.3.2) ? If so, any expectations on release date?\n>\n> Well, at 5 minutes it was really only 1 line of code per minute. ;)\n>\n> I'm hoping that somebody else on the list who has worked in the\n> transport code recently can comment on whether this is the right fix.\n> Did you test it? Does it fix your issue?\n>\n> If it seems OK, then somebody needs to submit a cleaned-up version with\n> commit message to Junio, who will probably cook it in \"next\" for at\n> least a few weeks, and then hopefully it would be in v1.7.3.2. He does\n> maintenance releases as-needed, which seems to generally be every few\n> weeks.\n>\n> -Peff\n>\n"},{"id":"153413","messageId":"7vzkuim1zx.fsf@alter.siamese.dyndns.org","threadId":"25431","inReplyTo":"20101012193830.GB8620@sigill.intra.peff.net","subject":"Re: Push not writing to standard error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T17:33:22Z","receivedAt":"2010-10-13T17:33:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Oct 12, 2010 at 03:32:04PM -0400, Jeff King wrote:\n>\n>> It looks like transport_set_verbosity gets called correctly, and then\n>> sets the \"progress\" flag for the transport. But for the push side, I\n>> don't see any transports actually looking at that flag. I think there\n>> needs to be code in git_transport_push to handle the progress flag, and\n>> it just isn't there.\n>\n> Here's a quick 5-minute patch. It works on my test case:\n>\n> rm -rf parent child\n> git init parent &&\n> git clone parent child &&\n> cd child &&\n> echo content >file && git add file && git commit -m one &&\n> git push --progress origin master:foo >foo.out 2>&1 &&\n> cat foo.out\n\nDoes it still work with \"git push\" without --progress? I didn't apply nor\ntest, but just wondering as the manpage description suggests progress is\nimplicitly set when standard error is terminal even when there is no\ncommand line --progress is given, and also interaction with -q option, but\nthe patch does not seem to show such subtleties...\n\n>\n> but I didn't even run the test suite. Maybe somebody more clueful in the\n> area can pick it up?\n>\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index 481602d..efd9be6 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -48,6 +48,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n> \t\tNULL,\n> \t\tNULL,\n> \t\tNULL,\n> +\t\tNULL,\n> \t};\n> \tstruct child_process po;\n> \tint i;\n> @@ -59,6 +60,8 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n> \t\targv[i++] = \"--delta-base-offset\";\n> \tif (args->quiet)\n> \t\targv[i++] = \"-q\";\n> +\tif (args->progress)\n> +\t\targv[i++] = \"--progress\";\n> \tmemset(&po, 0, sizeof(po));\n> \tpo.argv = argv;\n> \tpo.in = -1;\n> diff --git a/send-pack.h b/send-pack.h\n> index 60b4ba6..fcf4707 100644\n> --- a/send-pack.h\n> +++ b/send-pack.h\n> @@ -4,6 +4,7 @@\n> struct send_pack_args {\n> \tunsigned verbose:1,\n> \t\tquiet:1,\n> +\t\tprogress:1,\n> \t\tporcelain:1,\n> \t\tsend_mirror:1,\n> \t\tforce_update:1,\n> diff --git a/transport.c b/transport.c\n> index 4dba6f8..0078660 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -789,6 +789,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n> \targs.use_thin_pack = data->options.thin;\n> \targs.verbose = (transport->verbose > 0);\n> \targs.quiet = (transport->verbose < 0);\n> +\targs.progress = transport->progress;\n> \targs.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n> \targs.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);\n> \n"},{"id":"153416","messageId":"20101013174543.GA13752@sigill.intra.peff.net","threadId":"25431","inReplyTo":"7vzkuim1zx.fsf@alter.siamese.dyndns.org","subject":"Re: Push not writing to standard error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-13T17:45:44Z","receivedAt":"2010-10-13T17:45:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 13, 2010 at 10:33:22AM -0700, Junio C Hamano wrote:\n\n> > Here's a quick 5-minute patch. It works on my test case:\n> >\n> > rm -rf parent child\n> > git init parent &&\n> > git clone parent child &&\n> > cd child &&\n> > echo content >file && git add file && git commit -m one &&\n> > git push --progress origin master:foo >foo.out 2>&1 &&\n> > cat foo.out\n> \n> Does it still work with \"git push\" without --progress? I didn't apply nor\n> test, but just wondering as the manpage description suggests progress is\n> implicitly set when standard error is terminal even when there is no\n> command line --progress is given, and also interaction with -q option, but\n> the patch does not seem to show such subtleties...\n\nYes, it works in both of those cases. The transport code already does\nthe right thing to set transport->progress (see the code at the end of\ntransport_set_verbosity). And we even pass that value on to remote\nhelpers, which presumably make use of it. But the internal\ngit_transport_push simply ignored it (probably because it predates the\nrest of the transport code, but I didn't check).\n\nWhat concerns me a bit is that \"git push --no-progress\" does not do what\nI expected (turn off progress, but keep the status table which would\notherwise be suppressed by \"-q\"). Instead, --no-progress is silently\nignored. We should at least set it to NONEG to generate an error, but\nideally we would handle it properly.\n\nHowever, that bug exists with or without my patch. The transport code\nseems to only ever consider \"force progress\" or \"default progress\", but\nnever \"no progress\".\n\n-Peff\n"},{"id":"153723","messageId":"4CBC784A.1040805@mhg2.com","threadId":"25431","inReplyTo":"AANLkTim6j7cXj2-1JnKdNLb8KFJK86F02tzeByDBskEa@mail.gmail.com","subject":"Re: Push not writing to standard error","fromName":"Scott R. Godin","fromEmail":"scottg.wp-hackers@mhg2.com","sentAt":"2010-10-18T16:39:38Z","receivedAt":"2010-10-18T16:39:38Z","isPatch":false,"sender":{"key":"scottg.wp-hackers@mhg2.com","avatar":"https://gravatar.com/avatar/766980733f46a32860153479ca94372e4fff5c4632e8586dbbfb143051d231ce?d=mp&s=160"},"body":"On 10/12/2010 03:04 PM, Chase Brammer wrote:\n> git fetch origin master --progress> /fetch_error_ouput.txt 2>&1\n\nJust as a small tip, you can shorthand this in bash using\n\tgit fech origin master --progress >& /fetch_error_output.txt\n\nHTH :)\n\n\n-- \n(please respond to the list as opposed to my email box directly,\nunless you are supplying private information you don't want public\non the list)\n"}]}