{"thread":{"id":"22723","subject":"[PATCH v2 0/7] Re: 'git svn log' no longer uses the pager","startedAt":"2010-02-19T06:50:10Z","lastAt":"2010-02-22T09:12:30Z","messageCount":26,"participants":["Jonathan Nieder","Jeff King","Brandon Casey","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"135045","messageId":"20100219065010.GA22258@progeny.tock","threadId":"22723","inReplyTo":null,"subject":"[PATCH v2 0/7] Re: 'git svn log' no longer uses the pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T06:50:10Z","receivedAt":"2010-02-19T06:50:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Here is a replacement for the jn/maint-fix-pager series currently in\npu. As for the previous version, the point is to fix the breakage\nfrom dec543 (am -i, git-svn: use \"git var GIT_PAGER\", 2009-10-30) \nreported at http://thread.gmane.org/gmane.comp.version-control.git/139831\nI also added some new tests to make sure pagination works in other\nways.\n\nIn this version, patches 1-5 are the same fixes as before, with\ncommit messages tweaked to take into account feedback from the\nprevious round. You can see what they fix by using 'git svn log'\nand the '(v)iew' option of 'git am --interactive', which both\nstopped paginating output in commit dec543. You can see what they\ndon’t break by running some other command such as 'git log' in\ncircumstances where its output is not supposed to be paginated.\n\nThe test in patch 6 has some fixes that didn’t make it to pu last\ntime. To avoid risk of spewing useless output to /dev/tty, the\ntests requiring a terminal only run with the --verbose option.\n\nPatch 7 is the one I am most interested in feedback about. It allows\nautomatic runs of tests without a real terminal by creating its own\npty as needed. But it might be a portability nightmare: is\nposix_openpt widely available? Will open(\"/dev/ptmx\", ...) do just\nas well most places? And how important is it that these tests run\non the most obscure platforms?\n\nBecause these questions are still up in the air for me, I do not\nthink patch 7 is suitable for use outside pu yet. What I would most\nlike to hear is that /dev/ptmx works often enough, because that is\na condition that can be tested for at runtime without adding\ncomplication to the Makefile. But if that is not the case, there\nare other options.\n\nJonathan Nieder (7):\n Fix 'git var' usage synopsis\n Make 'git var GIT_PAGER' always print the configured pager\n git.1: Clarify the behavior of the --paginate option\n git svn: Fix launching of pager\n am: Fix launching of pager\n tests: Add tests for automatic use of pager\n t7006-pager: if stdout is not a terminal, make a new one\n\n .gitignore | 1 +\n Documentation/git-var.txt | 2 +-\n Documentation/git.txt | 8 ++-\n Makefile | 6 ++\n builtin-var.c | 4 +-\n cache.h | 2 +-\n git-am.sh | 5 +-\n git-sh-setup.sh | 13 ++++\n git-svn.perl | 9 ++-\n pager.c | 6 +-\n t/t7006-pager.sh | 171 +++++++++++++++++++++++++++++++++++++++++++++\n test-terminal.c | 62 ++++++++++++++++\n 12 files changed, 274 insertions(+), 15 deletions(-)\n create mode 100644 t/t7006-pager.sh\n create mode 100644 test-terminal.c\n"},{"id":"135046","messageId":"20100219065156.GA29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH 1/7] Fix 'git var' usage synopsis","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T06:51:57Z","receivedAt":"2010-02-19T06:51:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The parameter to 'git var' is not optional.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\nSame as v1, probably should be ejected from the series. I am\nincluding it for reference anyway.\n\n Documentation/git-var.txt | 2 +-\n builtin-var.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex bb98182..458f3e2 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -8,7 +8,7 @@ git-var - Show a git logical variable\n \n SYNOPSIS\n --------\n-'git var' [ -l | ]\n+'git var' ( -l | )\n \n DESCRIPTION\n -----------\ndiff --git a/builtin-var.c b/builtin-var.c\nindex 2280518..e6ee7bc 100644\n--- a/builtin-var.c\n+++ b/builtin-var.c\n@@ -6,7 +6,7 @@\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n \n-static const char var_usage[] = \"git var [-l | ]\";\n+static const char var_usage[] = \"git var (-l | )\";\n \n static const char *editor(int flag)\n {\n-- \n1.7.0\n"},{"id":"135047","messageId":"20100219070053.GB29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH 2/7] Make 'git var GIT_PAGER' always print the configured pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T07:00:53Z","receivedAt":"2010-02-19T07:00:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Scripted commands that want to use git’s configured pager know better\nthan ‘git var’ does whether stdout is going to be a tty at the\nappropriate time. Checking isatty(1) as git_pager() does now won’t\ncut it, since the output of git var itself is almost never a terminal.\nThe symptom is that when used by humans, ‘git var GIT_PAGER’ behaves\nas it should, but when used by scripts, it always returns ‘cat’!\n\nSo avoid tricks with isatty() and just always print the configured\npager.\n\nThis does not fix callers to check isatty(1) themselves yet.\nNevertheless, this patch alone is enough to fix 'am --interactive',\nsince as an interactive command its behavior before before dec543\n(2009-10-30) was to always paginate.\n\nThere are unfortunately no tests for am --interactive or git svn log\nin the test suite, so test suite runs would detect none of this.\n\nThanks to Sebastian Celis for the report and Jeff King for the\nanalysis.\n\nReported-by: Sebastian Celis \nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\nThis commit message should be clearer, I hope.\n\n builtin-var.c | 2 +-\n cache.h | 2 +-\n pager.c | 6 +++---\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-var.c b/builtin-var.c\nindex e6ee7bc..70fdb4d 100644\n--- a/builtin-var.c\n+++ b/builtin-var.c\n@@ -20,7 +20,7 @@ static const char *editor(int flag)\n \n static const char *pager(int flag)\n {\n-\tconst char *pgm = git_pager();\n+\tconst char *pgm = git_pager(1);\n \n \tif (!pgm)\n \t\tpgm = \"cat\";\ndiff --git a/cache.h b/cache.h\nindex d478eff..d454b7e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -775,7 +775,7 @@ extern const char *git_committer_info(int);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\n extern const char *fmt_name(const char *name, const char *email);\n extern const char *git_editor(void);\n-extern const char *git_pager(void);\n+extern const char *git_pager(int stdout_is_tty);\n \n struct checkout {\n \tconst char *base_dir;\ndiff --git a/pager.c b/pager.c\nindex 2c7e8ec..dac358f 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -48,11 +48,11 @@ static void wait_for_pager_signal(int signo)\n \traise(signo);\n }\n \n-const char *git_pager(void)\n+const char *git_pager(int stdout_is_tty)\n {\n \tconst char *pager;\n \n-\tif (!isatty(1))\n+\tif (!stdout_is_tty)\n \t\treturn NULL;\n \n \tpager = getenv(\"GIT_PAGER\");\n@@ -73,7 +73,7 @@ const char *git_pager(void)\n \n void setup_pager(void)\n {\n-\tconst char *pager = git_pager();\n+\tconst char *pager = git_pager(isatty(1));\n \n \tif (!pager)\n \t\treturn;\n-- \n1.7.0\n"},{"id":"135049","messageId":"20100219070639.GC29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH 3/7] git.1: Clarify the behavior of the --paginate option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T07:06:39Z","receivedAt":"2010-02-19T07:06:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The --paginate option is meant to negate the effect of an explicit or\nimplicit pager. = false setting. Thus it turns the pager on if\noutput is going to a terminal rather than unconditionally.\n\nExplaining this requires mentioning configuration options very early\nin the manual, when some users might not be familiar with them yet.\nProvide some pointers to help: to the relevant section of git.1 for\nthe configuration mechanism in general and to git-config.1 for the\npager. options.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\nAdd a second paragraph to explain the less obvious part of the patch.\nJeff reverse-engineered this all correctly in [1], but one shouldn’t\nhave to think that hard to see where a patch is coming from.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/139831/focus=139971\n\n Documentation/git.txt | 8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 01c4631..f26641a 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -229,7 +229,10 @@ help ...`.\n \n -p::\n --paginate::\n-\tPipe all output into 'less' (or if set, $PAGER).\n+\tPipe all output into 'less' (or if set, $PAGER) if standard\n+\toutput is a terminal. This overrides the `pager.`\n+\tconfiguration options (see the \"Configuration Mechanism\" section\n+\tbelow).\n \n --no-pager::\n \tDo not pipe git output into a pager.\n@@ -401,7 +404,8 @@ people. Here is an example:\n ------------\n \n Various commands read from the configuration file and adjust\n-their operation accordingly.\n+their operation accordingly. See linkgit:git-config[1] for a\n+list.\n \n \n Identifier Terminology\n-- \n1.7.0\n"},{"id":"135050","messageId":"20100219070954.GD29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH 4/7] git svn: Fix launching of pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T07:09:54Z","receivedAt":"2010-02-19T07:09:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In commit dec543e (am -i, git-svn: use \"git var GIT_PAGER\"), I tried\nto teach git svn to defer to git var on what pager to use. In the\nprocess, I introduced two bugs:\n\n - The value set for $pager in config_pager has local scope, so\n run_pager never sees it;\n\n - git var cannot tell whether git svn’s output is going to a\n terminal, so the value chosen for $pager does not reflect that\n information.\n\nFix them.\n\nReported-by: Sebastian Celis \nSigned-off-by: Jonathan Nieder \nAcked-by: Eric Wong \nSigned-off-by: Junio C Hamano \n---\nEric Wong wrote:\n\n> Thanks Jonathan, confirmed this works with patch 2 of this series.\n>\n> Acked-by: Eric Wong \n\nThanks.\n\n git-svn.perl | 9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 265852f..473a0b9 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -5459,7 +5459,12 @@ sub git_svn_log_cmd {\n \n # adapted from pager.c\n sub config_pager {\n-\tchomp(my $pager = command_oneline(qw(var GIT_PAGER)));\n+\tif (! -t *STDOUT) {\n+\t\t$ENV{GIT_PAGER_IN_USE} = 'false';\n+\t\t$pager = undef;\n+\t\treturn;\n+\t}\n+\tchomp($pager = command_oneline(qw(var GIT_PAGER)));\n \tif ($pager eq 'cat') {\n \t\t$pager = undef;\n \t}\n@@ -5467,7 +5472,7 @@ sub config_pager {\n }\n \n sub run_pager {\n-\treturn unless -t *STDOUT && defined $pager;\n+\treturn unless defined $pager;\n \tpipe my ($rfd, $wfd) or return;\n \tdefined(my $pid = fork) or ::fatal \"Can't fork: $!\";\n \tif (!$pid) {\n-- \n1.7.0\n"},{"id":"135051","messageId":"20100219071203.GE29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH 5/7] am: Fix launching of pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T07:12:03Z","receivedAt":"2010-02-19T07:12:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The pagination functionality in git am has some problems:\n\n - It does not check if stdout is a tty, so it always paginates.\n\n - If $GIT_PAGER uses any environment variables, they are being\n ignored, since it does not run $GIT_PAGER through eval.\n\n - If $GIT_PAGER is set to the empty string, instead of passing\n output through to stdout, it tries to run $dotest/patch.\n\nFix them. While at it, move the definition of git_pager() to\ngit-sh-setup so authors of other commands are not tempted to\nreimplement it with the same mistakes.\n\nHelped-by: Jeff King \nSigned-off-by: Jonathan Nieder \n---\nSame as before, included for reference.\n\n git-am.sh | 5 +----\n git-sh-setup.sh | 13 +++++++++++++\n 2 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 3c08d53..b11af03 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -663,10 +663,7 @@ do\n \t\t[eE]*) git_editor \"$dotest/final-commit\"\n \t\t action=again ;;\n \t\t[vV]*) action=again\n-\t\t : ${GIT_PAGER=$(git var GIT_PAGER)}\n-\t\t : ${LESS=-FRSX}\n-\t\t export LESS\n-\t\t $GIT_PAGER \"$dotest/patch\" ;;\n+\t\t git_pager \"$dotest/patch\" ;;\n \t\t*) action=again ;;\n \t\tesac\n \t done\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex d56426d..44fb467 100755\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -107,6 +107,19 @@ git_editor() {\n \teval \"$GIT_EDITOR\" '\"$@\"'\n }\n \n+git_pager() {\n+\tif test -t 1\n+\tthen\n+\t\tGIT_PAGER=$(git var GIT_PAGER)\n+\telse\n+\t\tGIT_PAGER=cat\n+\tfi\n+\t: ${LESS=-FRSX}\n+\texport LESS\n+\n+\teval \"$GIT_PAGER\" '\"$@\"'\n+}\n+\n sane_grep () {\n \tGREP_OPTIONS= LC_ALL=C grep \"$@\"\n }\n-- \n1.7.0\n"},{"id":"135053","messageId":"20100219071857.GF29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH 6/7] tests: Add tests for automatic use of pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T07:18:58Z","receivedAt":"2010-02-19T07:18:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Git’s automatic pagination support has some subtleties. Add some\ntests to make sure we don’t break:\n\n - when git will use a pager by default;\n - the effect of the --paginate and --no-pager options;\n - the effect of pagination on use of color;\n - how the choice of pager is configured.\n\nThis does not yet test:\n\n - use of pager by scripted commands (git svn and git am);\n - effect of the pager.* configuration variables;\n - setting of the LESS variable.\n\nSome features involve checking whether stdout is a terminal, so many\nof these tests are skipped unless output is passed through to the\nterminal (i.e., unless $GIT_TEST_OPTS includes --verbose).\n\nThe immediate purpose for these tests was to avoid making things worse\nafter the breakage from my jn/editor-pager series (see commit 376f39,\n2009-11-20). Thanks to Sebastian Celis \nfor the report.\n\nHelped-by: Jeff King \nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\n t/t7006-pager.sh | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 163 insertions(+), 0 deletions(-)\n create mode 100644 t/t7006-pager.sh\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nnew file mode 100644\nindex 0000000..2e9cb9d\n--- /dev/null\n+++ b/t/t7006-pager.sh\n@@ -0,0 +1,163 @@\n+#!/bin/sh\n+\n+test_description='Test automatic use of a pager.'\n+\n+. ./test-lib.sh\n+\n+rm -f stdout_is_tty\n+test_expect_success 'is stdout a terminal?' '\n+\tif test -t 1\n+\tthen\n+\t\t: > stdout_is_tty\n+\tfi\n+'\n+\n+if test -e stdout_is_tty\n+then\n+\ttest_set_prereq TTY\n+else\n+\tsay stdout is not a terminal, so skipping some tests.\n+fi\n+\n+unset GIT_PAGER GIT_PAGER_IN_USE\n+git config --unset core.pager\n+PAGER='cat > paginated.out'\n+export PAGER\n+\n+test_expect_success 'setup' '\n+\ttest_commit initial\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'some commands use a pager' '\n+\tgit log &&\n+\ttest -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'some commands do not use a pager' '\n+\tgit rev-list HEAD &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success 'no pager when stdout is a pipe' '\n+\tgit log | cat &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success 'no pager when stdout is a regular file' '\n+\tgit log > file &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'git --paginate rev-list uses a pager' '\n+\tgit --paginate rev-list HEAD &&\n+\ttest -e paginated.out\n+'\n+\n+rm -f file paginated.out\n+test_expect_success 'no pager even with --paginate when stdout is a pipe' '\n+\tgit --paginate log | cat &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'no pager with --no-pager' '\n+\tgit --no-pager log &&\n+\t! test -e paginated.out\n+'\n+\n+# A colored commit log will begin with an appropriate ANSI escape\n+# for the first color; the text \"commit\" comes later.\n+colorful() {\n+\tread firstline < $1\n+\t! expr \"$firstline\" : \"^[a-zA-Z]\" >/dev/null\n+}\n+\n+rm -f colorful.log colorless.log\n+test_expect_success 'tests can detect color' '\n+\tgit log --no-color > colorless.log &&\n+\tgit log --color > colorful.log &&\n+\t! colorful colorless.log &&\n+\tcolorful colorful.log\n+'\n+\n+rm -f colorless.log\n+git config color.ui auto\n+test_expect_success 'no color when stdout is a regular file' '\n+\tgit log > colorless.log &&\n+\t! colorful colorless.log\n+'\n+\n+rm -f paginated.out\n+git config color.ui auto\n+test_expect_success TTY 'color when writing to a pager' '\n+\tTERM=vt100 test_terminal git log &&\n+\tcolorful paginated.out\n+'\n+\n+rm -f colorful.log\n+git config color.ui auto\n+test_expect_success 'color when writing to a file intended for a pager' '\n+\tTERM=vt100 GIT_PAGER_IN_USE=true git log > colorful.log &&\n+\tcolorful colorful.log\n+'\n+\n+unset PAGER GIT_PAGER\n+git config --unset core.pager\n+test_expect_success 'determine default pager' '\n+\tless=$(git var GIT_PAGER) &&\n+\ttest -n \"$less\"\n+'\n+\n+if expr \"$less\" : '^[a-z]*$' > /dev/null && test_have_prereq TTY\n+then\n+\ttest_set_prereq SIMPLEPAGER\n+fi\n+\n+unset PAGER GIT_PAGER\n+git config --unset core.pager\n+rm -f default_pager_used\n+test_expect_success SIMPLEPAGER 'default pager is used by default' '\n+\tcat > $less <<-EOF &&\n+\t#!$SHELL_PATH\n+\t: > default_pager_used\n+\tEOF\n+\tchmod +x $less &&\n+\tPATH=.:$PATH git log &&\n+\ttest -e default_pager_used\n+'\n+\n+unset GIT_PAGER\n+git config --unset core.pager\n+rm -f PAGER_used\n+test_expect_success TTY 'PAGER overrides default pager' '\n+\tPAGER=\": > PAGER_used\" &&\n+\texport PAGER &&\n+\tgit log &&\n+\ttest -e PAGER_used\n+'\n+\n+unset GIT_PAGER\n+rm -f core.pager_used\n+test_expect_success TTY 'core.pager overrides PAGER' '\n+\tPAGER=: &&\n+\texport PAGER &&\n+\tgit config core.pager \": > core.pager_used\" &&\n+\tgit log &&\n+\ttest -e core.pager_used\n+'\n+\n+rm -f GIT_PAGER_used\n+test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n+\tgit config core.pager : &&\n+\tGIT_PAGER=\": > GIT_PAGER_used\" &&\n+\texport GIT_PAGER &&\n+\tgit log &&\n+\ttest -e GIT_PAGER_used\n+'\n+\n+test_done\n-- \n1.7.0\n"},{"id":"135054","messageId":"20100219072331.GG29916@progeny.tock","threadId":"22723","inReplyTo":"20100219065010.GA22258@progeny.tock","subject":"[PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T07:23:31Z","receivedAt":"2010-02-19T07:23:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Test t7006-pager skips most of its tests except when running with\n--verbose, for lack of a terminal to use. Thus many of the tests\nwould not get much coverage, since whole testsuite runs with --verbose\nare rare. Redirecting output to /dev/tty would be problematic because\n(1) there really could be no controlling terminal and (2) test\nbreakage might manifest itself by the producing extraneous output.\nLuckily, there is a way around this: Unix98-style pseudo-terminals\nallow us to create a fake terminal on the fly and capture its output.\n\nDo so. The new test-terminal command to accomplish this uses\nposix_openpt (from SuSv3) to create a terminal because that is good\nenough on Linux. I would like some feedback on what platforms are\nmissing that function and thus what alternate interfaces are worth\nsupporting. The perl IO::Tty module could take care of this for us,\nbut I do not think it is worth the extra dependency.\n\nThe test-terminal facility can be disabled by passing\nNO_POSIX_OPENPT=YesPlease to the makefile, in which case the test\nwill revert to its old behavior (skipping most tests unless\nGIT_TEST_OPTS includes --verbose).\n\nSigned-off-by: Jonathan Nieder \n---\nRequiring users to add NO_POSIX_OPENPT themselves doesn’t seem so\ngreat. I did it that way just so I could send something and get\nsome feedback; a more suitable version would need a way to detect\nmissing openpt on the fly, for example by noticing the compilation\nfailure and continuing anyway.\n\n .gitignore | 1 +\n Makefile | 6 +++++\n t/t7006-pager.sh | 28 +++++++++++++++--------\n test-terminal.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 87 insertions(+), 10 deletions(-)\n create mode 100644 test-terminal.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 8df8f88..6b405ba 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -169,6 +169,7 @@\n /test-run-command\n /test-sha1\n /test-sigchain\n+/test-terminal\n /common-cmds.h\n *.tar.gz\n *.dsc\ndiff --git a/Makefile b/Makefile\nindex 7bf2fca..9c900a7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -45,6 +45,8 @@ all::\n #\n # Define NO_MEMMEM if you don't have memmem.\n #\n+# Define NO_POSIX_OPENPT if you don't have posix_openpt (for tests).\n+#\n # Define NO_STRLCPY if you don't have strlcpy.\n #\n # Define NO_STRTOUMAX if you don't have strtoumax in the C library.\n@@ -1727,6 +1729,10 @@ TEST_PROGRAMS_NEED_X += test-sha1\n TEST_PROGRAMS_NEED_X += test-sigchain\n TEST_PROGRAMS_NEED_X += test-index-version\n \n+ifndef NO_POSIX_OPENPT\n+TEST_PROGRAMS_NEED_X += test-terminal\n+endif\n+\n TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))\n \n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 2e9cb9d..21788e3 100644\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -5,18 +5,26 @@ test_description='Test automatic use of a pager.'\n . ./test-lib.sh\n \n rm -f stdout_is_tty\n-test_expect_success 'is stdout a terminal?' '\n+test_expect_success 'set up terminal for tests' '\n \tif test -t 1\n \tthen\n \t\t: > stdout_is_tty\n+\telif test-terminal sh -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() { test-terminal \"$@\"; }\n \ttest_set_prereq TTY\n else\n-\tsay stdout is not a terminal, so skipping some tests.\n+\tsay no usable terminal, so skipping some tests\n fi\n \n unset GIT_PAGER GIT_PAGER_IN_USE\n@@ -30,13 +38,13 @@ test_expect_success 'setup' '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands use a pager' '\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e paginated.out\n '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands do not use a pager' '\n-\tgit rev-list HEAD &&\n+\ttest_terminal git rev-list HEAD &&\n \t! test -e paginated.out\n '\n \n@@ -54,7 +62,7 @@ test_expect_success 'no pager when stdout is a regular file' '\n \n rm -f paginated.out\n test_expect_success TTY 'git --paginate rev-list uses a pager' '\n-\tgit --paginate rev-list HEAD &&\n+\ttest_terminal git --paginate rev-list HEAD &&\n \ttest -e paginated.out\n '\n \n@@ -66,7 +74,7 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '\n \n rm -f paginated.out\n test_expect_success TTY 'no pager with --no-pager' '\n-\tgit --no-pager log &&\n+\ttest_terminal git --no-pager log &&\n \t! test -e paginated.out\n '\n \n@@ -127,7 +135,7 @@ test_expect_success SIMPLEPAGER 'default pager is used by default' '\n \t: > default_pager_used\n \tEOF\n \tchmod +x $less &&\n-\tPATH=.:$PATH git log &&\n+\tPATH=.:$PATH test_terminal git log &&\n \ttest -e default_pager_used\n '\n \n@@ -137,7 +145,7 @@ rm -f PAGER_used\n test_expect_success TTY 'PAGER overrides default pager' '\n \tPAGER=\": > PAGER_used\" &&\n \texport PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e PAGER_used\n '\n \n@@ -147,7 +155,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '\n \tPAGER=: &&\n \texport PAGER &&\n \tgit config core.pager \": > core.pager_used\" &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e core.pager_used\n '\n \n@@ -156,7 +164,7 @@ test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n \tgit config core.pager : &&\n \tGIT_PAGER=\": > GIT_PAGER_used\" &&\n \texport GIT_PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e GIT_PAGER_used\n '\n \ndiff --git a/test-terminal.c b/test-terminal.c\nnew file mode 100644\nindex 0000000..f4d6a71\n--- /dev/null\n+++ b/test-terminal.c\n@@ -0,0 +1,62 @@\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+\n+static void newtty(int *master, int *slave)\n+{\n+\tint fd;\n+\tconst char *term;\n+\n+\tfd = posix_openpt(O_RDWR|O_NOCTTY);\n+\tif (fd == -1 || grantpt(fd) || unlockpt(fd) || !(term = ptsname(fd)))\n+\t\tdie_errno(\"Could not allocate a new pseudo-terminal\");\n+\t*master = fd;\n+\t*slave = open(term, O_RDWR|O_NOCTTY);\n+\tif (*slave == -1)\n+\t\tdie_errno(\"Could not open pseudo-terminal: %s\", term);\n+}\n+\n+static void setup_child(struct child_process *chld,\n+\t\t\tconst char **args, int out)\n+{\n+\tmemset(chld, 0, sizeof(*chld));\n+\tchld->argv = args;\n+\tchld->out = out;\n+}\n+\n+static void xsendfile(int out_fd, int in_fd)\n+{\n+\tchar buf[BUFSIZ];\n+\n+\t/* Note: the real sendfile() cannot read from a terminal. */\n+\tfor (;;) {\n+\t\tssize_t n = xread(in_fd, buf, sizeof(buf));\n+\n+\t\t/*\n+\t\t * It is unspecified by POSIX whether reads\n+\t\t * from a disconnected terminal will return\n+\t\t * EIO (as in AIX 4.x, IRIX, and Linux) or\n+\t\t * end-of-file. Either is fine.\n+\t\t */\n+\t\tif (n == 0 || (n == -1 && errno == EIO))\n+\t\t\tbreak;\n+\t\tif (n == -1)\n+\t\t\tdie_errno(\"cannot read from child\");\n+\t\twrite_or_die(out_fd, buf, n);\n+\t}\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tint masterfd, slavefd;\n+\tstruct child_process chld;\n+\n+\tif (argc < 2 || (argc == 2 && !strcmp(argv[1], \"-h\")))\n+\t\tusage(\"test-terminal program args\");\n+\n+\tnewtty(&masterfd, &slavefd);\n+\tsetup_child(&chld, argv + 1, slavefd);\n+\tif (start_command(&chld))\n+\t\treturn error(\"failed to execute child\");\n+\txsendfile(1, masterfd);\n+\treturn finish_command(&chld);\n+}\n-- \n1.7.0\n"},{"id":"135059","messageId":"20100219080819.GA13691@coredump.intra.peff.net","threadId":"22723","inReplyTo":"20100219072331.GG29916@progeny.tock","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-19T08:08:19Z","receivedAt":"2010-02-19T08:08:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 19, 2010 at 01:23:31AM -0600, Jonathan Nieder wrote:\n\n> Test t7006-pager skips most of its tests except when running with\n> --verbose, for lack of a terminal to use. Thus many of the tests\n> would not get much coverage, since whole testsuite runs with --verbose\n> are rare. Redirecting output to /dev/tty would be problematic because\n> (1) there really could be no controlling terminal and (2) test\n> breakage might manifest itself by the producing extraneous output.\n> Luckily, there is a way around this: Unix98-style pseudo-terminals\n> allow us to create a fake terminal on the fly and capture its output.\n> \n> Do so. The new test-terminal command to accomplish this uses\n> posix_openpt (from SuSv3) to create a terminal because that is good\n> enough on Linux. I would like some feedback on what platforms are\n> missing that function and thus what alternate interfaces are worth\n> supporting. The perl IO::Tty module could take care of this for us,\n> but I do not think it is worth the extra dependency.\n\nSolaris 8 and 9 seem to be lacking it. Solaris 10 does have it. AIX 5.2\nand 6.1 both have it.\n\nSo it would mean some platforms couldn't run all tests. That is probably\ngood enough, given that most of our terminal-related bugs have not been\nplatform-specific problems.\n\nStill, it seems like just wrapping isatty would be simpler. I guess you\nare opposed to carrying around test-specific code in the main git\nbinary?\n\n-Peff\n"},{"id":"135062","messageId":"20100219081947.GA12975@progeny.tock","threadId":"22723","inReplyTo":"20100219080819.GA13691@coredump.intra.peff.net","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-19T08:19:47Z","receivedAt":"2010-02-19T08:19:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Solaris 8 and 9 seem to be lacking it. Solaris 10 does have it. AIX 5.2\n> and 6.1 both have it.\n> \n> So it would mean some platforms couldn't run all tests. That is probably\n> good enough, given that most of our terminal-related bugs have not been\n> platform-specific problems.\n\nHmm, how about /dev/ptmx? (One can check by replacing posix_openpt(...)\nwith open(\"/dev/ptmx\", ...) in the test-terminal.c I sent.)\n\n> Still, it seems like just wrapping isatty would be simpler. I guess you\n> are opposed to carrying around test-specific code in the main git\n> binary?\n\nNo, not opposed. Just lazy and not so interested in working on it.\nI do not want to just take the implementation you provided because I\nwant to test the scripted git commands, too, though I haven’t gotten\naround to that.\n\nJonathan\n"},{"id":"135065","messageId":"20100219083440.GC13691@coredump.intra.peff.net","threadId":"22723","inReplyTo":"20100219081947.GA12975@progeny.tock","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-19T08:34:41Z","receivedAt":"2010-02-19T08:34:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 19, 2010 at 02:19:47AM -0600, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > Solaris 8 and 9 seem to be lacking it. Solaris 10 does have it. AIX 5.2\n> > and 6.1 both have it.\n> > \n> > So it would mean some platforms couldn't run all tests. That is probably\n> > good enough, given that most of our terminal-related bugs have not been\n> > platform-specific problems.\n> \n> Hmm, how about /dev/ptmx? (One can check by replacing posix_openpt(...)\n> with open(\"/dev/ptmx\", ...) in the test-terminal.c I sent.)\n\nSolaris 8 does have /dev/ptmx. Sorry, I wasn't able to test it, as my\nsol8 box is very vanilla right now, and I can't actually compile git on\nit (it doesn't even have gmake). :)\n\nMaybe Brandon (cc'd) can try it.\n\n> No, not opposed. Just lazy and not so interested in working on it.\n> I do not want to just take the implementation you provided because I\n> want to test the scripted git commands, too, though I haven’t gotten\n> around to that.\n\nAh, right, I forgot about scripts. That makes things a little uglier,\nbut it should theoretically be possible.\n\n-Peff\n"},{"id":"135086","messageId":"28d--OB9y5MtIy1nJel2Km6d5sqJ3yX6fVUhecDU5ehJUYJmZlE0-A@cipher.nrlssc.navy.mil","threadId":"22723","inReplyTo":"20100219083440.GC13691@coredump.intra.peff.net","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-02-19T16:25:29Z","receivedAt":"2010-02-19T16:25:29Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Jeff King wrote:\n> On Fri, Feb 19, 2010 at 02:19:47AM -0600, Jonathan Nieder wrote:\n> \n>> Jeff King wrote:\n>>\n>>> Solaris 8 and 9 seem to be lacking it. Solaris 10 does have it. AIX 5.2\n>>> and 6.1 both have it.\n>>>\n>>> So it would mean some platforms couldn't run all tests. That is probably\n>>> good enough, given that most of our terminal-related bugs have not been\n>>> platform-specific problems.\n>> Hmm, how about /dev/ptmx? (One can check by replacing posix_openpt(...)\n>> with open(\"/dev/ptmx\", ...) in the test-terminal.c I sent.)\n> \n> Solaris 8 does have /dev/ptmx. Sorry, I wasn't able to test it, as my\n> sol8 box is very vanilla right now, and I can't actually compile git on\n> it (it doesn't even have gmake). :)\n> \n> Maybe Brandon (cc'd) can try it.\n\nI'm compiling vanilla master now. If that compiles without error,\nI'll apply Jonathan's series and make the changes to test-terminal.c\nthat he suggested. I have access to Solaris 7, so you can imagine\nhow fast the machine is... and how long the compile will take.\n\n-brandon\n"},{"id":"135136","messageId":"setkR9a6yaRMOU11ekUmlY6ty8nNSPCcidBKotdDxpQsZLCxc54hKw@cipher.nrlssc.navy.mil","threadId":"22723","inReplyTo":"28d--OB9y5MtIy1nJel2Km6d5sqJ3yX6fVUhecDU5ehJUYJmZlE0-A@cipher.nrlssc.navy.mil","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-02-20T00:29:21Z","receivedAt":"2010-02-20T00:29:21Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":">> On Fri, Feb 19, 2010 at 02:19:47AM -0600, Jonathan Nieder wrote:\n>>\n>>> Jeff King wrote:\n>>>\n>>>> Solaris 8 and 9 seem to be lacking it. Solaris 10 does have it. AIX 5.2\n>>>> and 6.1 both have it.\n>>>>\n>>>> So it would mean some platforms couldn't run all tests. That is probably\n>>>> good enough, given that most of our terminal-related bugs have not been\n>>>> platform-specific problems.\n>>> Hmm, how about /dev/ptmx? (One can check by replacing posix_openpt(...)\n>>> with open(\"/dev/ptmx\", ...) in the test-terminal.c I sent.)\n\nDidn't work on Solaris 7. I applied your series on top of master with the\nchange you described. Here's the diff:\n\ndiff --git a/test-terminal.c b/test-terminal.c\nindex f4d6a71..6408a7d 100644\n--- a/test-terminal.c\n+++ b/test-terminal.c\n@@ -6,7 +6,7 @@ static void newtty(int *master, int *slave)\n int fd;\n const char *term;\n \n- fd = posix_openpt(O_RDWR|O_NOCTTY);\n+ fd = open(\"/dev/ptmx\", O_RDWR|O_NOCTTY);\n if (fd == -1 || grantpt(fd) || unlockpt(fd) || !(term = ptsname(fd)))\n die_errno(\"Could not allocate a new pseudo-terminal\");\n *master = fd;\n\n\nHere's the result of the terminal test:\n\n # ./test-terminal sh -c \"test -t 1\"\n # echo $?\n 1\n\n\nAnd here's the output of t7006-pager:\n*** t7006-pager.sh ***\n* ok 1: set up terminal for tests\n* no usable terminal, so skipping some tests\n* ok 2: setup\n* skip 3: some commands use a pager\n* skip 4: some commands do not use a pager\n* ok 5: no pager when stdout is a pipe\n* ok 6: no pager when stdout is a regular file\n* skip 7: git --paginate rev-list uses a pager\n* ok 8: no pager even with --paginate when stdout is a pipe\n* skip 9: no pager with --no-pager\n* ok 10: tests can detect color\n* ok 11: no color when stdout is a regular file\n* skip 12: color when writing to a pager\n* ok 13: color when writing to a file intended for a pager\n* ok 14: determine default pager\n* skip 15: default pager is used by default\n* skip 16: PAGER overrides default pager\n* skip 17: core.pager overrides PAGER\n* skip 18: GIT_PAGER overrides core.pager\n* passed all 18 test(s)\n\nhth,\n-brandon\n"},{"id":"135137","messageId":"20100220003950.GA18550@progeny.tock","threadId":"22723","inReplyTo":"setkR9a6yaRMOU11ekUmlY6ty8nNSPCcidBKotdDxpQsZLCxc54hKw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-20T00:39:51Z","receivedAt":"2010-02-20T00:39:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Brandon Casey wrote:\n>>> On Fri, Feb 19, 2010 at 02:19:47AM -0600, Jonathan Nieder wrote:\n\n>>>> Hmm, how about /dev/ptmx? (One can check by replacing posix_openpt(...)\n>>>> with open(\"/dev/ptmx\", ...) in the test-terminal.c I sent.)\n> \n> Didn't work on Solaris 7. I applied your series on top of master with the\n> change you described. Here's the diff:\n[...]\n> Here's the result of the terminal test:\n> \n> # ./test-terminal sh -c \"test -t 1\"\n> # echo $?\n> 1\n[...]\n>\n> And here's the output of t7006-pager:\n> *** t7006-pager.sh ***\n> * ok 1: set up terminal for tests\n> * no usable terminal, so skipping some tests\n\nThanks for trying it out. That’s an excellent outcome: it means that\ntest-terminal compiled without trouble with no makefile magic. It\ndoes seem strange to me that there was no error message. Is\nsh -c \"test -t 1\" false for an ordinary terminal?\n\nJonathan\n"},{"id":"135140","messageId":"ee63ef31002191942h7fbbb6bt627cd36ea343e606@mail.gmail.com","threadId":"22723","inReplyTo":"20100220003950.GA18550@progeny.tock","subject":"Re: [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2010-02-20T03:42:00Z","receivedAt":"2010-02-20T03:42:00Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Fri, Feb 19, 2010 at 6:39 PM, Jonathan Nieder wrote:\n> Brandon Casey wrote:\n>>>> On Fri, Feb 19, 2010 at 02:19:47AM -0600, Jonathan Nieder wrote:\n>\n>>>>> Hmm, how about /dev/ptmx?  (One can check by replacing posix_openpt(...)\n>>>>> with open(\"/dev/ptmx\", ...) in the test-terminal.c I sent.)\n>>\n>> Didn't work on Solaris 7.  I applied your series on top of master with the\n>> change you described.  Here's the diff:\n> [...]\n>> Here's the result of the terminal test:\n>>\n>>   # ./test-terminal sh -c \"test -t 1\"\n>>   # echo $?\n>>   1\n> [...]\n>>\n>> And here's the output of t7006-pager:\n>> *** t7006-pager.sh ***\n>> *   ok 1: set up terminal for tests\n>> * no usable terminal, so skipping some tests\n>\n> Thanks for trying it out.  That’s an excellent outcome: it means that\n> test-terminal compiled without trouble with no makefile magic.  It\n> does seem strange to me that there was no error message.  Is\n> sh -c \"test -t 1\" false for an ordinary terminal?\n\nNo, it is true. I also substituted /usr/xpg4/bin/sh and got the same result.\n\n-brandon\n"},{"id":"135143","messageId":"20100220052504.GA24697@progeny.tock","threadId":"22723","inReplyTo":"ee63ef31002191942h7fbbb6bt627cd36ea343e606@mail.gmail.com","subject":"[PATCH v2 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-20T05:25:18Z","receivedAt":"2010-02-20T05:25:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Testing pagination requires (fake or real) access to a terminal so we\ncan see whether the pagination automatically kicks in, which makes it\nhard to get good coverage when running tests without --verbose. There\nare a number of ways to work around that:\n\n - Replace all isatty calls with calls to a custom xisatty wrapper\n that usually checks for a terminal but can be overridden for tests.\n This would be workable, but it would require implementing xisatty\n separately in three languages (C, shell, and perl) and making sure\n that any code that is to be tested always uses the wrapper.\n\n - Redirect stdout to /dev/tty. This would be problematic because\n there might be no terminal available, and even if a terminal is\n available, it might not be appropriate to spew output to it.\n\n - Create a new pseudo-terminal on the fly and capture its output.\n\nThis patch implements the third approach.\n\nThe new test-terminal command opens /dev/ptmx to create a terminal and\nexecutes the program specified by its arguments with that terminal as\nstdout. All platforms I know of with SuSv3 posix_openpt implement it\nas open(\"/dev/ptmx\", ...), so this should be just as portable.\n\nPre-SuSv3 systems may not have the proper support, but that is okay.\nIf SVR4 STREAMS are supported, all the functions used by test-terminal\nwill be available and succeed, though the result will not satisfy\nisatty() unless the appropriate ioctl()s are used. The test suite\nchecks for this and will not use test-terminal in that case.\n\nOn non-Unixlike and pre-SVR4 systems, this functionality should be\ndisabled by passing NO_GRANTPT=YesPlease to the makefile. I do not\nknow whether Cygwin or HP-UX implements the required functions\nappropriately, so to be safe this patch includes that option for them.\n\nOn platforms where test-terminal does not work, the test script will\nmaintain its old behavior (skipping most of its tests unless\nGIT_TEST_OPTS includes --verbose).\n\nThanks to Brandon Casey and Jeff King\n for porting help.\n\nSigned-off-by: Jonathan Nieder \n---\nBrandon Casey wrote:\n> On Fri, Feb 19, 2010 at 6:39 PM, Jonathan Nieder wrote:\n\n>> Thanks for trying it out.  That’s an excellent outcome: it means that\n>> test-terminal compiled without trouble with no makefile magic.  It\n>> does seem strange to me that there was no error message.  Is\n>> sh -c \"test -t 1\" false for an ordinary terminal?\n>\n> No, it is true. I also substituted /usr/xpg4/bin/sh and got the same result.\n\nAh, okay. I read up a little more, and it seems that old STREAMS-based\nimplementations of pseudo-terminals require\n\n\tioctl(*slave, I_PUSH, \"ptem\");\n\tioctl(*slave, I_PUSH, \"ldterm\");\n\tioctl(*slave, I_PUSH, \"pckt\");\n\nwhere I_PUSH is ('S' << 8) | 2. I am terrified of ioctl number conflicts\nand do not want to try that, so it is nice to see that this fails so\ngracefully.\n\nThanks again. Here’s a revised patch.\n\n .gitignore | 1 +\n Makefile | 11 +++++++++\n t/t7006-pager.sh | 28 +++++++++++++++--------\n test-terminal.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 92 insertions(+), 10 deletions(-)\n create mode 100644 test-terminal.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 8df8f88..6b405ba 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -169,6 +169,7 @@\n /test-run-command\n /test-sha1\n /test-sigchain\n+/test-terminal\n /common-cmds.h\n *.tar.gz\n *.dsc\ndiff --git a/Makefile b/Makefile\nindex 7bf2fca..0df3795 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -55,6 +55,9 @@ all::\n #\n # Define NO_UNSETENV if you don't have unsetenv in the C library.\n #\n+# Define NO_GRANTPT if you don't have grantpt, unlockpt, and ptsname\n+# (for tests).\n+#\n # Define NO_MKDTEMP if you don't have mkdtemp in the C library.\n #\n # Define NO_MKSTEMPS if you don't have mkstemps in the C library.\n@@ -803,6 +806,7 @@ ifeq ($(uname_O),Cygwin)\n \tNO_TRUSTABLE_FILEMODE = UnfortunatelyYes\n \tOLD_ICONV = UnfortunatelyYes\n \tNO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease\n+\tNO_GRANTPT = YesPlease\n \t# There are conflicting reports about this.\n \t# On some boxes NO_MMAP is needed, and not so elsewhere.\n \t# Try commenting this out if you suspect MMAP is more efficient\n@@ -910,6 +914,7 @@ ifeq ($(uname_S),HP-UX)\n \tNO_UNSETENV = YesPlease\n \tNO_HSTRERROR = YesPlease\n \tNO_SYS_SELECT_H = YesPlease\n+\tNO_GRANTPT = YesPlease\n \tSNPRINTF_RETURNS_BOGUS = YesPlease\n endif\n ifeq ($(uname_S),Windows)\n@@ -937,6 +942,7 @@ ifeq ($(uname_S),Windows)\n \tNO_PERL_MAKEMAKER = YesPlease\n \tRUNTIME_PREFIX = YesPlease\n \tNO_POSIX_ONLY_PROGRAMS = YesPlease\n+\tNO_GRANTPT = YesPlease\n \tNO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease\n \tNO_NSEC = YesPlease\n \tUSE_WIN32_MMAP = YesPlease\n@@ -989,6 +995,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_PERL_MAKEMAKER = YesPlease\n \tRUNTIME_PREFIX = YesPlease\n \tNO_POSIX_ONLY_PROGRAMS = YesPlease\n+\tNO_GRANTPT = YesPlease\n \tNO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease\n \tNO_NSEC = YesPlease\n \tUSE_WIN32_MMAP = YesPlease\n@@ -1727,6 +1734,10 @@ TEST_PROGRAMS_NEED_X += test-sha1\n TEST_PROGRAMS_NEED_X += test-sigchain\n TEST_PROGRAMS_NEED_X += test-index-version\n \n+ifndef NO_GRANTPT\n+TEST_PROGRAMS_NEED_X += test-terminal\n+endif\n+\n TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))\n \n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 2e9cb9d..21788e3 100644\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -5,18 +5,26 @@ test_description='Test automatic use of a pager.'\n . ./test-lib.sh\n \n rm -f stdout_is_tty\n-test_expect_success 'is stdout a terminal?' '\n+test_expect_success 'set up terminal for tests' '\n \tif test -t 1\n \tthen\n \t\t: > stdout_is_tty\n+\telif test-terminal sh -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() { test-terminal \"$@\"; }\n \ttest_set_prereq TTY\n else\n-\tsay stdout is not a terminal, so skipping some tests.\n+\tsay no usable terminal, so skipping some tests\n fi\n \n unset GIT_PAGER GIT_PAGER_IN_USE\n@@ -30,13 +38,13 @@ test_expect_success 'setup' '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands use a pager' '\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e paginated.out\n '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands do not use a pager' '\n-\tgit rev-list HEAD &&\n+\ttest_terminal git rev-list HEAD &&\n \t! test -e paginated.out\n '\n \n@@ -54,7 +62,7 @@ test_expect_success 'no pager when stdout is a regular file' '\n \n rm -f paginated.out\n test_expect_success TTY 'git --paginate rev-list uses a pager' '\n-\tgit --paginate rev-list HEAD &&\n+\ttest_terminal git --paginate rev-list HEAD &&\n \ttest -e paginated.out\n '\n \n@@ -66,7 +74,7 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '\n \n rm -f paginated.out\n test_expect_success TTY 'no pager with --no-pager' '\n-\tgit --no-pager log &&\n+\ttest_terminal git --no-pager log &&\n \t! test -e paginated.out\n '\n \n@@ -127,7 +135,7 @@ test_expect_success SIMPLEPAGER 'default pager is used by default' '\n \t: > default_pager_used\n \tEOF\n \tchmod +x $less &&\n-\tPATH=.:$PATH git log &&\n+\tPATH=.:$PATH test_terminal git log &&\n \ttest -e default_pager_used\n '\n \n@@ -137,7 +145,7 @@ rm -f PAGER_used\n test_expect_success TTY 'PAGER overrides default pager' '\n \tPAGER=\": > PAGER_used\" &&\n \texport PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e PAGER_used\n '\n \n@@ -147,7 +155,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '\n \tPAGER=: &&\n \texport PAGER &&\n \tgit config core.pager \": > core.pager_used\" &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e core.pager_used\n '\n \n@@ -156,7 +164,7 @@ test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n \tgit config core.pager : &&\n \tGIT_PAGER=\": > GIT_PAGER_used\" &&\n \texport GIT_PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e GIT_PAGER_used\n '\n \ndiff --git a/test-terminal.c b/test-terminal.c\nnew file mode 100644\nindex 0000000..6408a7d\n--- /dev/null\n+++ b/test-terminal.c\n@@ -0,0 +1,62 @@\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+\n+static void newtty(int *master, int *slave)\n+{\n+\tint fd;\n+\tconst char *term;\n+\n+\tfd = open(\"/dev/ptmx\", O_RDWR|O_NOCTTY);\n+\tif (fd == -1 || grantpt(fd) || unlockpt(fd) || !(term = ptsname(fd)))\n+\t\tdie_errno(\"Could not allocate a new pseudo-terminal\");\n+\t*master = fd;\n+\t*slave = open(term, O_RDWR|O_NOCTTY);\n+\tif (*slave == -1)\n+\t\tdie_errno(\"Could not open pseudo-terminal: %s\", term);\n+}\n+\n+static void setup_child(struct child_process *chld,\n+\t\t\tconst char **args, int out)\n+{\n+\tmemset(chld, 0, sizeof(*chld));\n+\tchld->argv = args;\n+\tchld->out = out;\n+}\n+\n+static void xsendfile(int out_fd, int in_fd)\n+{\n+\tchar buf[BUFSIZ];\n+\n+\t/* Note: the real sendfile() cannot read from a terminal. */\n+\tfor (;;) {\n+\t\tssize_t n = xread(in_fd, buf, sizeof(buf));\n+\n+\t\t/*\n+\t\t * It is unspecified by POSIX whether reads\n+\t\t * from a disconnected terminal will return\n+\t\t * EIO (as in AIX 4.x, IRIX, and Linux) or\n+\t\t * end-of-file. Either is fine.\n+\t\t */\n+\t\tif (n == 0 || (n == -1 && errno == EIO))\n+\t\t\tbreak;\n+\t\tif (n == -1)\n+\t\t\tdie_errno(\"cannot read from child\");\n+\t\twrite_or_die(out_fd, buf, n);\n+\t}\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tint masterfd, slavefd;\n+\tstruct child_process chld;\n+\n+\tif (argc < 2 || (argc == 2 && !strcmp(argv[1], \"-h\")))\n+\t\tusage(\"test-terminal program args\");\n+\n+\tnewtty(&masterfd, &slavefd);\n+\tsetup_child(&chld, argv + 1, slavefd);\n+\tif (start_command(&chld))\n+\t\treturn error(\"failed to execute child\");\n+\txsendfile(1, masterfd);\n+\treturn finish_command(&chld);\n+}\n-- \n1.7.0\n"},{"id":"135145","messageId":"7vaav42ya1.fsf@alter.siamese.dyndns.org","threadId":"22723","inReplyTo":"20100220052504.GA24697@progeny.tock","subject":"Re: [PATCH v2 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-20T06:53:42Z","receivedAt":"2010-02-20T06:53:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> - Create a new pseudo-terminal on the fly and capture its output.\n>\n> This patch implements the third approach.\n\nYuck. You must be having too much fun ;-).\n\n> On non-Unixlike and pre-SVR4 systems, this functionality should be\n> disabled by passing NO_GRANTPT=YesPlease to the makefile. I do not\n> know whether Cygwin or HP-UX implements the required functions\n> appropriately, so to be safe this patch includes that option for them.\n>\n> On platforms where test-terminal does not work, the test script will\n> maintain its old behavior (skipping most of its tests unless\n> GIT_TEST_OPTS includes --verbose).\n\nIf we are truly serious about doing this, it might be worthwhile to check\nhow expect (http://expect.nist.gov) does this portably.\n"},{"id":"135147","messageId":"20100220085025.GA26788@progeny.tock","threadId":"22723","inReplyTo":"7vaav42ya1.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-20T08:50:25Z","receivedAt":"2010-02-20T08:50:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Testing pagination requires (fake or real) access to a terminal so we\ncan see whether the pagination automatically kicks in, which makes it\nhard to get good coverage when running tests without --verbose. There\nare a number of ways to work around that:\n\n - Replace all isatty calls with calls to a custom xisatty wrapper\n that usually checks for a terminal but can be overridden for tests.\n This would be workable, but it would require implementing xisatty\n separately in three languages (C, shell, and perl) and making sure\n that any code that is to be tested always uses the wrapper.\n\n - Redirect stdout to /dev/tty. This would be problematic because\n there might be no terminal available, and even if a terminal is\n available, it might not be appropriate to spew output to it.\n\n - Create a new pseudo-terminal on the fly and capture its output.\n\nThis patch implements the third approach.\n\nThe new test-terminal.perl helper uses IO::Pty from Expect.pm to create\na terminal and executes the program specified by its arguments with\nthat terminal as stdout. If the IO::Pty module is missing or not\nworking on a system, the test script will maintain its old behavior\n(skipping most of its tests unless GIT_TEST_OPTS includes --verbose).\n\nSigned-off-by: Jonathan Nieder \n---\nJunio C Hamano wrote:\n> Jonathan Nieder writes:\n\n> > On non-Unixlike and pre-SVR4 systems, this functionality should be\n> > disabled by passing NO_GRANTPT=YesPlease to the makefile. I do not\n> > know whether Cygwin or HP-UX implements the required functions\n> > appropriately, so to be safe this patch includes that option for them.\n> >\n> > On platforms where test-terminal does not work, the test script will\n> > maintain its old behavior (skipping most of its tests unless\n> > GIT_TEST_OPTS includes --verbose).\n> \n> If we are truly serious about doing this, it might be worthwhile to check\n> how expect (http://expect.nist.gov) does this portably.\n\nI was not interested in making this work perfectly on ancient systems,\nbecause that would involve providing variants for old-style BSD ptys,\nSTREAMS-based ptys, and the modern simpler ptys. Looking at the expect\nsource, it seems there are even more variants to worry about than I had\nthought.\n\nOn the other hand, I do think it would be better to make this work (or\nnot work) without makefile changes on all platforms. So how about this?\nIt lets the IO::Pty library used to implement Expect.pm do the dirty\nwork for us.\n\nThe main downsides are that this means less coverage (not everyone has\nIO::Pty installed) and it is much slower than the C version.\n\n .gitignore | 1 +\n t/t7006-pager.sh | 33 ++++++++++++++------\n t/t7006/test-terminal.perl | 70 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 94 insertions(+), 10 deletions(-)\n create mode 100755 t/t7006/test-terminal.perl\n\ndiff --git a/.gitignore b/.gitignore\nindex 8df8f88..6b405ba 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -169,6 +169,7 @@\n /test-run-command\n /test-sha1\n /test-sigchain\n+/test-terminal\n /common-cmds.h\n *.tar.gz\n *.dsc\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 2e9cb9d..da0f962 100644\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -5,18 +5,31 @@ test_description='Test automatic use of a pager.'\n . ./test-lib.sh\n \n rm -f stdout_is_tty\n-test_expect_success 'is stdout a terminal?' '\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\"/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 stdout is not a terminal, so skipping some tests.\n+\tsay no usable terminal, so skipping some tests\n fi\n \n unset GIT_PAGER GIT_PAGER_IN_USE\n@@ -30,13 +43,13 @@ test_expect_success 'setup' '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands use a pager' '\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e paginated.out\n '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands do not use a pager' '\n-\tgit rev-list HEAD &&\n+\ttest_terminal git rev-list HEAD &&\n \t! test -e paginated.out\n '\n \n@@ -54,7 +67,7 @@ test_expect_success 'no pager when stdout is a regular file' '\n \n rm -f paginated.out\n test_expect_success TTY 'git --paginate rev-list uses a pager' '\n-\tgit --paginate rev-list HEAD &&\n+\ttest_terminal git --paginate rev-list HEAD &&\n \ttest -e paginated.out\n '\n \n@@ -66,7 +79,7 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '\n \n rm -f paginated.out\n test_expect_success TTY 'no pager with --no-pager' '\n-\tgit --no-pager log &&\n+\ttest_terminal git --no-pager log &&\n \t! test -e paginated.out\n '\n \n@@ -127,7 +140,7 @@ test_expect_success SIMPLEPAGER 'default pager is used by default' '\n \t: > default_pager_used\n \tEOF\n \tchmod +x $less &&\n-\tPATH=.:$PATH git log &&\n+\tPATH=.:$PATH test_terminal git log &&\n \ttest -e default_pager_used\n '\n \n@@ -137,7 +150,7 @@ rm -f PAGER_used\n test_expect_success TTY 'PAGER overrides default pager' '\n \tPAGER=\": > PAGER_used\" &&\n \texport PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e PAGER_used\n '\n \n@@ -147,7 +160,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '\n \tPAGER=: &&\n \texport PAGER &&\n \tgit config core.pager \": > core.pager_used\" &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e core.pager_used\n '\n \n@@ -156,7 +169,7 @@ test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n \tgit config core.pager : &&\n \tGIT_PAGER=\": > GIT_PAGER_used\" &&\n \texport GIT_PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e GIT_PAGER_used\n '\n \ndiff --git a/t/t7006/test-terminal.perl b/t/t7006/test-terminal.perl\nnew file mode 100755\nindex 0000000..b51cfc6\n--- /dev/null\n+++ b/t/t7006/test-terminal.perl\n@@ -0,0 +1,70 @@\n+#!/usr/bin/perl\n+use strict;\n+use warnings;\n+use IO::Pty;\n+\n+# Fork and execute @$argv 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+\tmy $buf;\n+\n+\t# Note: the real sendfile() cannot read from a terminal.\n+\tfor (;;) {\n+\t\tmy $n = sysread $in, $buf, 4096;\n+\n+\t\t# It is unspecified by POSIX whether reads\n+\t\t# from a disconnected terminal will return\n+\t\t# EIO (as in AIX 4.x, IRIX, and Linux) or\n+\t\t# end-of-file. Either is fine.\n+\t\tif (!defined($n) && $!{EIO}) {\n+\t\t\treturn;\n+\t\t} elsif (!defined($n)) {\n+\t\t\tdie \"cannot read from child: $!\";\n+\t\t} elsif ($n == 0) {\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tprint $out $buf or die \"write error: $!\";\n+\t\t}\n+\t}\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.0\n"},{"id":"135150","messageId":"20100220094820.GA32456@progeny.tock","threadId":"22723","inReplyTo":"20100220085025.GA26788@progeny.tock","subject":"[PATCH squash] Simplify test-terminal.perl","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-20T09:48:20Z","receivedAt":"2010-02-20T09:48:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The File::Copy module is included with perl (since version 5.002,\n1995), so this simplification comes for free. It does not make the\ntest noticeably faster or slower.\n\nSigned-off-by: Jonathan Nieder \n---\nJonathan Nieder wrote:\n> it is much slower than the C version.\n\nI take that back. The perl version takes roughly 3.7 s and the\nC version 3.4 s here (both with hot cache). Noticeable alone, but a\ntiny blip in the context of a full test run.\n\nSorry to mislead.\n\n t/t7006/test-terminal.perl | 26 +++++++-------------------\n 1 files changed, 7 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t7006/test-terminal.perl b/t/t7006/test-terminal.perl\nindex b51cfc6..73ff809 100755\n--- a/t/t7006/test-terminal.perl\n+++ b/t/t7006/test-terminal.perl\n@@ -2,8 +2,9 @@\n use strict;\n use warnings;\n use IO::Pty;\n+use File::Copy;\n \n-# Fork and execute @$argv with stdout redirected to $out.\n+# Run @$argv in the background with stdout redirected to $out.\n sub start_child {\n \tmy ($argv, $out) = @_;\n \tmy $pid = fork;\n@@ -37,26 +38,13 @@ sub finish_child {\n sub xsendfile {\n \tmy ($out, $in) = @_;\n \n-\tmy $buf;\n-\n \t# Note: the real sendfile() cannot read from a terminal.\n-\tfor (;;) {\n-\t\tmy $n = sysread $in, $buf, 4096;\n \n-\t\t# It is unspecified by POSIX whether reads\n-\t\t# from a disconnected terminal will return\n-\t\t# EIO (as in AIX 4.x, IRIX, and Linux) or\n-\t\t# end-of-file. Either is fine.\n-\t\tif (!defined($n) && $!{EIO}) {\n-\t\t\treturn;\n-\t\t} elsif (!defined($n)) {\n-\t\t\tdie \"cannot read from child: $!\";\n-\t\t} elsif ($n == 0) {\n-\t\t\treturn;\n-\t\t} else {\n-\t\t\tprint $out $buf or die \"write error: $!\";\n-\t\t}\n-\t}\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-- \n1.7.0\n"},{"id":"135167","messageId":"7v8wanq0c3.fsf@alter.siamese.dyndns.org","threadId":"22723","inReplyTo":"20100219071857.GF29916@progeny.tock","subject":"Re: [PATCH 6/7] tests: Add tests for automatic use of pager","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-20T17:33:00Z","receivedAt":"2010-02-20T17:33:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\n> new file mode 100644\n> index 0000000..2e9cb9d\n> --- /dev/null\n> +++ b/t/t7006-pager.sh\n> @@ -0,0 +1,163 @@\n> ...\n> +rm -f paginated.out\n> +git config color.ui auto\n> +test_expect_success TTY 'color when writing to a pager' '\n> +\tTERM=vt100 test_terminal git log &&\n> +\tcolorful paginated.out\n> +'\n\nI didn't see test_terminal defined up to this point. Am I missing\nsomething?\n"},{"id":"135206","messageId":"20100221020317.GA7651@progeny.tock","threadId":"22723","inReplyTo":"7v8wanq0c3.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2 6/7] tests: Add tests for automatic use of pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-21T02:03:18Z","receivedAt":"2010-02-21T02:03:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Git’s automatic pagination support has some subtleties. Add some\ntests to make sure we don’t break:\n\n - when git will use a pager by default;\n - the effect of the --paginate and --no-pager options;\n - the effect of pagination on use of color;\n - how the choice of pager is configured.\n\nThis does not yet test:\n\n - use of the pager by scripted commands (git svn and git am);\n - effect of the pager.* configuration variables;\n - setting of the LESS variable.\n\nSome features involve checking whether stdout is a terminal, so many\nof these tests are skipped unless output is passed through to the\nterminal (i.e., unless $GIT_TEST_OPTS includes --verbose).\n\nThe immediate purpose for these tests was to avoid making things worse\nafter the breakage from my jn/editor-pager series (see commit 376f39,\n2009-11-20). Thanks to Sebastian Celis \nfor the report.\n\nHelped-by: Jeff King \nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\nOn Sat, Feb 20, 2010 at 09:33:00AM -0800, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> +\tTERM=vt100 test_terminal git log &&\n> \n> I didn't see test_terminal defined up to this point. Am I missing\n> something?\n\nGood catch, thanks.\n\n t/t7006-pager.sh | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 163 insertions(+), 0 deletions(-)\n create mode 100644 t/t7006-pager.sh\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nnew file mode 100644\nindex 0000000..4f52ea5\n--- /dev/null\n+++ b/t/t7006-pager.sh\n@@ -0,0 +1,163 @@\n+#!/bin/sh\n+\n+test_description='Test automatic use of a pager.'\n+\n+. ./test-lib.sh\n+\n+rm -f stdout_is_tty\n+test_expect_success 'is stdout a terminal?' '\n+\tif test -t 1\n+\tthen\n+\t\t: > stdout_is_tty\n+\tfi\n+'\n+\n+if test -e stdout_is_tty\n+then\n+\ttest_set_prereq TTY\n+else\n+\tsay stdout is not a terminal, so skipping some tests.\n+fi\n+\n+unset GIT_PAGER GIT_PAGER_IN_USE\n+git config --unset core.pager\n+PAGER='cat > paginated.out'\n+export PAGER\n+\n+test_expect_success 'setup' '\n+\ttest_commit initial\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'some commands use a pager' '\n+\tgit log &&\n+\ttest -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'some commands do not use a pager' '\n+\tgit rev-list HEAD &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success 'no pager when stdout is a pipe' '\n+\tgit log | cat &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success 'no pager when stdout is a regular file' '\n+\tgit log > file &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'git --paginate rev-list uses a pager' '\n+\tgit --paginate rev-list HEAD &&\n+\ttest -e paginated.out\n+'\n+\n+rm -f file paginated.out\n+test_expect_success 'no pager even with --paginate when stdout is a pipe' '\n+\tgit --paginate log | cat &&\n+\t! test -e paginated.out\n+'\n+\n+rm -f paginated.out\n+test_expect_success TTY 'no pager with --no-pager' '\n+\tgit --no-pager log &&\n+\t! test -e paginated.out\n+'\n+\n+# A colored commit log will begin with an appropriate ANSI escape\n+# for the first color; the text \"commit\" comes later.\n+colorful() {\n+\tread firstline < $1\n+\t! expr \"$firstline\" : \"^[a-zA-Z]\" >/dev/null\n+}\n+\n+rm -f colorful.log colorless.log\n+test_expect_success 'tests can detect color' '\n+\tgit log --no-color > colorless.log &&\n+\tgit log --color > colorful.log &&\n+\t! colorful colorless.log &&\n+\tcolorful colorful.log\n+'\n+\n+rm -f colorless.log\n+git config color.ui auto\n+test_expect_success 'no color when stdout is a regular file' '\n+\tgit log > colorless.log &&\n+\t! colorful colorless.log\n+'\n+\n+rm -f paginated.out\n+git config color.ui auto\n+test_expect_success TTY 'color when writing to a pager' '\n+\tTERM=vt100 git log &&\n+\tcolorful paginated.out\n+'\n+\n+rm -f colorful.log\n+git config color.ui auto\n+test_expect_success 'color when writing to a file intended for a pager' '\n+\tTERM=vt100 GIT_PAGER_IN_USE=true git log > colorful.log &&\n+\tcolorful colorful.log\n+'\n+\n+unset PAGER GIT_PAGER\n+git config --unset core.pager\n+test_expect_success 'determine default pager' '\n+\tless=$(git var GIT_PAGER) &&\n+\ttest -n \"$less\"\n+'\n+\n+if expr \"$less\" : '^[a-z]*$' > /dev/null && test_have_prereq TTY\n+then\n+\ttest_set_prereq SIMPLEPAGER\n+fi\n+\n+unset PAGER GIT_PAGER\n+git config --unset core.pager\n+rm -f default_pager_used\n+test_expect_success SIMPLEPAGER 'default pager is used by default' '\n+\tcat > $less <<-EOF &&\n+\t#!$SHELL_PATH\n+\t: > default_pager_used\n+\tEOF\n+\tchmod +x $less &&\n+\tPATH=.:$PATH git log &&\n+\ttest -e default_pager_used\n+'\n+\n+unset GIT_PAGER\n+git config --unset core.pager\n+rm -f PAGER_used\n+test_expect_success TTY 'PAGER overrides default pager' '\n+\tPAGER=\": > PAGER_used\" &&\n+\texport PAGER &&\n+\tgit log &&\n+\ttest -e PAGER_used\n+'\n+\n+unset GIT_PAGER\n+rm -f core.pager_used\n+test_expect_success TTY 'core.pager overrides PAGER' '\n+\tPAGER=: &&\n+\texport PAGER &&\n+\tgit config core.pager \": > core.pager_used\" &&\n+\tgit log &&\n+\ttest -e core.pager_used\n+'\n+\n+rm -f GIT_PAGER_used\n+test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n+\tgit config core.pager : &&\n+\tGIT_PAGER=\": > GIT_PAGER_used\" &&\n+\texport GIT_PAGER &&\n+\tgit log &&\n+\ttest -e GIT_PAGER_used\n+'\n+\n+test_done\n-- \n1.7.0\n"},{"id":"135207","messageId":"20100221020922.GB7651@progeny.tock","threadId":"22723","inReplyTo":"20100221020317.GA7651@progeny.tock","subject":"[PATCH v4 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-21T02:09:22Z","receivedAt":"2010-02-21T02:09:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Testing pagination requires (fake or real) access to a terminal so we\ncan see whether the pagination automatically kicks in, which makes it\nhard to get good coverage when running tests without --verbose. There\nare a number of ways to work around that:\n\n - Replace all isatty calls with calls to a custom xisatty wrapper\n that usually checks for a terminal but can be overridden for tests.\n This would be workable, but it would require implementing xisatty\n separately in three languages (C, shell, and perl) and making sure\n that any code that is to be tested always uses the wrapper.\n\n - Redirect stdout to /dev/tty. This would be problematic because\n there might be no terminal available, and even if a terminal is\n available, it might not be appropriate to spew output to it.\n\n - Create a new pseudo-terminal on the fly and capture its output.\n\nThis patch implements the third approach.\n\nThe new test-terminal.perl helper uses IPC::Open3 and IO::Pty from\nExpect.pm to create a terminal and execute the program specified by\nits arguments with that terminal as stdout. If either module is\nmissing or not working on a system, the test script will maintain its\nold behavior (skipping most of its tests unless GIT_TEST_OPTS includes\n--verbose).\n\nIt also uses File::Copy, which has been part of core Perl since\nversion 5.002 (1996).\n\nSigned-off-by: Jonathan Nieder \n---\nHere’s test-terminal on top of that. As long as this using IO::Pty\nfrom CPAN, there’s not much reason not to use the often-used\nIPC::Open3 to simplify this.\n\nThe nice thing about using a script for this is that it fails\ngracefully and doesn’t affect the makefile.\n\nThanks for bearing with me.\n\n t/t7006-pager.sh | 35 ++++++++++++++++++++++++-----------\n t/t7006/test-terminal.perl | 28 ++++++++++++++++++++++++++++\n 2 files changed, 52 insertions(+), 11 deletions(-)\n create mode 100755 t/t7006/test-terminal.perl\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 4f52ea5..da0f962 100644\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -5,18 +5,31 @@ test_description='Test automatic use of a pager.'\n . ./test-lib.sh\n \n rm -f stdout_is_tty\n-test_expect_success 'is stdout a terminal?' '\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\"/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 stdout is not a terminal, so skipping some tests.\n+\tsay no usable terminal, so skipping some tests\n fi\n \n unset GIT_PAGER GIT_PAGER_IN_USE\n@@ -30,13 +43,13 @@ test_expect_success 'setup' '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands use a pager' '\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e paginated.out\n '\n \n rm -f paginated.out\n test_expect_success TTY 'some commands do not use a pager' '\n-\tgit rev-list HEAD &&\n+\ttest_terminal git rev-list HEAD &&\n \t! test -e paginated.out\n '\n \n@@ -54,7 +67,7 @@ test_expect_success 'no pager when stdout is a regular file' '\n \n rm -f paginated.out\n test_expect_success TTY 'git --paginate rev-list uses a pager' '\n-\tgit --paginate rev-list HEAD &&\n+\ttest_terminal git --paginate rev-list HEAD &&\n \ttest -e paginated.out\n '\n \n@@ -66,7 +79,7 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '\n \n rm -f paginated.out\n test_expect_success TTY 'no pager with --no-pager' '\n-\tgit --no-pager log &&\n+\ttest_terminal git --no-pager log &&\n \t! test -e paginated.out\n '\n \n@@ -95,7 +108,7 @@ test_expect_success 'no color when stdout is a regular file' '\n rm -f paginated.out\n git config color.ui auto\n test_expect_success TTY 'color when writing to a pager' '\n-\tTERM=vt100 git log &&\n+\tTERM=vt100 test_terminal git log &&\n \tcolorful paginated.out\n '\n \n@@ -127,7 +140,7 @@ test_expect_success SIMPLEPAGER 'default pager is used by default' '\n \t: > default_pager_used\n \tEOF\n \tchmod +x $less &&\n-\tPATH=.:$PATH git log &&\n+\tPATH=.:$PATH test_terminal git log &&\n \ttest -e default_pager_used\n '\n \n@@ -137,7 +150,7 @@ rm -f PAGER_used\n test_expect_success TTY 'PAGER overrides default pager' '\n \tPAGER=\": > PAGER_used\" &&\n \texport PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e PAGER_used\n '\n \n@@ -147,7 +160,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '\n \tPAGER=: &&\n \texport PAGER &&\n \tgit config core.pager \": > core.pager_used\" &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e core.pager_used\n '\n \n@@ -156,7 +169,7 @@ test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n \tgit config core.pager : &&\n \tGIT_PAGER=\": > GIT_PAGER_used\" &&\n \texport GIT_PAGER &&\n-\tgit log &&\n+\ttest_terminal git log &&\n \ttest -e GIT_PAGER_used\n '\n \ndiff --git a/t/t7006/test-terminal.perl b/t/t7006/test-terminal.perl\nnew file mode 100755\nindex 0000000..fc3fbb4\n--- /dev/null\n+++ b/t/t7006/test-terminal.perl\n@@ -0,0 +1,28 @@\n+#!/usr/bin/perl\n+use strict;\n+use warnings;\n+use IO::Pty;\n+use File::Copy;\n+use IPC::Open3;\n+\n+# Wait for $_[0] to finish.\n+sub finish_child {\n+\t# Simplified from wait_or_whine() in run-command.c.\n+\twaitpid($_[0], 0) == $_[0] or die \"waitpid failed: $!\";\n+\tif ($? & 127) {\n+\t\tmy $code = $? & 127;\n+\t\twarn \"died of signal $code\";\n+\t\treturn $code - 128;\n+\t}\n+\treturn $? >> 8;\n+}\n+\n+@ARGV >= 1 or die \"usage: test-terminal program args\";\n+my $master = new IO::Pty;\n+my $slave = $master->slave;\n+my $pid = open3(\\*STDIN, '>&' . fileno($slave), \\*STDERR, @ARGV);\n+close $slave;\n+# Reads from a disconnected terminal may return EIO or end-of-file.\n+# Either is fine.\n+copy($master, \\*STDOUT, 4096) or $!{EIO} or die \"cannot copy from child: $!\";\n+exit(finish_child($pid));\n-- \n1.7.0\n"},{"id":"135227","messageId":"20100221073058.GC2840@coredump.intra.peff.net","threadId":"22723","inReplyTo":"20100221020922.GB7651@progeny.tock","subject":"Re: [PATCH v4 7/7] t7006-pager: if stdout is not a terminal, make a new one","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-21T07:30:59Z","receivedAt":"2010-02-21T07:30:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 20, 2010 at 08:09:22PM -0600, Jonathan Nieder wrote:\n\n> Testing pagination requires (fake or real) access to a terminal so we\n> can see whether the pagination automatically kicks in, which makes it\n> hard to get good coverage when running tests without --verbose. There\n> are a number of ways to work around that:\n> \n> - Replace all isatty calls with calls to a custom xisatty wrapper\n> that usually checks for a terminal but can be overridden for tests.\n> This would be workable, but it would require implementing xisatty\n> separately in three languages (C, shell, and perl) and making sure\n> that any code that is to be tested always uses the wrapper.\n> \n> - Redirect stdout to /dev/tty. This would be problematic because\n> there might be no terminal available, and even if a terminal is\n> available, it might not be appropriate to spew output to it.\n> \n> - Create a new pseudo-terminal on the fly and capture its output.\n> \n> This patch implements the third approach.\n\nJust to wrap up my end of this patch discussion, I think the approach\nyou take here is the sanest one. While it would be nice to get test\ncoverage on every system, I don't think it is worth the effort of\ntrying to write portable terminal creation code. And this way at least\nthe code in git is fairly minimal.\n\nSo looks good to me.\n\n-Peff\n"},{"id":"135275","messageId":"4B823E21.1060707@viscovery.net","threadId":"22723","inReplyTo":"20100221020317.GA7651@progeny.tock","subject":"Re: [PATCH v2 6/7] tests: Add tests for automatic use of pager","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-02-22T08:19:45Z","receivedAt":"2010-02-22T08:19:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Don't the following pager settings suffer from a race condition?\n\n> +\tcat > $less <<-EOF &&\n> +\t#!$SHELL_PATH\n> +\t: > default_pager_used\n> +\tEOF\n\n> +\tPAGER=\": > PAGER_used\" &&\n\n> +\tPAGER=: &&\n\n> +\tgit config core.pager \": > core.pager_used\" &&\n\n> +\tgit config core.pager : &&\n> +\tGIT_PAGER=\": > GIT_PAGER_used\" &&\n\nThey depend on that the subsequent 'git log' writes to the pipe (and does\nnot fill it) before the pager can run. If the pager runs first and\ncompletes, and then 'git log' writes to the pipe, it will die from\nSIGPIPE, won't it?\n\nI suggest to rewrite all ':' to something that processes stdin, eg. 'wc'\n(not 'cat', because this is a magic token).\n\n-- Hannes\n"},{"id":"135282","messageId":"20100222084633.GA14432@progeny.tock","threadId":"22723","inReplyTo":"4B823E21.1060707@viscovery.net","subject":"[PATCH 8/7] tests: Fix race condition in t7006-pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-22T08:46:33Z","receivedAt":"2010-02-22T08:46:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Pagers that do not consume their input are dangerous: for example,\n\n $ GIT_PAGER=: git log\n $ echo $?\n 141\n $\n\nThe only reason these tests were able to work before was that\n'git log' would write to the pipe (and not fill it) before the\npager had time to terminate and close the pipe.\n\nFix it by using a program that consumes its input, namely wc (as\nsuggested by Johannes).\n\nReported-by: Johannes Sixt \nSigned-off-by: Jonathan Nieder \n---\nJohannes Sixt wrote:\n\n> Don't the following pager settings suffer from a race condition?\n\nYes. Thanks for noticing.\n\n t/t7006-pager.sh | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex da0f962..ec6fd06 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -137,7 +137,7 @@ rm -f default_pager_used\n test_expect_success SIMPLEPAGER 'default pager is used by default' '\n \tcat > $less <<-EOF &&\n \t#!$SHELL_PATH\n-\t: > default_pager_used\n+\twc > default_pager_used\n \tEOF\n \tchmod +x $less &&\n \tPATH=.:$PATH test_terminal git log &&\n@@ -148,7 +148,7 @@ unset GIT_PAGER\n git config --unset core.pager\n rm -f PAGER_used\n test_expect_success TTY 'PAGER overrides default pager' '\n-\tPAGER=\": > PAGER_used\" &&\n+\tPAGER=\"wc > PAGER_used\" &&\n \texport PAGER &&\n \ttest_terminal git log &&\n \ttest -e PAGER_used\n@@ -159,7 +159,7 @@ rm -f core.pager_used\n test_expect_success TTY 'core.pager overrides PAGER' '\n \tPAGER=: &&\n \texport PAGER &&\n-\tgit config core.pager \": > core.pager_used\" &&\n+\tgit config core.pager \"wc > core.pager_used\" &&\n \ttest_terminal git log &&\n \ttest -e core.pager_used\n '\n@@ -167,7 +167,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '\n rm -f GIT_PAGER_used\n test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n \tgit config core.pager : &&\n-\tGIT_PAGER=\": > GIT_PAGER_used\" &&\n+\tGIT_PAGER=\"wc > GIT_PAGER_used\" &&\n \texport GIT_PAGER &&\n \ttest_terminal git log &&\n \ttest -e GIT_PAGER_used\n-- \n1.7.0\n"},{"id":"135281","messageId":"20100222091230.GA16490@progeny.tock","threadId":"22723","inReplyTo":"20100222084633.GA14432@progeny.tock","subject":"Re: [PATCH 8/7] tests: Fix race condition in t7006-pager","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-22T09:12:30Z","receivedAt":"2010-02-22T09:12:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Johannes Sixt wrote:\n> \n> > Don't the following pager settings suffer from a race condition?\n> \n> Yes. Thanks for noticing.\n\nAnd to save the reader time wondering: yes, the following examples\nsuffer from the same race, but if they are used then we _want_ the\ntest to fail. It might make sense to squash them in anyway to make\nthe patch more self-explanatory.\n\nThere is also an instance of PAGER='cat > paginated.out'; since the\npager string only starts with but does not equal \"cat\", this works\nfine. It needed for test ‘12: color when writing to a pager’ to be\nable to read the paginated output.\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex ec6fd06..d9202d5 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -157,7 +157,7 @@ test_expect_success TTY 'PAGER overrides default pager' '\n unset GIT_PAGER\n rm -f core.pager_used\n test_expect_success TTY 'core.pager overrides PAGER' '\n-\tPAGER=: &&\n+\tPAGER=wc &&\n \texport PAGER &&\n \tgit config core.pager \"wc > core.pager_used\" &&\n \ttest_terminal git log &&\n@@ -166,7 +166,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '\n \n rm -f GIT_PAGER_used\n test_expect_success TTY 'GIT_PAGER overrides core.pager' '\n-\tgit config core.pager : &&\n+\tgit config core.pager wc &&\n \tGIT_PAGER=\"wc > GIT_PAGER_used\" &&\n \texport GIT_PAGER &&\n \ttest_terminal git log &&\n"}]}