{"thread":{"id":"30378","subject":"Re: 1.7.10 doesn't show file pushstatus","startedAt":"2012-05-01T06:58:33Z","lastAt":"2012-05-02T01:04:20Z","messageCount":14,"participants":["Jeff King","Clemens Buchacher","Junio C Hamano","David Ebbo","Zbigniew Jędrzejewski-Szmek","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"190401","messageId":"20120501065832.GA17777@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501010609.GA14715@jupiter.local","subject":"Re: 1.7.10 doesn't show file pushstatus","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T06:58:33Z","receivedAt":"2012-05-01T06:58:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"> I just updated to msysGit 1.7.10 and I noticed I don't see any details\n> while pushing (like file upload speed and % completion). Was this\n> intentionally removed? If so why?\n\nNo, it's a regression. I can reproduce it easily, and it bisects to\nClemens' 01fdc21 (push/fetch/clone --no-progress suppresses progress\noutput) which went into v1.7.9.2 (and v1.7.10).\n\nThe problematic hunk is:\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 71f258e..9df341c 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -58,7 +58,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n                argv[i++] = \"--thin\";\n        if (args->use_ofs_delta)\n                argv[i++] = \"--delta-base-offset\";\n-       if (args->quiet)\n+       if (args->quiet || !args->progress)\n                argv[i++] = \"-q\";\n        if (args->progress)\n                argv[i++] = \"--progress\";\n\nwhich seems wrong to me. In send-pack, args->progress may be unset if we\ndidn't get a --progress flag on the command line. Shouldn't we be\nfalling back to isatty in that case (or leaving \"-q\" unset so that\npack-objects can do so)? Does it need to also be converted into a\ntri-state of yes/no/unknown as the other places in that patch were?\nClemens?\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190403","messageId":"20120501073326.GA21087@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501065832.GA17777@sigill.intra.peff.net","subject":"Re: 1.7.10 doesn't show file pushstatus","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T07:33:26Z","receivedAt":"2012-05-01T07:33:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 01, 2012 at 02:58:32AM -0400, Jeff King wrote:\n\n> > I just updated to msysGit 1.7.10 and I noticed I don't see any details\n> > while pushing (like file upload speed and % completion). Was this\n> > intentionally removed? If so why?\n> \n> No, it's a regression. I can reproduce it easily, and it bisects to\n> Clemens' 01fdc21 (push/fetch/clone --no-progress suppresses progress\n> output) which went into v1.7.9.2 (and v1.7.10).\n\nHmm. OK, I think I see what is going on. For most protocols, send_pack\nhappens without a separate process these days. So the stack is like:\n\n  - transport_push\n    - git_transport_push\n      - send_pack\n\nwhere the \"progress\" flag to send_pack is a boolean; we have already\nfigured out at the transport layer whether we want progress, and are\ntelling it what to do. Thus this hunk from 01fdc21 makes sense:\n\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index 71f258e..9df341c 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -58,7 +58,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n>                 argv[i++] = \"--thin\";\n>         if (args->use_ofs_delta)\n>                 argv[i++] = \"--delta-base-offset\";\n> -       if (args->quiet)\n> +       if (args->quiet || !args->progress)\n>                 argv[i++] = \"-q\";\n>         if (args->progress)\n>                 argv[i++] = \"--progress\";\n\nIf we have been asked not to have progress, then we tell pack-objects\n\"-q\" (which is, for historical reasons, the way to spell \"--no-progress\"\nfor pack-objects).\n\nBut send-pack is also a command in its own right, and gets invoked\nseparately for the --stateless-rpc case (i.e., smart http). In that case\nyou end up with:\n\n  - transport_push\n    - transport-helper.c:push_refs\n      - [pipes to git-remote-https]\n        - remote-curl.c:push_git\n          - [forks send-pack]\n            - cmd_send_pack\n              - send_pack\n\nIn this case, send pack gets its arguments from the command-line, not\nfrom the options set at the transport layer. Remote-curl will pass along\n\"--quiet\" if we get that from the transport layer, but it does not\notherwise pass along the \"progress\" flag. So there are two problems:\n\n  1. send-pack defaults its progress boolean to 0. Before 01fdc21, that\n     was OK, because it meant \"don't explicitly ask for progress\". But\n     after 01fdc21 that now means \"explicitly ask for no progress\", and\n     the direct-transport code paths were updated without updating\n     cmd_send_pack.\n\n  2. There's no way to tell send-pack explicitly \"yes, I would like\n     progress, no matter what isatty(2) says\". I doubt anybody cares\n     much, but it probably makes sense to handle that for the sake of\n     completeness.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190406","messageId":"20120501084048.GA21904@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501073326.GA21087@sigill.intra.peff.net","subject":"Re: 1.7.10 doesn't show file pushstatus","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T08:40:49Z","receivedAt":"2012-05-01T08:40:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 01, 2012 at 03:33:26AM -0400, Jeff King wrote:\n\n> In this case, send pack gets its arguments from the command-line, not\n> from the options set at the transport layer. Remote-curl will pass along\n> \"--quiet\" if we get that from the transport layer, but it does not\n> otherwise pass along the \"progress\" flag. So there are two problems:\n> \n>   1. send-pack defaults its progress boolean to 0. Before 01fdc21, that\n>      was OK, because it meant \"don't explicitly ask for progress\". But\n>      after 01fdc21 that now means \"explicitly ask for no progress\", and\n>      the direct-transport code paths were updated without updating\n>      cmd_send_pack.\n> \n>   2. There's no way to tell send-pack explicitly \"yes, I would like\n>      progress, no matter what isatty(2) says\". I doubt anybody cares\n>      much, but it probably makes sense to handle that for the sake of\n>      completeness.\n\nThe following patch series fixes this:\n\n  [1/3]: send-pack: show progress when isatty(2)\n  [2/3]: teach send-pack about --[no-]progress\n  [3/3]: t5541: test more combinations of --progress\n\nThe first patch fixes (1) above, restoring send-pack's original behavior\n(for remote-curl as well as anybody else who happens to call it). Note\nthat in doing so, it breaks \"push --no-progress\" for http remotes that\n01fdc21 tried to fix. But it didn't actually fix it; it only appeared to\nwork because progress was _never_ on for http.  Fortunately, the\nexisting test in t5541 still passes because it's poorly written (it uses\nboth \"--quiet\" and \"--no-progress\", unmindful of the fact that the\nlatter does absolutely nothing).\n\nThe second patch fixes it correctly by propagating the options through\nremote-curl (and as a bonus, it makes (2) above work). Other transport\nhelpers that use send-pack would want to do the same thing (but I don't\nknow of any that exist).\n\nThe third patch just expands the test coverage.\n\nThese are prepared directly on top of 01fdc21, so they can go on the\nmaint track.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190407","messageId":"20120501084139.GA4998@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501084048.GA21904@sigill.intra.peff.net","subject":"[PATCH 1/3] send-pack: show progress when isatty(2)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T08:41:42Z","receivedAt":"2012-05-01T08:41:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The send_pack_args struct has two verbosity flags: \"quiet\"\nand \"progress\". Originally, if \"quiet\" was set, we would\ntell pack-objects explicitly to be quiet, and if \"progress\"\nwas set, we would tell it to show progress. Otherwise, we\ntold it neither, and it relied on isatty(2) to make the\ndecision itself.\n\nHowever, commit 01fdc21 changed the meaning of these\nvariables. Now both \"quiet\" and \"!progress\" instruct us to\ntell pack-objects to be quiet (and a non-zero \"progress\"\nmeans the same as before). This works well for transports\nwhich call send_pack directly, as the transport code copies\ntransport->progress into send_pack_args->progress, and they\nboth have the same meaning.\n\nHowever, the code path of calling \"git send-pack\" was left\nbehind. It always sets \"progress\" to 0, and thus always\ntells pack-objects to be quiet.  We can work around this by\nchecking isatty(2) ourselves in the cmd_send_pack code path,\nrestoring the original behavior of the send-pack command.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/send-pack.c |    3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 9df341c..7d22715 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -492,6 +492,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\tif (!args.quiet)\n+\t\targs.progress = isatty(2);\n+\n \tif (args.stateless_rpc) {\n \t\tconn = NULL;\n \t\tfd[0] = 0;\n-- \n1.7.10.630.g31718\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190408","messageId":"20120501084224.GB4998@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501084048.GA21904@sigill.intra.peff.net","subject":"[PATCH 2/3] teach send-pack about --[no-]progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T08:42:24Z","receivedAt":"2012-05-01T08:42:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The send_pack function gets a \"progress\" flag saying \"yes,\ndefinitely show progress\" or \"no, definitely do not show\nprogress\". This gets set properly by transport_push when\nsend_pack is called directly.\n\nHowever, when the send-pack command is executed separately\n(as it is for the remote-curl helper), there is no way to\ntell it \"definitely do this\". As a result, we do not\nproperly respect \"git push --no-progress\" for smart-http\nremotes; you will still get progress if stderr is a tty.\n\nThis patch teaches send-pack --progress and --no-progress,\nand teaches remote-curl to pass the appropriate option to\noverride send-pack's isatty check. This fixes the\n--no-progress case above, and as a bonus, also makes \"git\npush --progress\" work when stderr is not a tty.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/send-pack.c |   14 ++++++++++++--\n remote-curl.c       |    1 +\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 7d22715..d5d7105 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -410,6 +410,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tconst char *receivepack = \"git-receive-pack\";\n \tint flags;\n \tint nonfastforward = 0;\n+\tint progress = -1;\n \n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\n@@ -452,6 +453,14 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\targs.verbose = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--progress\")) {\n+\t\t\t\tprogress = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!strcmp(arg, \"--no-progress\")) {\n+\t\t\t\tprogress = 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--thin\")) {\n \t\t\t\targs.use_thin_pack = 1;\n \t\t\t\tcontinue;\n@@ -492,8 +501,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tif (!args.quiet)\n-\t\targs.progress = isatty(2);\n+\tif (progress == -1)\n+\t\tprogress = !args.quiet && isatty(2);\n+\targs.progress = progress;\n \n \tif (args.stateless_rpc) {\n \t\tconn = NULL;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex d159fe7..e5e9490 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -774,6 +774,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)\n \t\targv[argc++] = \"--quiet\";\n \telse if (options.verbosity > 1)\n \t\targv[argc++] = \"--verbose\";\n+\targv[argc++] = options.progress ? \"--progress\" : \"--no-progress\";\n \targv[argc++] = url;\n \tfor (i = 0; i < nr_spec; i++)\n \t\targv[argc++] = specs[i];\n-- \n1.7.10.630.g31718\n"},{"id":"190409","messageId":"20120501084307.GC4998@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501084048.GA21904@sigill.intra.peff.net","subject":"[PATCH 3/3] t5541: test more combinations of --progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T08:43:08Z","receivedAt":"2012-05-01T08:43:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously, we tested only that \"push --quiet --no-progress\"\nwas silent. However, there are many other combinations that\nwere not tested:\n\n  1. no options at all (but stderr as a tty)\n  2. --no-progress by itself\n  3. --quiet by itself\n  4. --progress (when stderr not a tty)\n\nThese are tested elsewhere for general \"push\", but it is\nimportant to test them separately for http. It follows a\nvery different code path than git://, and options must be\nrelayed across a remote helper to a separate send-pack\nprocess (and in fact cases (1), (2), and (4) have all been\nbroken just for http at some point in the past).\n\nWe can drop the \"--quiet --no-progress\" test, as it is not\nreally interesting (it is already handled by testing them\nseparately in (2) and (3) above).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t5541-http-push.sh |   27 +++++++++++++++++++++++++--\n 1 file changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex d66ed24..a1b10bd 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -215,12 +215,35 @@ test_expect_success 'push --mirror to repo with alternates' '\n \tgit push --mirror \"$HTTPD_URL\"/smart/alternates-mirror.git\n '\n \n-test_expect_success TTY 'quiet push' '\n+test_expect_success TTY 'push shows progress when stderr is a tty' '\n+\tcd \"$ROOT_PATH\"/test_repo_clone &&\n+\ttest_commit noisy &&\n+\ttest_terminal git push 2>&1 | tee output &&\n+\tgrep \"^Writing objects\" output\n+'\n+\n+test_expect_success TTY 'push --quiet silences status and progress' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_commit quiet &&\n-\ttest_terminal git push --quiet --no-progress 2>&1 | tee output &&\n+\ttest_terminal git push --quiet 2>&1 | tee output &&\n \ttest_cmp /dev/null output\n '\n \n+test_expect_success TTY 'push --no-progress silences progress but not status' '\n+\tcd \"$ROOT_PATH\"/test_repo_clone &&\n+\ttest_commit no-progress &&\n+\ttest_terminal git push --no-progress 2>&1 | tee output &&\n+\tgrep \"^To http\" output &&\n+\t! grep \"^Writing objects\"\n+'\n+\n+test_expect_success 'push --progress shows progress to non-tty' '\n+\tcd \"$ROOT_PATH\"/test_repo_clone &&\n+\ttest_commit progress &&\n+\tgit push --progress 2>&1 | tee output &&\n+\tgrep \"^To http\" output &&\n+\tgrep \"^Writing objects\" output\n+'\n+\n stop_httpd\n test_done\n-- \n1.7.10.630.g31718\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190411","messageId":"20120501093230.GA22633@ecki.lan","threadId":"30378","inReplyTo":"20120501084048.GA21904@sigill.intra.peff.net","subject":"Re: 1.7.10 doesn't show file pushstatus","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-05-01T09:32:30Z","receivedAt":"2012-05-01T09:32:30Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Tue, May 01, 2012 at 04:40:49AM -0400, Jeff King wrote:\n> \n> The following patch series fixes this:\n> \n>   [1/3]: send-pack: show progress when isatty(2)\n>   [2/3]: teach send-pack about --[no-]progress\n>   [3/3]: t5541: test more combinations of --progress\n\nThanks. Looks good to me, although I completely missed this before. But\nthe tests should hopefully take care of any future regressions.\n"},{"id":"190412","messageId":"20120501093501.GB22633@ecki.lan","threadId":"30378","inReplyTo":"20120501084307.GC4998@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] t5541: test more combinations of --progress","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-05-01T09:35:01Z","receivedAt":"2012-05-01T09:35:01Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Can we add this on top or squashed in? I regret using tee when I\noriginally wrote the test.\n\n--8<--\nSubject: [PATCH] t5541: check return codes\n\nBy piping output to tee, the return code of the command is hidden.\nInstead, redirect output to a file directly.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n t/t5541-http-push.sh |    8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex 986210a..c07973e 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -218,21 +218,21 @@ test_expect_success 'push --mirror to repo with alternates' '\n test_expect_success TTY 'push shows progress when stderr is a tty' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_commit noisy &&\n-\ttest_terminal git push 2>&1 | tee output &&\n+\ttest_terminal git push >output 2>&1 &&\n \tgrep \"^Writing objects\" output\n '\n \n test_expect_success TTY 'push --quiet silences status and progress' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_commit quiet &&\n-\ttest_terminal git push --quiet 2>&1 | tee output &&\n+\ttest_terminal git push --quiet >output 2>&1 &&\n \ttest_cmp /dev/null output\n '\n \n test_expect_success TTY 'push --no-progress silences progress but not status' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_commit no-progress &&\n-\ttest_terminal git push --no-progress 2>&1 | tee output &&\n+\ttest_terminal git push --no-progress >output 2>&1 &&\n \tgrep \"^To http\" output &&\n \t! grep \"^Writing objects\"\n '\n@@ -240,7 +240,7 @@ test_expect_success TTY 'push --no-progress silences progress but not status' '\n test_expect_success 'push --progress shows progress to non-tty' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_commit progress &&\n-\tgit push --progress 2>&1 | tee output &&\n+\tgit push --progress >output 2>&1 &&\n \tgrep \"^To http\" output &&\n \tgrep \"^Writing objects\" output\n '\n-- \n1.7.10\n"},{"id":"190413","messageId":"20120501093719.GA7538@sigill.intra.peff.net","threadId":"30378","inReplyTo":"20120501093501.GB22633@ecki.lan","subject":"Re: [PATCH 3/3] t5541: test more combinations of --progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T09:37:19Z","receivedAt":"2012-05-01T09:37:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 01, 2012 at 11:35:01AM +0200, Clemens Buchacher wrote:\n\n> Can we add this on top or squashed in? I regret using tee when I\n> originally wrote the test.\n> \n> --8<--\n> Subject: [PATCH] t5541: check return codes\n> \n> By piping output to tee, the return code of the command is hidden.\n> Instead, redirect output to a file directly.\n\nUgh, absolutely. I blindly followed the existing style without thinking\nabout whether using tee was sane or not. And it's not. Thanks.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190446","messageId":"7vd36najrc.fsf@alter.siamese.dyndns.org","threadId":"30378","inReplyTo":"20120501093230.GA22633@ecki.lan","subject":"Re: 1.7.10 doesn't show file pushstatus","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T17:33:43Z","receivedAt":"2012-05-01T17:33:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, both.\n"},{"id":"190449","messageId":"12419089.206.1335894799764.JavaMail.geo-discussion-forums@pbcgj9","threadId":"30378","inReplyTo":"20120501093719.GA7538@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] t5541: test more combinations of --progress","fromName":"David Ebbo","fromEmail":"david.ebbo@gmail.com","sentAt":"2012-05-01T17:53:19Z","receivedAt":"2012-05-01T17:53:19Z","isPatch":true,"sender":{"key":"david.ebbo@gmail.com","avatar":"https://gravatar.com/avatar/afaf05bed461bda07768f1ab79afcbb8b56055774757f1b8968c67e97cbd61b6?d=mp&s=160"},"body":"Many thanks to Jeff King and others to help track this one down! \n\nDavid\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190478","messageId":"4FA04B4C.4090507@in.waw.pl","threadId":"30378","inReplyTo":"20120501084307.GC4998@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] t5541: test more combinations of --progress","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-05-01T20:45:00Z","receivedAt":"2012-05-01T20:45:00Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 05/01/2012 10:43 AM, Jeff King wrote:\n> -test_expect_success TTY 'quiet push' '\n> +test_expect_success TTY 'push shows progress when stderr is a tty' '\n> +\tcd \"$ROOT_PATH\"/test_repo_clone &&\n> +\ttest_commit noisy &&\n> +\ttest_terminal git push 2>&1 | tee output &&\n> +\tgrep \"^Writing objects\" output\n> +'\n> +\n> +test_expect_success TTY 'push --quiet silences status and progress' '\n>  \tcd \"$ROOT_PATH\"/test_repo_clone &&\n>  \ttest_commit quiet &&\n> -\ttest_terminal git push --quiet --no-progress 2>&1 | tee output &&\n> +\ttest_terminal git push --quiet 2>&1 | tee output &&\n>  \ttest_cmp /dev/null output\n>  '\n>  \n> +test_expect_success TTY 'push --no-progress silences progress but not status' '\n> +\tcd \"$ROOT_PATH\"/test_repo_clone &&\n> +\ttest_commit no-progress &&\n> +\ttest_terminal git push --no-progress 2>&1 | tee output &&\n> +\tgrep \"^To http\" output &&\n> +\t! grep \"^Writing objects\"\n        ! grep \"^Writing objects\" output\n\n> +'\n> +\n> +test_expect_success 'push --progress shows progress to non-tty' '\n> +\tcd \"$ROOT_PATH\"/test_repo_clone &&\n> +\ttest_commit progress &&\n> +\tgit push --progress 2>&1 | tee output &&\n> +\tgrep \"^To http\" output &&\n> +\tgrep \"^Writing objects\" output\n> +'\n> +\nI understand that test_i18ngrep is not necessary, because pack-objects.c\nis not internationalized. But wouldn't it make sense to use\ntest_i18ngrep in preparation, so that tests don't have to be modified\nlater on?\n\n-\nZbyszek\n"},{"id":"190480","messageId":"7vehr38vyl.fsf@alter.siamese.dyndns.org","threadId":"30378","inReplyTo":"4FA04B4C.4090507@in.waw.pl","subject":"Re: [PATCH 3/3] t5541: test more combinations of --progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T20:53:06Z","receivedAt":"2012-05-01T20:53:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:\n\n>> +test_expect_success 'push --progress shows progress to non-tty' '\n>> +\tcd \"$ROOT_PATH\"/test_repo_clone &&\n>> +\ttest_commit progress &&\n>> +\tgit push --progress 2>&1 | tee output &&\n>> +\tgrep \"^To http\" output &&\n>> +\tgrep \"^Writing objects\" output\n>> +'\n>> +\n> I understand that test_i18ngrep is not necessary, because pack-objects.c\n> is not internationalized. But wouldn't it make sense to use\n> test_i18ngrep in preparation, so that tests don't have to be modified\n> later on?\n\nIn this test, we are not interested in making sure the progress output is\nproperly localized.  I'd rather see it keep using grep and if you really\ncare, run \"git push\" under \"LANG=C LC_ALL=C\" or something reliable instead.\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"190511","messageId":"alpine.DEB.1.00.1205020301560.27828@bonsai2","threadId":"30378","inReplyTo":"20120501084048.GA21904@sigill.intra.peff.net","subject":"Re: Re: 1.7.10 doesn't show file pushstatus","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-05-02T01:04:20Z","receivedAt":"2012-05-02T01:04:20Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Tue, 1 May 2012, Jeff King wrote:\n\n> On Tue, May 01, 2012 at 03:33:26AM -0400, Jeff King wrote:\n> \n> > In this case, send pack gets its arguments from the command-line, not\n> > from the options set at the transport layer. Remote-curl will pass along\n> > \"--quiet\" if we get that from the transport layer, but it does not\n> > otherwise pass along the \"progress\" flag. So there are two problems:\n> > \n> >   1. send-pack defaults its progress boolean to 0. Before 01fdc21, that\n> >      was OK, because it meant \"don't explicitly ask for progress\". But\n> >      after 01fdc21 that now means \"explicitly ask for no progress\", and\n> >      the direct-transport code paths were updated without updating\n> >      cmd_send_pack.\n> > \n> >   2. There's no way to tell send-pack explicitly \"yes, I would like\n> >      progress, no matter what isatty(2) says\". I doubt anybody cares\n> >      much, but it probably makes sense to handle that for the sake of\n> >      completeness.\n> \n> The following patch series fixes this:\n> \n>   [1/3]: send-pack: show progress when isatty(2)\n>   [2/3]: teach send-pack about --[no-]progress\n>   [3/3]: t5541: test more combinations of --progress\n\nThanks. I applied and pushed it after testing on Linux.\n\nDavid, if you want to relieve me of maintenance burden in the future, you\n\n# apply the patches\n\n# push them to a fork on GitHub\n\n# run the tests\n\n# report back to the mailing list (including the outcome of the test --\n  detailed, if it fails)\n\nCiao,\nJohannes\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"}]}