Weird portability issue in TestLib.pm's slurp_file

Started by Tom Laneover 10 years ago2 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

For grins I tried running the TAP tests on my ancient HPUX box that
hosts pademelon and gaur. At first they showed a lot of failures,
which I eventually determined were happening because slurp_file was
only retrieving part of the postmaster logfile, causing issues_sql_like
to mistakenly report a failure. I don't know exactly why slurp_file
is misbehaving; it may well be a bug in perl 5.8.9. But anyway,
rewriting it like this makes the problem go away:

diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index da67f33c7e38929067350c7cf0da8fd8c5c1e43d..3940891e5d0533af93bacf3ff4af5a6fb5f10117 100644
*** a/src/test/perl/TestLib.pm
--- b/src/test/perl/TestLib.pm
*************** sub slurp_dir
*** 158,166 ****
  sub slurp_file
  {
  	local $/;
! 	local @ARGV = @_;
! 	my $contents = <>;
  	$contents =~ s/\r//g if $Config{osname} eq 'msys';
  	return $contents;
  }
--- 158,169 ----

sub slurp_file
{
+ my ($filename) = @_;
local $/;
! open(my $in, $filename)
! or die "could not read \"$filename\": $!";
! my $contents = <$in>;
! close $in;
$contents =~ s/\r//g if $Config{osname} eq 'msys';
return $contents;
}

To my admittedly-no-perl-expert eye, the existing coding is too cute
by half anyway, eg what it will do in error situations is documented
nowhere that I can find in man perlop.

Any objections to pushing this?

regards, tom lane

PS: I'm not planning to turn on TAP testing on pademelon/gaur; looks like
that would add about 40 minutes to what's already an hour and a half per
buildfarm run. But it might be good to check it manually every so often.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#1)
Re: Weird portability issue in TestLib.pm's slurp_file

On Tue, Dec 8, 2015 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

For grins I tried running the TAP tests on my ancient HPUX box that
hosts pademelon and gaur. At first they showed a lot of failures,
which I eventually determined were happening because slurp_file was
only retrieving part of the postmaster logfile, causing issues_sql_like
to mistakenly report a failure. I don't know exactly why slurp_file
is misbehaving; it may well be a bug in perl 5.8.9. But anyway,
rewriting it like this makes the problem go away:

diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index da67f33c7e38929067350c7cf0da8fd8c5c1e43d..3940891e5d0533af93bacf3ff4af5a6fb5f10117 100644
*** a/src/test/perl/TestLib.pm
--- b/src/test/perl/TestLib.pm
*************** sub slurp_dir
*** 158,166 ****
sub slurp_file
{
local $/;
!       local @ARGV = @_;
!       my $contents = <>;
$contents =~ s/\r//g if $Config{osname} eq 'msys';
return $contents;
}
--- 158,169 ----

sub slurp_file
{
+ my ($filename) = @_;
local $/;
! open(my $in, $filename)
! or die "could not read \"$filename\": $!";
! my $contents = <$in>;
! close $in;
$contents =~ s/\r//g if $Config{osname} eq 'msys';
return $contents;
}

To my admittedly-no-perl-expert eye, the existing coding is too cute
by half anyway, eg what it will do in error situations is documented
nowhere that I can find in man perlop.

Any objections to pushing this?

Your rewrite looks like a big improvement to me.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers