On 11/11/19 4:28 PM, Mark Dilger wrote:
>
>
>>>>>
>>>>
>>>> On further consideration, I'm wondering why we don't just
>>>> unconditionally use a closed input pty for all these functions (except
>>>> run_log). None of them use any input, and making the client worry
>>>> about
>>>> whether or not to close it seems something of an abstraction break.
>>>> There would be no API change at all involved in this case, just a
>>>> bit of
>>>> extra cleanliness. Would need testing on Windows, I'll go and do that
>>>> now.
>>>>
>>>>
>>>> Thoughts?
>>>
>>> That sounds a lot better than your previous patch.
>>>
>>> PostgresNode.pm and TestLib.pm both invoke IPC::Run::run. If you
>>> change all the invocations in TestLib to close input pty, should you
>>> do the same for PostgresNode? I don't have a strong argument for
>>> doing so, but it seems cleaner to have both libraries invoking
>>> commands under identical conditions, such that if commands were
>>> borrowed from one library and called from the other they would behave
>>> the same.
>>>
>>> PostgresNode already uses TestLib, so perhaps setting up the
>>> environment can be abstracted into something, perhaps TestLib::run,
>>> and then used everywhere that IPC::Run::run currently is used.
>>
>>
>>
>> I don't think we need to do that. In the case of the PostgresNode.pm
>> uses we know what the executable is, unlike the the TestLib.pm cases.
>> They are our own executables and we don't expect them to be doing
>> anything funky with /dev/tty.
>
> Ok. I think your proposal sounds fine.
Here's a patch for that. The pty stuff crashes and burns on my Windows
test box, so there I just set stdin to an empty string via the usual
pipe mechanism.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index a377cdb226..5ff138360c 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -87,6 +87,8 @@ our @EXPORT = qw(
our ($windows_os, $tmp_check, $log_path, $test_logfile);
+my @no_stdin;
+
BEGIN
{
@@ -178,6 +180,20 @@ INIT
autoflush STDOUT 1;
autoflush STDERR 1;
autoflush $testlog 1;
+
+ # Settings to close stdin for certain commands.
+ # On non-Windows, use a pseudo-terminal, so that libraries like openssl
+ # which open the tty if they think stdin isn't one for a password
+ # don't block. Windows doesn't have ptys, so just provide an empty
+ # string for stdin.
+ if ($windows_os)
+ {
+ @no_stdin = ('<', \"");
+ }
+ else
+ {
+ @no_stdin = ('<pty<', \"\x04");
+ }
}
END
@@ -343,7 +359,7 @@ sub run_command
{
my ($cmd) = @_;
my ($stdout, $stderr);
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr, @no_stdin;
chomp($stdout);
chomp($stderr);
return ($stdout, $stderr);
@@ -576,7 +592,7 @@ sub check_pg_config
my ($regexp) = @_;
my ($stdout, $stderr);
my $result = IPC::Run::run [ 'pg_config', '--includedir' ], '>',
- \$stdout, '2>', \$stderr
+ \$stdout, '2>', \$stderr, @no_stdin
or die "could not execute pg_config";
chomp($stdout);
$stdout =~ s/\r$//;
@@ -673,7 +689,7 @@ sub program_help_ok
my ($stdout, $stderr);
print("# Running: $cmd --help\n");
my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
- \$stderr;
+ \$stderr, @no_stdin;
ok($result, "$cmd --help exit code 0");
isnt($stdout, '', "$cmd --help goes to stdout");
is($stderr, '', "$cmd --help nothing to stderr");
@@ -695,7 +711,7 @@ sub program_version_ok
my ($stdout, $stderr);
print("# Running: $cmd --version\n");
my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
- \$stderr;
+ \$stderr, @no_stdin;
ok($result, "$cmd --version exit code 0");
isnt($stdout, '', "$cmd --version goes to stdout");
is($stderr, '', "$cmd --version nothing to stderr");
@@ -718,8 +734,7 @@ sub program_options_handling_ok
my ($stdout, $stderr);
print("# Running: $cmd --not-a-valid-option\n");
my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
- \$stdout,
- '2>', \$stderr;
+ \$stdout, '2>', \$stderr, @no_stdin;
ok(!$result, "$cmd with invalid option nonzero exit code");
isnt($stderr, '', "$cmd with invalid option prints error message");
return;
@@ -740,7 +755,7 @@ sub command_like
my ($cmd, $expected_stdout, $test_name) = @_;
my ($stdout, $stderr);
print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr, @no_stdin;
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
like($stdout, $expected_stdout, "$test_name: matches");
@@ -769,7 +784,8 @@ sub command_like_safe
my $stdoutfile = File::Temp->new();
my $stderrfile = File::Temp->new();
print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', $stdoutfile, '2>', $stderrfile;
+ my $result = IPC::Run::run $cmd, '>', $stdoutfile, '2>', $stderrfile,
+ @no_stdin;
$stdout = slurp_file($stdoutfile);
$stderr = slurp_file($stderrfile);
ok($result, "$test_name: exit code 0");
@@ -793,7 +809,7 @@ sub command_fails_like
my ($cmd, $expected_stderr, $test_name) = @_;
my ($stdout, $stderr);
print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr, @no_stdin;
ok(!$result, "$test_name: exit code not 0");
like($stderr, $expected_stderr, "$test_name: matches");
return;
@@ -831,7 +847,7 @@ sub command_checks_all
# run command
my ($stdout, $stderr);
print("# Running: " . join(" ", @{$cmd}) . "\n");
- IPC::Run::run($cmd, '>', \$stdout, '2>', \$stderr);
+ IPC::Run::run($cmd, '>', \$stdout, '2>', \$stderr, @no_stdin);
# See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR
my $ret = $?;
