{"thread":{"id":"25432","subject":"[PATCH] Fix to push --progress. The --progress flag was not being passed into tranport.c from send-pack.h, making the --progress flag unusable","startedAt":"2010-10-12T22:21:20Z","lastAt":"2010-10-22T19:42:52Z","messageCount":41,"participants":["Chase Brammer","Jonathan Nieder","Junio C Hamano","Jeff King","Tay Ray Chuan","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"153352","messageId":"AANLkTin9_kofdy49WF4V_JoovVR+G8DN7vn-cz3p84fz@mail.gmail.com","threadId":"25432","inReplyTo":null,"subject":"[PATCH] Fix to push --progress. The --progress flag was not being passed into tranport.c from send-pack.h, making the --progress flag unusable","fromName":"Chase Brammer","fromEmail":"cbrammer@gmail.com","sentAt":"2010-10-12T22:21:20Z","receivedAt":"2010-10-12T22:21:20Z","isPatch":true,"sender":{"key":"cbrammer@gmail.com","avatar":"https://gravatar.com/avatar/96536ec36bad2565256a6f6aaee25498608816ff1334a5dfefc404cf1cf2459e?d=mp&s=160"},"body":"The result of this is external tools and tools writing standard error\nto a file from bash would not be able to receive progress information\nduring a push. Similar functionality is seen in fetch, which still\nworks.\n\nAn example that previously would output no information for --progress:\ngit push origin master --progress > ~/push_error_output.txt 2>&1\n\nThe above example and others now work with this patch.\n\nHelped-by: Jeff King \nHelped-by: Jonathan Nieder \nSigned-off-by: Chase Brammer \n---\n builtin/send-pack.c | 3 +++\n send-pack.h | 1 +\n transport.c | 1 +\n 3 files changed, 5 insertions(+), 0 deletions(-)\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,\nstruct 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,\nstruct 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\n*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-- \n1.7.3.1\n"},{"id":"153355","messageId":"20101012224456.GC15587@burratino","threadId":"25432","inReplyTo":"AANLkTin9_kofdy49WF4V_JoovVR+G8DN7vn-cz3p84fz@mail.gmail.com","subject":"Re: [PATCH] Fix to push --progress. The --progress flag was not being passed into tranport.c from send-pack.h, making the --progress flag unusable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-12T22:44:57Z","receivedAt":"2010-10-12T22:44:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Chase Brammer wrote:\n\n> The result of this is external tools and tools writing standard error\n> to a file from bash would not be able to receive progress information\n> during a push. Similar functionality is seen in fetch, which still\n> works.\n\nA bit of protocol: since the patch is by Jeff, this should have\n\n\tFrom: Jeff King \n\nat the beginning of the log message. See Documentation/SubmittingPatches\nfor details.\n\n> Helped-by: Jonathan Nieder \n\nI didn't help. :)\n\n> builtin/send-pack.c | 3 +++\n> send-pack.h | 1 +\n> transport.c | 1 +\n> 3 files changed, 5 insertions(+), 0 deletions(-)\n\nIt's not necessary by any means, but it would be nice to add a test\nfor this to t/ so no one breaks the new functionality in the future.\n\nThanks.\n"},{"id":"153419","messageId":"7vr5fum19m.fsf@alter.siamese.dyndns.org","threadId":"25432","inReplyTo":"AANLkTin9_kofdy49WF4V_JoovVR+G8DN7vn-cz3p84fz@mail.gmail.com","subject":"Re: [PATCH] Fix to push --progress. The --progress flag was not being passed into tranport.c from send-pack.h, making the --progress flag unusable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T17:49:09Z","receivedAt":"2010-10-13T17:49:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chase Brammer writes:\n\n> The result of this is external tools and tools writing standard error\n\nThanks for leaving a record in the list archive, but (1) what Jonathan\nsaid, plus (2) Please do something about that overlong Subject: line.\n"},{"id":"153421","messageId":"20101013175508.GA14035@sigill.intra.peff.net","threadId":"25432","inReplyTo":"7vr5fum19m.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix to push --progress. The --progress flag was not being passed into tranport.c from send-pack.h, making the --progress flag unusable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-13T17:55:08Z","receivedAt":"2010-10-13T17:55:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 13, 2010 at 10:49:09AM -0700, Junio C Hamano wrote:\n\n> Chase Brammer writes:\n> \n> > The result of this is external tools and tools writing standard error\n> \n> Thanks for leaving a record in the list archive, but (1) what Jonathan\n> said, plus (2) Please do something about that overlong Subject: line.\n\nActually, I was initially trying to foist testing and patch submission\noff on somebody else because I didn't want to spend more time on it. But\nI ended up doing it anyway, and I think I will have a two-patch series.\nThis one (with tests), and one to make --no-progress. I'll try to work\nit up this afternoon.\n\n-Peff\n"},{"id":"153426","messageId":"AANLkTim-0wzsX1XxbjXEyf0hg+5T5P_yPCoaNQ2ReKV5@mail.gmail.com","threadId":"25432","inReplyTo":"AANLkTin9_kofdy49WF4V_JoovVR+G8DN7vn-cz3p84fz@mail.gmail.com","subject":"Re: [PATCH] Fix to push --progress. The --progress flag was not being passed into tranport.c from send-pack.h, making the --progress flag unusable","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-13T18:40:56Z","receivedAt":"2010-10-13T18:40:56Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Oct 13, 2010 at 6:21 AM, Chase Brammer wrote:\n> The result of this is external tools and tools writing standard error\n> to a file from bash would not be able to receive progress information\n> during a push.  Similar functionality is seen in fetch, which still\n> works.\n>\n> An example that previously would output no information for --progress:\n> git push origin master --progress > ~/push_error_output.txt 2>&1\n>\n> The above example and others now work with this patch.\n>\n> Helped-by: Jeff King \n> Helped-by: Jonathan Nieder \n> Signed-off-by: Chase Brammer \n\nThe long subject line not withstanding, your patch seems corrupt.\n\nGuess I'll have to write it by hand.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"153440","messageId":"20101013193039.GA8941@burratino","threadId":"25432","inReplyTo":"1286998311-5112-2-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 1/3] t5523-push-upstream: add function to ensure fresh upstream repo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-13T19:30:39Z","receivedAt":"2010-10-13T19:30:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tay Ray Chuan wrote:\n\n> --- a/t/t5523-push-upstream.sh\n> +++ b/t/t5523-push-upstream.sh\n> @@ -3,8 +3,14 @@\n> test_description='push with --set-upstream'\n> . ./test-lib.sh\n> \n> +ensure_fresh_upstream() {\n> +\ttest -d parent &&\n> +\trm -rf parent\n> +\tgit init --bare parent\n> +}\n\nWouldn't\n\n\trm -rf parent &&\n\tgit init --bare parent\n\ndo it?\n"},{"id":"153432","messageId":"1286998311-5112-1-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"AANLkTin9_kofdy49WF4V_JoovVR+G8DN7vn-cz3p84fz@mail.gmail.com","subject":"[PATCH 0/3] fix push --progress over file://, git://, etc.","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-13T19:31:48Z","receivedAt":"2010-10-13T19:31:48Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"*** BLURB HERE ***\n\nContents:\n[PATCH 1/3] t5523-push-upstream: add function to ensure fresh upstream repo\n[PATCH 2/3] t5523-push-upstream: test progress messages\n[PATCH 3/3] push: pass --progress down to git-pack-objects\n\n builtin/send-pack.c | 3 +++\n send-pack.h | 1 +\n t/t5523-push-upstream.sh | 24 +++++++++++++++++++++++-\n transport.c | 1 +\n 4 files changed, 28 insertions(+), 1 deletions(-)\n\n--\n1.7.2.2.513.ge1ef3\n"},{"id":"153433","messageId":"1286998311-5112-2-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1286998311-5112-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 1/3] t5523-push-upstream: add function to ensure fresh upstream repo","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-13T19:31:49Z","receivedAt":"2010-10-13T19:31:49Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan \n---\n t/t5523-push-upstream.sh | 8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 00da707..337a69e 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -3,8 +3,14 @@\n test_description='push with --set-upstream'\n . ./test-lib.sh\n \n+ensure_fresh_upstream() {\n+\ttest -d parent &&\n+\trm -rf parent\n+\tgit init --bare parent\n+}\n+\n test_expect_success 'setup bare parent' '\n-\tgit init --bare parent &&\n+\tensure_fresh_upstream &&\n \tgit remote add upstream parent\n '\n \n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153436","messageId":"1286998311-5112-3-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1286998311-5112-2-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/3] t5523-push-upstream: test progress messages","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-13T19:31:50Z","receivedAt":"2010-10-13T19:31:50Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Reported-by: Chase Brammer \nSigned-off-by: Tay Ray Chuan \n---\n t/t5523-push-upstream.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 337a69e..554f55e 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -72,4 +72,20 @@ test_expect_success 'push -u HEAD' '\n \tcheck_config headbranch upstream refs/heads/headbranch\n '\n \n+test_expect_failure 'progress messages to non-tty' '\n+\tensure_fresh_upstream &&\n+\n+\t# skip progress messages, since stderr is non-tty\n+\tgit push -u upstream master >out 2>err &&\n+\t! grep \"Writing objects\" err\n+'\n+\n+test_expect_failure 'progress messages to non-tty (forced)' '\n+\tensure_fresh_upstream &&\n+\n+\t# force progress messages to stderr, even though it is non-tty\n+\tgit push -u --progress upstream master >out 2>err &&\n+\tgrep \"Writing objects\" err\n+'\n+\n test_done\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153434","messageId":"1286998311-5112-4-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1286998311-5112-3-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/3] push: pass --progress down to git-pack-objects","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-13T19:31:51Z","receivedAt":"2010-10-13T19:31:51Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jeff King \n\nWhen pushing via builtin transports (like file://, git://), the\nunderlying transport helper (in this case, git-pack-objects) did not get\nthe --progress option, even if it was passed to git push.\n\nFix this, and update the tests to reflect this.\n\nNote that according to the git-pack-objects documentation, we can safely\napply the usual --progress semantics for the transport commands like\nclone and fetch (and for pushing over other smart transports).\n\nReported-by: Chase Brammer \nHelped-by: Jonathan Nieder \nSigned-off-by: Tay Ray Chuan \n---\n builtin/send-pack.c | 3 +++\n send-pack.h | 1 +\n t/t5523-push-upstream.sh | 4 ++--\n transport.c | 1 +\n 4 files changed, 7 insertions(+), 2 deletions(-)\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..05d7ab1 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -5,6 +5,7 @@ struct send_pack_args {\n \tunsigned verbose:1,\n \t\tquiet:1,\n \t\tporcelain:1,\n+\t\tprogress:1,\n \t\tsend_mirror:1,\n \t\tforce_update:1,\n \t\tuse_thin_pack:1,\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 554f55e..113626b 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -72,7 +72,7 @@ test_expect_success 'push -u HEAD' '\n \tcheck_config headbranch upstream refs/heads/headbranch\n '\n \n-test_expect_failure 'progress messages to non-tty' '\n+test_expect_success 'progress messages to non-tty' '\n \tensure_fresh_upstream &&\n \n \t# skip progress messages, since stderr is non-tty\n@@ -80,7 +80,7 @@ test_expect_failure 'progress messages to non-tty' '\n \t! grep \"Writing objects\" err\n '\n \n-test_expect_failure 'progress messages to non-tty (forced)' '\n+test_expect_success 'progress messages to non-tty (forced)' '\n \tensure_fresh_upstream &&\n \n \t# force progress messages to stderr, even though it is non-tty\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-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153438","messageId":"AANLkTimo=Bd_XGvX=TPzVsds3xQGy9126+7Qg+zvk=d2@mail.gmail.com","threadId":"25432","inReplyTo":"1286998311-5112-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 0/3] fix push --progress over file://, git://, etc.","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-13T19:35:40Z","receivedAt":"2010-10-13T19:35:40Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Thu, Oct 14, 2010 at 3:31 AM, Tay Ray Chuan wrote:\n> *** BLURB HERE ***\n\nWhoops. Let me try again:\n\nThis patch series addresses the issue of git push not displaying\nprogress messages to non-tty stderr, even if --progress is used. As\nsuggested by the subject, this issue afflicts the \"builtin smart\ntransports\" - file://, git://, ssh://. (All of them use\ngit_transport_push() and thus git-pack-objects.)\n\n-- \nCheers,\nRay Chuan\n"},{"id":"153477","messageId":"AANLkTikUne3NmDMEsq07oN+4pr9APQJFKhF-FKLmSUbh@mail.gmail.com","threadId":"25432","inReplyTo":"1286998311-5112-4-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 3/3] push: pass --progress down to git-pack-objects","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-14T00:59:41Z","receivedAt":"2010-10-14T00:59:41Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Thu, Oct 14, 2010 at 3:31 AM, Tay Ray Chuan wrote:\n> From: Jeff King \n>\n> [snip]\n>\n> Reported-by: Chase Brammer \n> Helped-by: Jonathan Nieder \n> Signed-off-by: Tay Ray Chuan \n\nChase, I switched the authorship to Jeff - after all, he was the one\nwho wrote the patch. I hope you're fine with that.\n\nJeff, if this patch is ok, since you're the author, perhaps you might\nwant to add your SOB?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"153479","messageId":"20101014012415.GA20685@sigill.intra.peff.net","threadId":"25432","inReplyTo":"AANLkTikUne3NmDMEsq07oN+4pr9APQJFKhF-FKLmSUbh@mail.gmail.com","subject":"Re: [PATCH 3/3] push: pass --progress down to git-pack-objects","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-14T01:24:15Z","receivedAt":"2010-10-14T01:24:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 14, 2010 at 08:59:41AM +0800, Tay Ray Chuan wrote:\n\n> On Thu, Oct 14, 2010 at 3:31 AM, Tay Ray Chuan wrote:\n> > From: Jeff King \n> >\n> > [snip]\n> >\n> > Reported-by: Chase Brammer \n> > Helped-by: Jonathan Nieder \n> > Signed-off-by: Tay Ray Chuan \n> \n> Chase, I switched the authorship to Jeff - after all, he was the one\n> who wrote the patch. I hope you're fine with that.\n> \n> Jeff, if this patch is ok, since you're the author, perhaps you might\n> want to add your SOB?\n\nYeah, definitely:\n\nSigned-off-by: Jeff King \n\n-Peff\n"},{"id":"153482","messageId":"20101014030220.GB20685@sigill.intra.peff.net","threadId":"25432","inReplyTo":"1286998311-5112-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 0/3] more push progress tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-14T03:02:20Z","receivedAt":"2010-10-14T03:02:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 14, 2010 at 03:31:48AM +0800, Tay Ray Chuan wrote:\n\n> [PATCH 1/3] t5523-push-upstream: add function to ensure fresh upstream repo\n> [PATCH 2/3] t5523-push-upstream: test progress messages\n> [PATCH 3/3] push: pass --progress down to git-pack-objects\n\nI had hoped to have a fix for --no-progress, but munging the tests took\nso long that now I am sleepy. :) So here are some extra tests on top of\nyour series. The first two are refactoring, and the third has the new\ntests. It checks regular stderr-is-tty progress and that \"push -q\"\nsuppresses progress, as Junio asked elsewhere. And it reveals the bug in\n--no-progress.\n\nIt might make more sense to actually re-roll your series with the\nrefactoring at the front, and my 3/3 squashed into your 2/3.\n\nAlso, these tests feel a bit out of place in t5523, but I don't see a\nbetter place for them to go. Perhaps they should go in their own test\nscript. I don't feel strongly, though.\n\n [1/3]: tests: factor out terminal handling from t7006\n [2/3]: tests: test terminal output to both stdout and stderr\n [3/3]: t5523: test push progress output to tty\n\n-Peff\n"},{"id":"153483","messageId":"20101014030403.GA5626@sigill.intra.peff.net","threadId":"25432","inReplyTo":"20101014030220.GB20685@sigill.intra.peff.net","subject":"[PATCH 1/3] tests: factor out terminal handling from t7006","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-14T03:04:04Z","receivedAt":"2010-10-14T03:04:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Other tests besides the pager ones may want to check how we handle\noutput to a terminal. This patch makes the code reusable.\n\nSigned-off-by: Jeff King \n---\n t/lib-terminal.sh | 28 ++++++++++++++++++++++++++++\n t/t7006-pager.sh | 31 +------------------------------\n t/{t7006 => }/test-terminal.perl | 0\n 3 files changed, 29 insertions(+), 30 deletions(-)\n create mode 100644 t/lib-terminal.sh\n rename t/{t7006 => }/test-terminal.perl (100%)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nnew file mode 100644\nindex 0000000..6fc33db\n--- /dev/null\n+++ b/t/lib-terminal.sh\n@@ -0,0 +1,28 @@\n+#!/bin/sh\n+\n+test_expect_success 'set up terminal for tests' '\n+\tif test -t 1\n+\tthen\n+\t\t>stdout_is_tty\n+\telif\n+\t\ttest_have_prereq PERL &&\n+\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \\\n+\t\t\tsh -c \"test -t 1\"\n+\tthen\n+\t\t>test_terminal_works\n+\tfi\n+'\n+\n+if test -e stdout_is_tty\n+then\n+\ttest_terminal() { \"$@\"; }\n+\ttest_set_prereq TTY\n+elif test -e test_terminal_works\n+then\n+\ttest_terminal() {\n+\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n+\t}\n+\ttest_set_prereq TTY\n+else\n+\tsay \"# no usable terminal, so skipping some tests\"\n+fi\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex fb744e3..17e54d3 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -4,42 +4,13 @@ test_description='Test automatic use of a pager.'\n \n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-pager.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n cleanup_fail() {\n \techo >&2 cleanup failed\n \t(exit 1)\n }\n \n-test_expect_success 'set up terminal for tests' '\n-\trm -f stdout_is_tty ||\n-\tcleanup_fail &&\n-\n-\tif test -t 1\n-\tthen\n-\t\t>stdout_is_tty\n-\telif\n-\t\ttest_have_prereq PERL &&\n-\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/t7006/test-terminal.perl \\\n-\t\t\tsh -c \"test -t 1\"\n-\tthen\n-\t\t>test_terminal_works\n-\tfi\n-'\n-\n-if test -e stdout_is_tty\n-then\n-\ttest_terminal() { \"$@\"; }\n-\ttest_set_prereq TTY\n-elif test -e test_terminal_works\n-then\n-\ttest_terminal() {\n-\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/t7006/test-terminal.perl \"$@\"\n-\t}\n-\ttest_set_prereq TTY\n-else\n-\tsay \"# no usable terminal, so skipping some tests\"\n-fi\n-\n test_expect_success 'setup' '\n \tunset GIT_PAGER GIT_PAGER_IN_USE;\n \ttest_might_fail git config --unset core.pager &&\ndiff --git a/t/t7006/test-terminal.perl b/t/test-terminal.perl\nsimilarity index 100%\nrename from t/t7006/test-terminal.perl\nrename to t/test-terminal.perl\n-- \n1.7.3.1.204.g337d6.dirty\n"},{"id":"153484","messageId":"20101014030443.GB5626@sigill.intra.peff.net","threadId":"25432","inReplyTo":"20101014030220.GB20685@sigill.intra.peff.net","subject":"[PATCH 2/3] tests: test terminal output to both stdout and stderr","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-14T03:04:44Z","receivedAt":"2010-10-14T03:04:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some outputs (like the pager) care whether stdout is a\nterminal. Others (like progress meters) care about stderr.\n\nThis patch sets up both. Technically speaking, we could go\nfurther and set up just one (because either the other goes\nto a terminal, or because our tests are only interested in\none). This patch does both to keep the interface to\nlib-terminal simple.\n\nSigned-off-by: Jeff King \n---\n t/lib-terminal.sh | 8 ++++----\n t/test-terminal.perl | 31 ++++++++++++++++++++++++-------\n 2 files changed, 28 insertions(+), 11 deletions(-)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 6fc33db..3258b8f 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -1,19 +1,19 @@\n #!/bin/sh\n \n test_expect_success 'set up terminal for tests' '\n-\tif test -t 1\n+\tif test -t 1 && test -t 2\n \tthen\n-\t\t>stdout_is_tty\n+\t\t>have_tty\n \telif\n \t\ttest_have_prereq PERL &&\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \\\n-\t\t\tsh -c \"test -t 1\"\n+\t\t\tsh -c \"test -t 1 && test -t 2\"\n \tthen\n \t\t>test_terminal_works\n \tfi\n '\n \n-if test -e stdout_is_tty\n+if test -e have_tty\n then\n \ttest_terminal() { \"$@\"; }\n \ttest_set_prereq TTY\ndiff --git a/t/test-terminal.perl b/t/test-terminal.perl\nindex 73ff809..c2e9dac 100755\n--- a/t/test-terminal.perl\n+++ b/t/test-terminal.perl\n@@ -4,14 +4,15 @@ use warnings;\n use IO::Pty;\n use File::Copy;\n \n-# Run @$argv in the background with stdout redirected to $out.\n+# Run @$argv in the background with stdio redirected to $out and $err.\n sub start_child {\n-\tmy ($argv, $out) = @_;\n+\tmy ($argv, $out, $err) = @_;\n \tmy $pid = fork;\n \tif (not defined $pid) {\n \t\tdie \"fork failed: $!\"\n \t} elsif ($pid == 0) {\n \t\topen STDOUT, \">&\", $out;\n+\t\topen STDERR, \">&\", $err;\n \t\tclose $out;\n \t\texec(@$argv) or die \"cannot exec '$argv->[0]': $!\"\n \t}\n@@ -47,12 +48,28 @@ sub xsendfile {\n \tcopy($in, $out, 4096) or $!{EIO} or die \"cannot copy from child: $!\";\n }\n \n+sub copy_stdio {\n+\tmy ($out, $err) = @_;\n+\tmy $pid = fork;\n+\tdefined $pid or die \"fork failed: $!\";\n+\tif (!$pid) {\n+\t\tclose($out);\n+\t\txsendfile(\\*STDERR, $err);\n+\t\texit 0;\n+\t}\n+\tclose($err);\n+\txsendfile(\\*STDOUT, $out);\n+\tfinish_child($pid) == 0\n+\t\tor exit 1;\n+}\n+\n if ($#ARGV < 1) {\n \tdie \"usage: test-terminal program args\";\n }\n-my $master = new IO::Pty;\n-my $slave = $master->slave;\n-my $pid = start_child(\\@ARGV, $slave);\n-close $slave;\n-xsendfile(\\*STDOUT, $master);\n+my $master_out = new IO::Pty;\n+my $master_err = new IO::Pty;\n+my $pid = start_child(\\@ARGV, $master_out->slave, $master_err->slave);\n+close $master_out->slave;\n+close $master_err->slave;\n+copy_stdio($master_out, $master_err);\n exit(finish_child($pid));\n-- \n1.7.3.1.204.g337d6.dirty\n"},{"id":"153485","messageId":"20101014030505.GC5626@sigill.intra.peff.net","threadId":"25432","inReplyTo":"20101014030220.GB20685@sigill.intra.peff.net","subject":"[PATCH 3/3] t5523: test push progress output to tty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-14T03:05:05Z","receivedAt":"2010-10-14T03:05:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already test the non-tty cases, but until recent changes\nmade lib-terminal.sh available, we couldn't test the case\nwith a tty. These tests reveal a bug: --no-progress is\nsilently ignored.\n\nSigned-off-by: Jeff King \n---\n t/t5523-push-upstream.sh | 26 ++++++++++++++++++++++++--\n 1 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 113626b..78c5978 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -2,6 +2,7 @@\n \n test_description='push with --set-upstream'\n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n ensure_fresh_upstream() {\n \ttest -d parent &&\n@@ -72,7 +73,14 @@ test_expect_success 'push -u HEAD' '\n \tcheck_config headbranch upstream refs/heads/headbranch\n '\n \n-test_expect_success 'progress messages to non-tty' '\n+test_expect_success 'progress messages go to tty' '\n+\tensure_fresh_upstream &&\n+\n+\ttest_terminal git push -u upstream master >out 2>err &&\n+\tgrep \"Writing objects\" err\n+'\n+\n+test_expect_success 'progress messages do not go to non-tty' '\n \tensure_fresh_upstream &&\n \n \t# skip progress messages, since stderr is non-tty\n@@ -80,7 +88,7 @@ test_expect_success 'progress messages to non-tty' '\n \t! grep \"Writing objects\" err\n '\n \n-test_expect_success 'progress messages to non-tty (forced)' '\n+test_expect_success 'progress messages go to non-tty (forced)' '\n \tensure_fresh_upstream &&\n \n \t# force progress messages to stderr, even though it is non-tty\n@@ -88,4 +96,18 @@ test_expect_success 'progress messages to non-tty (forced)' '\n \tgrep \"Writing objects\" err\n '\n \n+test_expect_success 'push -q suppresses progress' '\n+\tensure_fresh_upstream &&\n+\n+\ttest_terminal git push -u -q upstream master >out 2>err &&\n+\t! grep \"Writing objects\" err\n+'\n+\n+test_expect_failure 'push --no-progress suppresses progress' '\n+\tensure_fresh_upstream &&\n+\n+\ttest_terminal git push -u --no-progress upstream master >out 2>err &&\n+\t! grep \"Writing objects\" err\n+'\n+\n test_done\n-- \n1.7.3.1.204.g337d6.dirty\n"},{"id":"153486","messageId":"20101014031014.GA14664@burratino","threadId":"25432","inReplyTo":"20101014030403.GA5626@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] tests: factor out terminal handling from t7006","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-14T03:10:14Z","receivedAt":"2010-10-14T03:10:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Other tests besides the pager ones may want to check how we handle\n> output to a terminal. This patch makes the code reusable.\n> \n> Signed-off-by: Jeff King \n\nThanks! Looks good to me.\n"},{"id":"153487","messageId":"20101014031642.GB14664@burratino","threadId":"25432","inReplyTo":"20101014030505.GC5626@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] t5523: test push progress output to tty","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-14T03:16:42Z","receivedAt":"2010-10-14T03:16:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> --- a/t/t5523-push-upstream.sh\n> +++ b/t/t5523-push-upstream.sh\n[...]\n> @@ -72,7 +73,14 @@ test_expect_success 'push -u HEAD' '\n> \tcheck_config headbranch upstream refs/heads/headbranch\n> '\n> \n> -test_expect_success 'progress messages to non-tty' '\n> +test_expect_success 'progress messages go to tty' '\n> +\tensure_fresh_upstream &&\n> +\n> +\ttest_terminal git push -u upstream master >out 2>err &&\n> +\tgrep \"Writing objects\" err\n> +'\n\nMissing TTY prerequisite. (Do you think test_terminal should check\n$prereq to prevent this?)\n\n> @@ -88,4 +96,18 @@ test_expect_success 'progress messages to non-tty (forced)' '\n> \tgrep \"Writing objects\" err\n> '\n> \n> +test_expect_success 'push -q suppresses progress' '\n> +\tensure_fresh_upstream &&\n> +\n> +\ttest_terminal git push -u -q upstream master >out 2>err &&\n> +\t! grep \"Writing objects\" err\n> +'\n\nLikewise.\n\n> +\n> +test_expect_failure 'push --no-progress suppresses progress' '\n> +\tensure_fresh_upstream &&\n> +\n> +\ttest_terminal git push -u --no-progress upstream master >out 2>err &&\n> +\t! grep \"Writing objects\" err\n> +'\n\nLikewise.\n\nRegards,\nJonathan\n"},{"id":"153488","messageId":"20101014032734.GC14664@burratino","threadId":"25432","inReplyTo":"20101014030443.GB5626@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] tests: test terminal output to both stdout and stderr","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-14T03:27:34Z","receivedAt":"2010-10-14T03:27:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Some outputs (like the pager) care whether stdout is a\n> terminal. Others (like progress meters) care about stderr.\n> \n> This patch sets up both. Technically speaking, we could go\n> further and set up just one (because either the other goes\n> to a terminal, or because our tests are only interested in\n> one).\n\nThis makes test_terminal more realistic, too: the usual case is for\nstdout and stderr to go to a terminal (unless explicitly captured or\nredirected).\n\nTests can use 'test_terminal sh -c \"foo >/dev/null\"' to test that a\ncommand correctly handles being run with stderr a terminal and\nstdout not.\n\nAnd I doubt this would make test_terminal much slower.\n\nSo for what it's worth:\nAcked-by: Jonathan Nieder \n\nThanks.\n"},{"id":"153489","messageId":"20101014033448.GB28197@sigill.intra.peff.net","threadId":"25432","inReplyTo":"20101014031642.GB14664@burratino","subject":"Re: [PATCH 3/3] t5523: test push progress output to tty","fromName":"Jeff King","fromEmail":"jrk@wrek.org","sentAt":"2010-10-14T03:34:48Z","receivedAt":"2010-10-14T03:34:48Z","isPatch":true,"sender":{"key":"jrk@wrek.org","avatar":null},"body":"On Wed, Oct 13, 2010 at 10:16:42PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > --- a/t/t5523-push-upstream.sh\n> > +++ b/t/t5523-push-upstream.sh\n> [...]\n> > @@ -72,7 +73,14 @@ test_expect_success 'push -u HEAD' '\n> > \tcheck_config headbranch upstream refs/heads/headbranch\n> > '\n> > \n> > -test_expect_success 'progress messages to non-tty' '\n> > +test_expect_success 'progress messages go to tty' '\n> > +\tensure_fresh_upstream &&\n> > +\n> > +\ttest_terminal git push -u upstream master >out 2>err &&\n> > +\tgrep \"Writing objects\" err\n> > +'\n> \n> Missing TTY prerequisite. (Do you think test_terminal should check\n> $prereq to prevent this?)\n\nOops, good catch. I think we should already catch it, as test_terminal\nwill not be defined at all in the no-tty case. We could print a nicer\nmessage, but it is not likely to be seen by the user. If they are\nusing \"-v\", then stderr probably _is_ a tty. And if not, they will not\nsee the message. There are ways around it, but they are not likely to be\nseen unless the user is really trying (e.g., \"./t5523-* -v >not_a_tty\").\n\n-Peff\n"},{"id":"153536","messageId":"20101014203721.GA28958@burratino","threadId":"25432","inReplyTo":"20101014033448.GB28197@sigill.intra.peff.net","subject":"[PATCH/RFC 0/2] test_terminal: check that TTY prerequisite is declared","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-14T20:37:21Z","receivedAt":"2010-10-14T20:37:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Wed, Oct 13, 2010 at 10:16:42PM -0500, Jonathan Nieder wrote:\n\n>> Missing TTY prerequisite. (Do you think test_terminal should check\n>> $prereq to prevent this?)\n>\n> Oops, good catch. I think we should already catch it, as test_terminal\n> will not be defined at all in the no-tty case. We could print a nicer\n> message, but\n\nI rather meant something like this.\n\nPatch 1 exposes the internal $prereq variable from\ntest_expect_(success|failure). Maybe it should be called\nGIT_TEST_something to avoid trampling other programs' namespaces? Not\nsure.\n\nPatch 2 introduces some magic autodetection so people that never run\ntests without -v can still notice the missing TTY prereqs.\n\nJonathan Nieder (2):\n test-lib: allow test code to check the list of declared prerequisites\n test_terminal: catch use without TTY prerequisite\n\n t/t7006-pager.sh | 20 ++++++++++++--------\n t/test-lib.sh | 26 +++++++++++++++++++-------\n 2 files changed, 31 insertions(+), 15 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"153537","messageId":"20101014204001.GB28958@burratino","threadId":"25432","inReplyTo":"20101014203721.GA28958@burratino","subject":"[PATCH 1/2] test-lib: allow test code to check the list of declared prerequisites","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-14T20:40:01Z","receivedAt":"2010-10-14T20:40:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This is plumbing to prepare helpers like test_terminal to notice buggy\ntest scripts that do not declare all of the necessary prerequisites.\n\nSigned-off-by: Jonathan Nieder \n---\nApplies on top of 892e6f7 (test-lib: make test_expect_code a test\ncommand) from the en/and-cascade-tests branch. On master,\ntext_expect_code would have to be adjusted, too.\n\n t/test-lib.sh | 26 +++++++++++++++++++-------\n 1 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d86edcd..cd69ffa 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -362,6 +362,15 @@ test_have_prereq () {\n \ttest $total_prereq = $ok_prereq\n }\n \n+test_declared_prereq () {\n+\tcase \",$test_prereq,\" in\n+\t*,$1,*)\n+\t\treturn 0\n+\t\t;;\n+\tesac\n+\treturn 1\n+}\n+\n # You are not expected to call test_ok_ and test_failure_ directly, use\n # the text_expect_* functions instead.\n \n@@ -414,17 +423,17 @@ test_skip () {\n \t\t\tbreak\n \t\tesac\n \tdone\n-\tif test -z \"$to_skip\" && test -n \"$prereq\" &&\n-\t ! test_have_prereq \"$prereq\"\n+\tif test -z \"$to_skip\" && test -n \"$test_prereq\" &&\n+\t ! test_have_prereq \"$test_prereq\"\n \tthen\n \t\tto_skip=t\n \tfi\n \tcase \"$to_skip\" in\n \tt)\n \t\tof_prereq=\n-\t\tif test \"$missing_prereq\" != \"$prereq\"\n+\t\tif test \"$missing_prereq\" != \"$test_prereq\"\n \t\tthen\n-\t\t\tof_prereq=\" of $prereq\"\n+\t\t\tof_prereq=\" of $test_prereq\"\n \t\tfi\n \n \t\tsay_color skip >&3 \"skipping test: $@\"\n@@ -438,9 +447,10 @@ test_skip () {\n }\n \n test_expect_failure () {\n-\ttest \"$#\" = 3 && { prereq=$1; shift; } || prereq=\n+\ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-failure\"\n+\texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n \t\tsay >&3 \"checking known breakage: $2\"\n@@ -456,9 +466,10 @@ test_expect_failure () {\n }\n \n test_expect_success () {\n-\ttest \"$#\" = 3 && { prereq=$1; shift; } || prereq=\n+\ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-success\"\n+\texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n \t\tsay >&3 \"expecting success: $2\"\n@@ -482,11 +493,12 @@ test_expect_success () {\n # Usage: test_external description command arguments...\n # Example: test_external 'Perl API' perl ../path/to/test.pl\n test_external () {\n-\ttest \"$#\" = 4 && { prereq=$1; shift; } || prereq=\n+\ttest \"$#\" = 4 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 3 ||\n \terror >&5 \"bug in the test script: not 3 or 4 parameters to test_external\"\n \tdescr=\"$1\"\n \tshift\n+\texport test_prereq\n \tif ! test_skip \"$descr\" \"$@\"\n \tthen\n \t\t# Announce the script to reduce confusion about the\n-- \n1.7.2.3\n"},{"id":"153539","messageId":"20101014204141.GC28958@burratino","threadId":"25432","inReplyTo":"20101014203721.GA28958@burratino","subject":"[PATCH 2/2] test_terminal: catch use without TTY prerequisite","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-14T20:41:41Z","receivedAt":"2010-10-14T20:41:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It is easy to forget to declare the TTY prerequisite when\nwriting tests on a system where it would always be satisfied\n(because IO::Pty is installed; see v1.7.3-rc0~33^2, 2010-08-16\nfor example). Automatically detect this problem so there is\nno need to remember.\n\n\ttest_terminal: need to declare TTY prerequisite\n\ttest_must_fail: command not found: test_terminal echo hi\n\ntest_terminal returns status 127 in this case to simulate\nnot being available.\n\nAlso replace the SIMPLEPAGERTTY prerequisite on one test with\n\"SIMPLEPAGER,TTY\", since (1) the latter is supported now and\n(2) the prerequisite detection relies on the TTY prereq being\nexplicitly declared.\n\nSigned-off-by: Jonathan Nieder \n---\nPenance for introducing that bug a few times.\n\n t/t7006-pager.sh | 20 ++++++++++++--------\n 1 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex fb744e3..53868f6 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -28,11 +28,11 @@ test_expect_success 'set up terminal for tests' '\n \n if test -e stdout_is_tty\n then\n-\ttest_terminal() { \"$@\"; }\n+\ttest_terminal_() { \"$@\"; }\n \ttest_set_prereq TTY\n elif test -e test_terminal_works\n then\n-\ttest_terminal() {\n+\ttest_terminal_() {\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/t7006/test-terminal.perl \"$@\"\n \t}\n \ttest_set_prereq TTY\n@@ -40,6 +40,15 @@ else\n \tsay \"# no usable terminal, so skipping some tests\"\n fi\n \n+test_terminal () {\n+\tif ! test_declared_prereq TTY\n+\tthen\n+\t\techo >&2 'test_terminal: need to declare TTY prerequisite'\n+\t\treturn 127\n+\tfi\n+\ttest_terminal_ \"$@\"\n+}\n+\n test_expect_success 'setup' '\n \tunset GIT_PAGER GIT_PAGER_IN_USE;\n \ttest_might_fail git config --unset core.pager &&\n@@ -213,11 +222,6 @@ test_expect_success 'color when writing to a file intended for a pager' '\n \tcolorful colorful.log\n '\n \n-if test_have_prereq SIMPLEPAGER && test_have_prereq TTY\n-then\n-\ttest_set_prereq SIMPLEPAGERTTY\n-fi\n-\n # Use this helper to make it easy for the caller of your\n # terminal-using function to specify whether it should fail.\n # If you write\n@@ -253,7 +257,7 @@ parse_args() {\n test_default_pager() {\n \tparse_args \"$@\"\n \n-\t$test_expectation SIMPLEPAGERTTY \"$cmd - default pager is used by default\" \"\n+\t$test_expectation SIMPLEPAGER,TTY \"$cmd - default pager is used by default\" \"\n \t\tunset PAGER GIT_PAGER;\n \t\ttest_might_fail git config --unset core.pager &&\n \t\trm -f default_pager_used ||\n-- \n1.7.2.3\n"},{"id":"153554","messageId":"20101015044252.GA22438@sigill.intra.peff.net","threadId":"25432","inReplyTo":"20101014203721.GA28958@burratino","subject":"Re: [PATCH/RFC 0/2] test_terminal: check that TTY prerequisite is declared","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-15T04:42:53Z","receivedAt":"2010-10-15T04:42:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 14, 2010 at 03:37:21PM -0500, Jonathan Nieder wrote:\n\n> > Oops, good catch. I think we should already catch it, as test_terminal\n> > will not be defined at all in the no-tty case. We could print a nicer\n> > message, but\n> \n> I rather meant something like this.\n> \n> Patch 1 exposes the internal $prereq variable from\n> test_expect_(success|failure). Maybe it should be called\n> GIT_TEST_something to avoid trampling other programs' namespaces? Not\n> sure.\n> \n> Patch 2 introduces some magic autodetection so people that never run\n> tests without -v can still notice the missing TTY prereqs.\n\nYeah, that is better, as it will catch the lack of prerequisite even on\nsystems where the prerequisite is met.\n\nIt seems like a lot of code to catch something small, but on the other\nhand, it does seem to be a repeated mistake.\n\n-Peff\n"},{"id":"153557","messageId":"AANLkTikkWw4Ju4jJFtvKX+s2LMkveQX-uBQyS41A=Vh2@mail.gmail.com","threadId":"25432","inReplyTo":"20101014204001.GB28958@burratino","subject":"Re: [PATCH 1/2] test-lib: allow test code to check the list of declared prerequisites","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-10-15T05:18:50Z","receivedAt":"2010-10-15T05:18:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Oct 14, 2010 at 20:40, Jonathan Nieder wrote:\n\n> +       case \",$test_prereq,\" in\n> +       *,$1,*)\n\nWon't this only work with:\n\n test_expect_success FOO,THINGYOUWANT,BAR '...'\n\nAnd not:\n\n test_expect_success THINGYOUWANT,FOO,BAR '...'\n\n?\n"},{"id":"153563","messageId":"20101015053414.GC21830@burratino","threadId":"25432","inReplyTo":"AANLkTikkWw4Ju4jJFtvKX+s2LMkveQX-uBQyS41A=Vh2@mail.gmail.com","subject":"Re: [PATCH 1/2] test-lib: allow test code to check the list of declared prerequisites","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-15T05:34:14Z","receivedAt":"2010-10-15T05:34:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> On Thu, Oct 14, 2010 at 20:40, Jonathan Nieder wrote:\n\n>> +       case \",$test_prereq,\" in\n>> +       *,$1,*)\n>\n> Won't this only work with:\n> \n> test_expect_success FOO,THINGYOUWANT,BAR '...'\n> \n> And not:\n> \n> test_expect_success THINGYOUWANT,FOO,BAR '...'\n> \n> ?\n\n\t$ case ,X,FOO,BAR, in\n\t *,X,*)\n\t\techo ok\n\t\t;;\n\t *)\n\t\techo not ok\n\t\t;;\n\t esac\n\tok\n\t$\n\nLooks safe to me. A * can match any string, including the empty string[1].\n\n[1] http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_13_02\n"},{"id":"153577","messageId":"AANLkTi=Cvgu-761fQdhwbHhHJ6AgWYUSAbsOt=Cstji-@mail.gmail.com","threadId":"25432","inReplyTo":"20101015044252.GA22438@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 0/2] test_terminal: check that TTY prerequisite is declared","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-15T11:27:34Z","receivedAt":"2010-10-15T11:27:34Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Fri, Oct 15, 2010 at 12:42 PM, Jeff King wrote:\n> On Thu, Oct 14, 2010 at 03:37:21PM -0500, Jonathan Nieder wrote:\n>\n>> > Oops, good catch. I think we should already catch it, as test_terminal\n>> > will not be defined at all in the no-tty case. We could print a nicer\n>> > message, but\n>>\n>> I rather meant something like this.\n>>\n>> Patch 1 exposes the internal $prereq variable from\n>> test_expect_(success|failure).  Maybe it should be called\n>> GIT_TEST_something to avoid trampling other programs' namespaces?  Not\n>> sure.\n>>\n>> Patch 2 introduces some magic autodetection so people that never run\n>> tests without -v can still notice the missing TTY prereqs.\n>\n> Yeah, that is better, as it will catch the lack of prerequisite even on\n> systems where the prerequisite is met.\n>\n> It seems like a lot of code to catch something small, but on the other\n> hand, it does seem to be a repeated mistake.\n\nI'll probably be re-rolling the push --progress fix series with this and Jeff's.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"153632","messageId":"1287254223-4496-1-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"AANLkTimo=Bd_XGvX=TPzVsds3xQGy9126+7Qg+zvk=d2@mail.gmail.com","subject":"[PATCH v2 0/8] fix push --progress over file://, git://, etc.","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:36:55Z","receivedAt":"2010-10-16T18:36:55Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This patch series addresses the issue of git push not displaying\nprogress messages to non-tty stderr, even if --progress is used. As\nsuggested by the subject, this issue afflicts the \"builtin smart\ntransports\" - file://, git://, ssh://. (All of them use\ngit_transport_push() and thus git-pack-objects.)\n\nThe last patch contains the actual fix; most of the other patches\nimprove tests that depend on a tty.\n\nContents:\n[PATCH v2 0/8] fix push --progress over file://, git://, etc.\n[PATCH v2 1/8] tests: factor out terminal handling from t7006\n[PATCH v2 2/8] tests: test terminal output to both stdout and stderr\n[PATCH v2 3/8] test-lib: allow test code to check the list of declared prerequisites\n[PATCH v2 4/8] test_terminal: catch use without TTY prerequisite\n[PATCH v2 5/8] test_terminal: give priority to test-terminal.perl usage\n[PATCH v2 6/8] t5523-push-upstream: add function to ensure fresh upstream repo\n[PATCH v2 7/8] t5523-push-upstream: test progress messages\n[PATCH v2 8/8] push: pass --progress down to git-pack-objects\n\nJeff King (3):\n tests: factor out terminal handling from t7006\n tests: test terminal output to both stdout and stderr\n push: pass --progress down to git-pack-objects\n\nJonathan Nieder (3):\n test-lib: allow test code to check the list of declared prerequisites\n test_terminal: catch use without TTY prerequisite\n test_terminal: give priority to test-terminal.perl usage\n\nTay Ray Chuan (2):\n t5523-push-upstream: add function to ensure fresh upstream repo\n t5523-push-upstream: test progress messages\n\n builtin/send-pack.c | 3 ++\n send-pack.h | 1 +\n t/lib-terminal.sh | 39 +++++++++++++++++++++++\n t/t5523-push-upstream.sh | 44 +++++++++++++++++++++++++-\n t/t7006-pager.sh | 38 +---------------------\n t/t7006/test-terminal.perl | 58 ----------------------------------\n t/test-lib.sh | 26 +++++++++++----\n t/test-terminal.perl | 75 ++++++++++++++++++++++++++++++++++++++++++++\n transport.c | 1 +\n 9 files changed, 183 insertions(+), 102 deletions(-)\n create mode 100644 t/lib-terminal.sh\n delete mode 100755 t/t7006/test-terminal.perl\n create mode 100755 t/test-terminal.perl\n\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153633","messageId":"1287254223-4496-2-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 1/8] tests: factor out terminal handling from t7006","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:36:56Z","receivedAt":"2010-10-16T18:36:56Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jeff King \n\nOther tests besides the pager ones may want to check how we handle\noutput to a terminal. This patch makes the code reusable.\n\nSigned-off-by: Jeff King \n---\n\n No change.\n\n t/lib-terminal.sh | 28 +++++++++++++++++++++\n t/t7006-pager.sh | 31 +----------------------\n t/t7006/test-terminal.perl | 58 --------------------------------------------\n t/test-terminal.perl | 58 ++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 87 insertions(+), 88 deletions(-)\n create mode 100644 t/lib-terminal.sh\n delete mode 100755 t/t7006/test-terminal.perl\n create mode 100755 t/test-terminal.perl\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nnew file mode 100644\nindex 0000000..6fc33db\n--- /dev/null\n+++ b/t/lib-terminal.sh\n@@ -0,0 +1,28 @@\n+#!/bin/sh\n+\n+test_expect_success 'set up terminal for tests' '\n+\tif test -t 1\n+\tthen\n+\t\t>stdout_is_tty\n+\telif\n+\t\ttest_have_prereq PERL &&\n+\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \\\n+\t\t\tsh -c \"test -t 1\"\n+\tthen\n+\t\t>test_terminal_works\n+\tfi\n+'\n+\n+if test -e stdout_is_tty\n+then\n+\ttest_terminal() { \"$@\"; }\n+\ttest_set_prereq TTY\n+elif test -e test_terminal_works\n+then\n+\ttest_terminal() {\n+\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n+\t}\n+\ttest_set_prereq TTY\n+else\n+\tsay \"# no usable terminal, so skipping some tests\"\n+fi\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex fb744e3..17e54d3 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -4,42 +4,13 @@ test_description='Test automatic use of a pager.'\n \n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-pager.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n cleanup_fail() {\n \techo >&2 cleanup failed\n \t(exit 1)\n }\n \n-test_expect_success 'set up terminal for tests' '\n-\trm -f stdout_is_tty ||\n-\tcleanup_fail &&\n-\n-\tif test -t 1\n-\tthen\n-\t\t>stdout_is_tty\n-\telif\n-\t\ttest_have_prereq PERL &&\n-\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/t7006/test-terminal.perl \\\n-\t\t\tsh -c \"test -t 1\"\n-\tthen\n-\t\t>test_terminal_works\n-\tfi\n-'\n-\n-if test -e stdout_is_tty\n-then\n-\ttest_terminal() { \"$@\"; }\n-\ttest_set_prereq TTY\n-elif test -e test_terminal_works\n-then\n-\ttest_terminal() {\n-\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/t7006/test-terminal.perl \"$@\"\n-\t}\n-\ttest_set_prereq TTY\n-else\n-\tsay \"# no usable terminal, so skipping some tests\"\n-fi\n-\n test_expect_success 'setup' '\n \tunset GIT_PAGER GIT_PAGER_IN_USE;\n \ttest_might_fail git config --unset core.pager &&\ndiff --git a/t/t7006/test-terminal.perl b/t/t7006/test-terminal.perl\ndeleted file mode 100755\nindex 73ff809..0000000\n--- a/t/t7006/test-terminal.perl\n+++ /dev/null\n@@ -1,58 +0,0 @@\n-#!/usr/bin/perl\n-use strict;\n-use warnings;\n-use IO::Pty;\n-use File::Copy;\n-\n-# Run @$argv in the background with stdout redirected to $out.\n-sub start_child {\n-\tmy ($argv, $out) = @_;\n-\tmy $pid = fork;\n-\tif (not defined $pid) {\n-\t\tdie \"fork failed: $!\"\n-\t} elsif ($pid == 0) {\n-\t\topen STDOUT, \">&\", $out;\n-\t\tclose $out;\n-\t\texec(@$argv) or die \"cannot exec '$argv->[0]': $!\"\n-\t}\n-\treturn $pid;\n-}\n-\n-# Wait for $pid to finish.\n-sub finish_child {\n-\t# Simplified from wait_or_whine() in run-command.c.\n-\tmy ($pid) = @_;\n-\n-\tmy $waiting = waitpid($pid, 0);\n-\tif ($waiting < 0) {\n-\t\tdie \"waitpid failed: $!\";\n-\t} elsif ($? & 127) {\n-\t\tmy $code = $? & 127;\n-\t\twarn \"died of signal $code\";\n-\t\treturn $code - 128;\n-\t} else {\n-\t\treturn $? >> 8;\n-\t}\n-}\n-\n-sub xsendfile {\n-\tmy ($out, $in) = @_;\n-\n-\t# Note: the real sendfile() cannot read from a terminal.\n-\n-\t# It is unspecified by POSIX whether reads\n-\t# from a disconnected terminal will return\n-\t# EIO (as in AIX 4.x, IRIX, and Linux) or\n-\t# end-of-file. Either is fine.\n-\tcopy($in, $out, 4096) or $!{EIO} or die \"cannot copy from child: $!\";\n-}\n-\n-if ($#ARGV < 1) {\n-\tdie \"usage: test-terminal program args\";\n-}\n-my $master = new IO::Pty;\n-my $slave = $master->slave;\n-my $pid = start_child(\\@ARGV, $slave);\n-close $slave;\n-xsendfile(\\*STDOUT, $master);\n-exit(finish_child($pid));\ndiff --git a/t/test-terminal.perl b/t/test-terminal.perl\nnew file mode 100755\nindex 0000000..73ff809\n--- /dev/null\n+++ b/t/test-terminal.perl\n@@ -0,0 +1,58 @@\n+#!/usr/bin/perl\n+use strict;\n+use warnings;\n+use IO::Pty;\n+use File::Copy;\n+\n+# Run @$argv in the background with stdout redirected to $out.\n+sub start_child {\n+\tmy ($argv, $out) = @_;\n+\tmy $pid = fork;\n+\tif (not defined $pid) {\n+\t\tdie \"fork failed: $!\"\n+\t} elsif ($pid == 0) {\n+\t\topen STDOUT, \">&\", $out;\n+\t\tclose $out;\n+\t\texec(@$argv) or die \"cannot exec '$argv->[0]': $!\"\n+\t}\n+\treturn $pid;\n+}\n+\n+# Wait for $pid to finish.\n+sub finish_child {\n+\t# Simplified from wait_or_whine() in run-command.c.\n+\tmy ($pid) = @_;\n+\n+\tmy $waiting = waitpid($pid, 0);\n+\tif ($waiting < 0) {\n+\t\tdie \"waitpid failed: $!\";\n+\t} elsif ($? & 127) {\n+\t\tmy $code = $? & 127;\n+\t\twarn \"died of signal $code\";\n+\t\treturn $code - 128;\n+\t} else {\n+\t\treturn $? >> 8;\n+\t}\n+}\n+\n+sub xsendfile {\n+\tmy ($out, $in) = @_;\n+\n+\t# Note: the real sendfile() cannot read from a terminal.\n+\n+\t# It is unspecified by POSIX whether reads\n+\t# from a disconnected terminal will return\n+\t# EIO (as in AIX 4.x, IRIX, and Linux) or\n+\t# end-of-file. Either is fine.\n+\tcopy($in, $out, 4096) or $!{EIO} or die \"cannot copy from child: $!\";\n+}\n+\n+if ($#ARGV < 1) {\n+\tdie \"usage: test-terminal program args\";\n+}\n+my $master = new IO::Pty;\n+my $slave = $master->slave;\n+my $pid = start_child(\\@ARGV, $slave);\n+close $slave;\n+xsendfile(\\*STDOUT, $master);\n+exit(finish_child($pid));\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153636","messageId":"1287254223-4496-3-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-2-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 2/8] tests: test terminal output to both stdout and stderr","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:36:57Z","receivedAt":"2010-10-16T18:36:57Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jeff King \n\nSome outputs (like the pager) care whether stdout is a\nterminal. Others (like progress meters) care about stderr.\n\nThis patch sets up both. Technically speaking, we could go\nfurther and set up just one (because either the other goes\nto a terminal, or because our tests are only interested in\none). This patch does both to keep the interface to\nlib-terminal simple.\n\nSigned-off-by: Jeff King \n---\n\n No change.\n\n t/lib-terminal.sh | 8 ++++----\n t/test-terminal.perl | 31 ++++++++++++++++++++++++-------\n 2 files changed, 28 insertions(+), 11 deletions(-)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 6fc33db..3258b8f 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -1,19 +1,19 @@\n #!/bin/sh\n \n test_expect_success 'set up terminal for tests' '\n-\tif test -t 1\n+\tif test -t 1 && test -t 2\n \tthen\n-\t\t>stdout_is_tty\n+\t\t>have_tty\n \telif\n \t\ttest_have_prereq PERL &&\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \\\n-\t\t\tsh -c \"test -t 1\"\n+\t\t\tsh -c \"test -t 1 && test -t 2\"\n \tthen\n \t\t>test_terminal_works\n \tfi\n '\n \n-if test -e stdout_is_tty\n+if test -e have_tty\n then\n \ttest_terminal() { \"$@\"; }\n \ttest_set_prereq TTY\ndiff --git a/t/test-terminal.perl b/t/test-terminal.perl\nindex 73ff809..c2e9dac 100755\n--- a/t/test-terminal.perl\n+++ b/t/test-terminal.perl\n@@ -4,14 +4,15 @@ use warnings;\n use IO::Pty;\n use File::Copy;\n \n-# Run @$argv in the background with stdout redirected to $out.\n+# Run @$argv in the background with stdio redirected to $out and $err.\n sub start_child {\n-\tmy ($argv, $out) = @_;\n+\tmy ($argv, $out, $err) = @_;\n \tmy $pid = fork;\n \tif (not defined $pid) {\n \t\tdie \"fork failed: $!\"\n \t} elsif ($pid == 0) {\n \t\topen STDOUT, \">&\", $out;\n+\t\topen STDERR, \">&\", $err;\n \t\tclose $out;\n \t\texec(@$argv) or die \"cannot exec '$argv->[0]': $!\"\n \t}\n@@ -47,12 +48,28 @@ sub xsendfile {\n \tcopy($in, $out, 4096) or $!{EIO} or die \"cannot copy from child: $!\";\n }\n \n+sub copy_stdio {\n+\tmy ($out, $err) = @_;\n+\tmy $pid = fork;\n+\tdefined $pid or die \"fork failed: $!\";\n+\tif (!$pid) {\n+\t\tclose($out);\n+\t\txsendfile(\\*STDERR, $err);\n+\t\texit 0;\n+\t}\n+\tclose($err);\n+\txsendfile(\\*STDOUT, $out);\n+\tfinish_child($pid) == 0\n+\t\tor exit 1;\n+}\n+\n if ($#ARGV < 1) {\n \tdie \"usage: test-terminal program args\";\n }\n-my $master = new IO::Pty;\n-my $slave = $master->slave;\n-my $pid = start_child(\\@ARGV, $slave);\n-close $slave;\n-xsendfile(\\*STDOUT, $master);\n+my $master_out = new IO::Pty;\n+my $master_err = new IO::Pty;\n+my $pid = start_child(\\@ARGV, $master_out->slave, $master_err->slave);\n+close $master_out->slave;\n+close $master_err->slave;\n+copy_stdio($master_out, $master_err);\n exit(finish_child($pid));\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153635","messageId":"1287254223-4496-4-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-3-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 3/8] test-lib: allow test code to check the list of declared prerequisites","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:36:58Z","receivedAt":"2010-10-16T18:36:58Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jonathan Nieder \n\nThis is plumbing to prepare helpers like test_terminal to notice buggy\ntest scripts that do not declare all of the necessary prerequisites.\n\nSigned-off-by: Jonathan Nieder \n---\n\n No change.\n\n t/test-lib.sh | 26 +++++++++++++++++++-------\n 1 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex dff5e25..94bff17 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -362,6 +362,15 @@ test_have_prereq () {\n \ttest $total_prereq = $ok_prereq\n }\n \n+test_declared_prereq () {\n+\tcase \",$test_prereq,\" in\n+\t*,$1,*)\n+\t\treturn 0\n+\t\t;;\n+\tesac\n+\treturn 1\n+}\n+\n # You are not expected to call test_ok_ and test_failure_ directly, use\n # the text_expect_* functions instead.\n \n@@ -414,17 +423,17 @@ test_skip () {\n \t\t\tbreak\n \t\tesac\n \tdone\n-\tif test -z \"$to_skip\" && test -n \"$prereq\" &&\n-\t ! test_have_prereq \"$prereq\"\n+\tif test -z \"$to_skip\" && test -n \"$test_prereq\" &&\n+\t ! test_have_prereq \"$test_prereq\"\n \tthen\n \t\tto_skip=t\n \tfi\n \tcase \"$to_skip\" in\n \tt)\n \t\tof_prereq=\n-\t\tif test \"$missing_prereq\" != \"$prereq\"\n+\t\tif test \"$missing_prereq\" != \"$test_prereq\"\n \t\tthen\n-\t\t\tof_prereq=\" of $prereq\"\n+\t\t\tof_prereq=\" of $test_prereq\"\n \t\tfi\n \n \t\tsay_color skip >&3 \"skipping test: $@\"\n@@ -438,9 +447,10 @@ test_skip () {\n }\n \n test_expect_failure () {\n-\ttest \"$#\" = 3 && { prereq=$1; shift; } || prereq=\n+\ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-failure\"\n+\texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n \t\tsay >&3 \"checking known breakage: $2\"\n@@ -456,9 +466,10 @@ test_expect_failure () {\n }\n \n test_expect_success () {\n-\ttest \"$#\" = 3 && { prereq=$1; shift; } || prereq=\n+\ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-success\"\n+\texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n \t\tsay >&3 \"expecting success: $2\"\n@@ -500,11 +511,12 @@ test_expect_code () {\n # Usage: test_external description command arguments...\n # Example: test_external 'Perl API' perl ../path/to/test.pl\n test_external () {\n-\ttest \"$#\" = 4 && { prereq=$1; shift; } || prereq=\n+\ttest \"$#\" = 4 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 3 ||\n \terror >&5 \"bug in the test script: not 3 or 4 parameters to test_external\"\n \tdescr=\"$1\"\n \tshift\n+\texport test_prereq\n \tif ! test_skip \"$descr\" \"$@\"\n \tthen\n \t\t# Announce the script to reduce confusion about the\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153634","messageId":"1287254223-4496-5-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-4-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 4/8] test_terminal: catch use without TTY prerequisite","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:36:59Z","receivedAt":"2010-10-16T18:36:59Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jonathan Nieder \n\nIt is easy to forget to declare the TTY prerequisite when\nwriting tests on a system where it would always be satisfied\n(because IO::Pty is installed; see v1.7.3-rc0~33^2, 2010-08-16\nfor example). Automatically detect this problem so there is\nno need to remember.\n\n\ttest_terminal: need to declare TTY prerequisite\n\ttest_must_fail: command not found: test_terminal echo hi\n\ntest_terminal returns status 127 in this case to simulate\nnot being available.\n\nAlso replace the SIMPLEPAGERTTY prerequisite on one test with\n\"SIMPLEPAGER,TTY\", since (1) the latter is supported now and\n(2) the prerequisite detection relies on the TTY prereq being\nexplicitly declared.\n\nSigned-off-by: Jonathan Nieder \n---\n\n Rebased on top of Jeff's series, so that lib-terminal's test_terminal is\n changed instead.\n\n t/lib-terminal.sh | 13 +++++++++++--\n t/t7006-pager.sh | 7 +------\n 2 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 3258b8f..5e7ee9a 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -15,14 +15,23 @@ test_expect_success 'set up terminal for tests' '\n \n if test -e have_tty\n then\n-\ttest_terminal() { \"$@\"; }\n+\ttest_terminal_() { \"$@\"; }\n \ttest_set_prereq TTY\n elif test -e test_terminal_works\n then\n-\ttest_terminal() {\n+\ttest_terminal_() {\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n \t}\n \ttest_set_prereq TTY\n else\n \tsay \"# no usable terminal, so skipping some tests\"\n fi\n+\n+test_terminal () {\n+\tif ! test_declared_prereq TTY\n+\tthen\n+\t\techo >&2 'test_terminal: need to declare TTY prerequisite'\n+\t\treturn 127\n+\tfi\n+\ttest_terminal_ \"$@\"\n+}\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 17e54d3..5641b59 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -184,11 +184,6 @@ test_expect_success 'color when writing to a file intended for a pager' '\n \tcolorful colorful.log\n '\n \n-if test_have_prereq SIMPLEPAGER && test_have_prereq TTY\n-then\n-\ttest_set_prereq SIMPLEPAGERTTY\n-fi\n-\n # Use this helper to make it easy for the caller of your\n # terminal-using function to specify whether it should fail.\n # If you write\n@@ -224,7 +219,7 @@ parse_args() {\n test_default_pager() {\n \tparse_args \"$@\"\n \n-\t$test_expectation SIMPLEPAGERTTY \"$cmd - default pager is used by default\" \"\n+\t$test_expectation SIMPLEPAGER,TTY \"$cmd - default pager is used by default\" \"\n \t\tunset PAGER GIT_PAGER;\n \t\ttest_might_fail git config --unset core.pager &&\n \t\trm -f default_pager_used ||\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153637","messageId":"1287254223-4496-6-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-5-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 5/8] test_terminal: give priority to test-terminal.perl usage","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:37:00Z","receivedAt":"2010-10-16T18:37:00Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jonathan Nieder \n\nTay Ray Chuan wrote:\n\n> [snip]\n> 2. For terminal tests that capture output/stderr, the TTY prerequisite\n> warning does not quite work for things like\n>\n> test_terminal foo >out 2>err\n>\n> because the warning gets \"swallowed\" up by the redirection that's\n> supposed only to be done by the subcommand.\n\nGood catch. Such cases (like Jeff's patch) are not well supported\ncurrently. :(\n\nThe outcome depends on whether stdout was already a terminal (in which\ncase test_terminal is a noop) or not (in which case test_terminal\nintroduces a pseudo-tty in the middle of the pipeline).\n\n\t$ test_terminal.perl sh -c 'test -t 1 && echo >&2 YES' >out\n\tYES\n\t$ sh -c 'test -t 1 && echo >&2 YES' >out\n\t$\n\nHow about this?\n\n - use the test_terminal script even when running with \"-v\"\n if IO::Pty is available, to allow commands like\n\n\ttest_terminal foo >out 2>err\n\n - add a separate TTYREDIR prerequisite which is only set\n when the test_terminal script is usable\n\n - write the \"need to declare TTY prerequisite\" message to fd 4,\n where it will be printed when running tests with -v, rather\n than being swallowed up by an unrelated redireciton.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Tay Ray Chuan \n---\n\n Picked up from a private discussion. I left the discussion in the\n patch message to give some background, and it also gives a nice\n summary of the changes.\n\n t/lib-terminal.sh | 24 +++++++++++++-----------\n 1 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 5e7ee9a..71d147f 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -1,36 +1,38 @@\n #!/bin/sh\n \n test_expect_success 'set up terminal for tests' '\n-\tif test -t 1 && test -t 2\n-\tthen\n-\t\t>have_tty\n-\telif\n+\tif\n \t\ttest_have_prereq PERL &&\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \\\n \t\t\tsh -c \"test -t 1 && test -t 2\"\n \tthen\n \t\t>test_terminal_works\n+\telif test -t 1 && test -t 2\n+\tthen\n+\t\t>have_tty\n \tfi\n '\n \n-if test -e have_tty\n-then\n-\ttest_terminal_() { \"$@\"; }\n-\ttest_set_prereq TTY\n-elif test -e test_terminal_works\n+if test -e test_terminal_works\n then\n \ttest_terminal_() {\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n \t}\n \ttest_set_prereq TTY\n+\ttest_set_prereq TTYREDIR\n+elif test -e have_tty\n+then\n+\ttest_terminal_() { \"$@\"; }\n+\ttest_set_prereq TTY\n+\n else\n \tsay \"# no usable terminal, so skipping some tests\"\n fi\n \n test_terminal () {\n-\tif ! test_declared_prereq TTY\n+\tif ! test_declared_prereq TTY && ! test_declared_prereq TTYREDIR\n \tthen\n-\t\techo >&2 'test_terminal: need to declare TTY prerequisite'\n+\t\techo >&4 'test_terminal: need to declare TTY prerequisite'\n \t\treturn 127\n \tfi\n \ttest_terminal_ \"$@\"\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153638","messageId":"1287254223-4496-7-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-6-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 6/8] t5523-push-upstream: add function to ensure fresh upstream repo","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:37:01Z","receivedAt":"2010-10-16T18:37:01Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan \n---\n t/t5523-push-upstream.sh | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 00da707..5a18533 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -3,8 +3,12 @@\n test_description='push with --set-upstream'\n . ./test-lib.sh\n \n+ensure_fresh_upstream() {\n+\trm -rf parent && git init --bare parent\n+}\n+\n test_expect_success 'setup bare parent' '\n-\tgit init --bare parent &&\n+\tensure_fresh_upstream &&\n \tgit remote add upstream parent\n '\n \n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153639","messageId":"1287254223-4496-8-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-7-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 7/8] t5523-push-upstream: test progress messages","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:37:02Z","receivedAt":"2010-10-16T18:37:02Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Reported-by: Chase Brammer \nSigned-off-by: Tay Ray Chuan \n---\n\n Squashed in Jeff's additional tests, as well as added (missing) TTY\n pre-requisites pointed out by Johnathan.\n\n t/t5523-push-upstream.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 5a18533..f43d760 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -2,6 +2,7 @@\n \n test_description='push with --set-upstream'\n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n ensure_fresh_upstream() {\n \trm -rf parent && git init --bare parent\n@@ -70,4 +71,41 @@ test_expect_success 'push -u HEAD' '\n \tcheck_config headbranch upstream refs/heads/headbranch\n '\n \n+test_expect_success TTY 'progress messages go to tty' '\n+\tensure_fresh_upstream &&\n+\n+\ttest_terminal git push -u upstream master >out 2>err &&\n+\tgrep \"Writing objects\" err\n+'\n+\n+test_expect_failure 'progress messages do not go to non-tty' '\n+\tensure_fresh_upstream &&\n+\n+\t# skip progress messages, since stderr is non-tty\n+\tgit push -u upstream master >out 2>err &&\n+\t! grep \"Writing objects\" err\n+'\n+\n+test_expect_failure 'progress messages go to non-tty (forced)' '\n+\tensure_fresh_upstream &&\n+\n+\t# force progress messages to stderr, even though it is non-tty\n+\tgit push -u --progress upstream master >out 2>err &&\n+\tgrep \"Writing objects\" err\n+'\n+\n+test_expect_success TTY 'push -q suppresses progress' '\n+\tensure_fresh_upstream &&\n+\n+\ttest_terminal git push -u -q upstream master >out 2>err &&\n+\t! grep \"Writing objects\" err\n+'\n+\n+test_expect_failure TTY 'push --no-progress suppresses progress' '\n+\tensure_fresh_upstream &&\n+\n+\ttest_terminal git push -u --no-progress upstream master >out 2>err &&\n+\t! grep \"Writing objects\" err\n+'\n+\n test_done\n-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153640","messageId":"1287254223-4496-9-git-send-email-rctay89@gmail.com","threadId":"25432","inReplyTo":"1287254223-4496-8-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 8/8] push: pass --progress down to git-pack-objects","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-10-16T18:37:03Z","receivedAt":"2010-10-16T18:37:03Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Jeff King \n\nWhen pushing via builtin transports (like file://, git://), the\nunderlying transport helper (in this case, git-pack-objects) did not get\nthe --progress option, even if it was passed to git push.\n\nFix this, and update the tests to reflect this.\n\nNote that according to the git-pack-objects documentation, we can safely\napply the usual --progress semantics for the transport commands like\nclone and fetch (and for pushing over other smart transports).\n\nReported-by: Chase Brammer \nHelped-by: Jonathan Nieder \nSigned-off-by: Jeff King \nSigned-off-by: Tay Ray Chuan \n---\n\n No significant changes other than those incurred while rebasing on\n top of Jeff's patches.\n\n builtin/send-pack.c | 3 +++\n send-pack.h | 1 +\n t/t5523-push-upstream.sh | 4 ++--\n transport.c | 1 +\n 4 files changed, 7 insertions(+), 2 deletions(-)\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..05d7ab1 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -5,6 +5,7 @@ struct send_pack_args {\n \tunsigned verbose:1,\n \t\tquiet:1,\n \t\tporcelain:1,\n+\t\tprogress:1,\n \t\tsend_mirror:1,\n \t\tforce_update:1,\n \t\tuse_thin_pack:1,\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex f43d760..c229fe6 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -78,7 +78,7 @@ test_expect_success TTY 'progress messages go to tty' '\n \tgrep \"Writing objects\" err\n '\n \n-test_expect_failure 'progress messages do not go to non-tty' '\n+test_expect_success 'progress messages do not go to non-tty' '\n \tensure_fresh_upstream &&\n \n \t# skip progress messages, since stderr is non-tty\n@@ -86,7 +86,7 @@ test_expect_failure 'progress messages do not go to non-tty' '\n \t! grep \"Writing objects\" err\n '\n \n-test_expect_failure 'progress messages go to non-tty (forced)' '\n+test_expect_success 'progress messages go to non-tty (forced)' '\n \tensure_fresh_upstream &&\n \n \t# force progress messages to stderr, even though it is non-tty\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-- \n1.7.2.2.513.ge1ef3\n"},{"id":"153643","messageId":"20101017003807.GF20883@burratino","threadId":"25432","inReplyTo":"1287254223-4496-6-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 5/8] test_terminal: give priority to test-terminal.perl usage","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-17T00:38:07Z","receivedAt":"2010-10-17T00:38:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tay Ray Chuan wrote:\n\n> - use the test_terminal script even when running with \"-v\"\n> if IO::Pty is available, to allow commands like\n> \n> \ttest_terminal foo >out 2>err\n> \n> - add a separate TTYREDIR prerequisite which is only set\n> when the test_terminal script is usable\n> \n> - write the \"need to declare TTY prerequisite\" message to fd 4,\n> where it will be printed when running tests with -v, rather\n> than being swallowed up by an unrelated redireciton.\n\nThe patches up to this one look good to me. This one behaves\nas advertised, but I think the API is lousy --- it is just\nbegging people to use the TTY prereq where TTYREDIR is needed.\n\nBetter to change TTY to mean TTYREDIR and drop support for\ntest_terminal on systems without IO::Pty:\n\n-- 8< --\nSubject: test_terminal: ensure redirections work reliably\n\nFor terminal tests that capture output/stderr, the TTY prerequisite\nwarning does not quite work for commands like\n\n\ttest_terminal foo >out 2>err\n\nbecause the warning gets \"swallowed\" up by the redirection that's\nsupposed only to be done by the subcommand.\n\nEven worse, the outcome depends on whether stdout was already a\nterminal (in which case test_terminal is a noop) or not (in which case\ntest_terminal introduces a pseudo-tty in the middle of the pipeline).\n\n\t$ test_terminal.perl sh -c 'test -t 1 && echo >&2 YES' >out\n\tYES\n\t$ sh -c 'test -t 1 && echo >&2 YES' >out\n\t$\n\nSo:\n\n - use the test_terminal script even when running with \"-v\".\n\n - skip tests that require a terminal when the test_terminal\n script is unusable because IO::Pty is not installed.\n\n - write the \"need to declare TTY prerequisite\" message to fd 4,\n where it will be printed when running tests with -v, rather\n than being swallowed up by an unrelated redireciton.\n\nNoticed-by: Tay Ray Chuan \nSigned-off-by: Jonathan Nieder \n---\nThe only other sane alternative I can think of is to introduce\nTTYNOREDIR, since at least people wouldn't be tempted to use\nthat. Distinguishing between\n\n\ttest_expect_success 'foo' '\n\t\ttest_terminal bar >out 2>err\n\t'\n\nand\n\n\ttest_expect_success 'foo' '\n\t\ttest_terminal bar\n\t'\n\nfrom a script run as\n\n\tsh t1234-some-script.sh >log 2>err.log\n\ndoes not seem to be easy without OS-specific hacks like\n\"readlink /dev/fd/1\".\n\n t/lib-terminal.sh | 38 ++++++++++----------------------------\n 1 files changed, 10 insertions(+), 28 deletions(-)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 5e7ee9a..c383b57 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -1,37 +1,19 @@\n #!/bin/sh\n \n test_expect_success 'set up terminal for tests' '\n-\tif test -t 1 && test -t 2\n-\tthen\n-\t\t>have_tty\n-\telif\n+\tif\n \t\ttest_have_prereq PERL &&\n \t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \\\n \t\t\tsh -c \"test -t 1 && test -t 2\"\n \tthen\n-\t\t>test_terminal_works\n+\t\ttest_set_prereq TTY &&\n+\t\ttest_terminal () {\n+\t\t\tif ! test_declared_prereq TTY\n+\t\t\tthen\n+\t\t\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n+\t\t\t\treturn 127\n+\t\t\tfi\n+\t\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n+\t\t}\n \tfi\n '\n-\n-if test -e have_tty\n-then\n-\ttest_terminal_() { \"$@\"; }\n-\ttest_set_prereq TTY\n-elif test -e test_terminal_works\n-then\n-\ttest_terminal_() {\n-\t\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n-\t}\n-\ttest_set_prereq TTY\n-else\n-\tsay \"# no usable terminal, so skipping some tests\"\n-fi\n-\n-test_terminal () {\n-\tif ! test_declared_prereq TTY\n-\tthen\n-\t\techo >&2 'test_terminal: need to declare TTY prerequisite'\n-\t\treturn 127\n-\tfi\n-\ttest_terminal_ \"$@\"\n-}\n-- \n1.7.2.3\n"},{"id":"153644","messageId":"20101017004613.GG20883@burratino","threadId":"25432","inReplyTo":"1287254223-4496-8-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 7/8] t5523-push-upstream: test progress messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-17T00:46:13Z","receivedAt":"2010-10-17T00:46:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tay Ray Chuan wrote:\n\n[...]\n> --- a/t/t5523-push-upstream.sh\n> +++ b/t/t5523-push-upstream.sh\n> @@ -70,4 +71,41 @@ test_expect_success 'push -u HEAD' '\n> \tcheck_config headbranch upstream refs/heads/headbranch\n> '\n> \n> +test_expect_success TTY 'progress messages go to tty' '\n> +\tensure_fresh_upstream &&\n> +\n> +\ttest_terminal git push -u upstream master >out 2>err &&\n> +\tgrep \"Writing objects\" err\n> +'\n\nThanks for testing the usual case. It is easy to forget sometimes.\n\nThe tests using the TTY prerequisite would need to use TTYREDIR\nunless we simplify the latter out of existence.\n"},{"id":"153645","messageId":"20101017005107.GH20883@burratino","threadId":"25432","inReplyTo":"1287254223-4496-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 0/8] fix push --progress over file://, git://, etc.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-17T00:51:07Z","receivedAt":"2010-10-17T00:51:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tay Ray Chuan wrote:\n\n> Jeff King (3):\n> tests: factor out terminal handling from t7006\n> tests: test terminal output to both stdout and stderr\n> push: pass --progress down to git-pack-objects\n> \n> Jonathan Nieder (3):\n> test-lib: allow test code to check the list of declared prerequisites\n> test_terminal: catch use without TTY prerequisite\n> test_terminal: give priority to test-terminal.perl usage\n> \n> Tay Ray Chuan (2):\n> t5523-push-upstream: add function to ensure fresh upstream repo\n> t5523-push-upstream: test progress messages\n\nI've sent some comments on patches 5 (give priority..) and 7 (test\nprogress messages). Except as mentioned,\n\nReviewed-by: Jonathan Nieder \n\nThanks for cleaning up the test_terminal mess.\n"},{"id":"154197","messageId":"20101022194252.GC13059@sigill.intra.peff.net","threadId":"25432","inReplyTo":"1287254223-4496-6-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 5/8] test_terminal: give priority to test-terminal.perl usage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-22T19:42:52Z","receivedAt":"2010-10-22T19:42:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 17, 2010 at 02:37:00AM +0800, Tay Ray Chuan wrote:\n\n> The outcome depends on whether stdout was already a terminal (in which\n> case test_terminal is a noop) or not (in which case test_terminal\n> introduces a pseudo-tty in the middle of the pipeline).\n> \n> \t$ test_terminal.perl sh -c 'test -t 1 && echo >&2 YES' >out\n> \tYES\n> \t$ sh -c 'test -t 1 && echo >&2 YES' >out\n> \t$\n> \n> How about this?\n> \n> - use the test_terminal script even when running with \"-v\"\n> if IO::Pty is available, to allow commands like\n> \n> \ttest_terminal foo >out 2>err\n> \n> - add a separate TTYREDIR prerequisite which is only set\n> when the test_terminal script is usable\n\nIs it even worth keeping the direct-to-tty code at all? Yes, it means\nthat people without IO::Pty can use _some_ terminal tests with \"-v\". But\nit creates a headache for test writers in understanding the subtle\ndifference between TTY and TTYREDIR.\n\n-Peff\n"}]}