Thread: Weird portability issue in TestLib.pm's slurp_file
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.
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