[PATCH 0/3] libgit2 fixes for CentOS 7.x users

[PATCH 0/3] libgit2 fixes for CentOS 7.x users

[Eric Wong]

These only spring up with libgit2-devel installed, but
3/3 seems important for all users of older Perl in
order to avoid future surprises.

Eric Wong (3):
  gcf2client: add alias for PublicInbox::Git::fail
  gcf2: fix autodie usage for older Perl
  treewide: more autodie safety fixes for older Perl

 lib/PublicInbox/Gcf2.pm         | 13 ++++++-------
 lib/PublicInbox/Gcf2Client.pm   |  1 +
 lib/PublicInbox/IO.pm           |  2 ++
 lib/PublicInbox/LEI.pm          |  7 +++----
 lib/PublicInbox/TestCommon.pm   |  5 ++---
 lib/PublicInbox/XapHelperCxx.pm | 18 ++++++++----------
 t/extsearch.t                   |  3 +--
 t/io.t                          | 10 +++++++++-
 8 files changed, 32 insertions(+), 27 deletions(-)

[PATCH 1/3] gcf2client: add alias for PublicInbox::Git::fail

[Eric Wong]

Ensure we can ->fail properly from other subs we can within
Gcf2Client.  This doesn't fix the test failures on CentOS 7.x,
but tries to make it easier to fix underlying problems and
report OOM errors and other things which the test suite doesn't
touch on.
---
 lib/PublicInbox/Gcf2Client.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 5220c474..48d8c5ac 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -52,6 +52,7 @@ no warnings 'once';
 
 *cat_async_step = \&PublicInbox::Git::cat_async_step; # for event_step
 *event_step = \&PublicInbox::Git::event_step;
+*fail = \&PublicInbox::Git::fail;
 *DESTROY = \&PublicInbox::Git::DESTROY;
 
 1;

[PATCH 2/3] gcf2: fix autodie usage for older Perl

[Eric Wong]

At least on Perl v5.16.3 on CentOS 7.x, use-ing autodie within
BEGIN {} affects all subroutines in that package, too.  So just
use autodie at the top-level and rely on CORE::* and try_cat
to handle cases where autodie isn't desired.
---
 lib/PublicInbox/Gcf2.pm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm
index e0219b55..dcbb201d 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Gcf2.pm
@@ -11,10 +11,9 @@ use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
 use IO::Handle; # autoflush
 use PublicInbox::Git;
 use PublicInbox::Lock;
-use autodie qw(close);
+use autodie qw(close open seek truncate);
 
 BEGIN {
-	use autodie;
 	my (%CFG, $c_src);
 	# PublicInbox::Spawn will set PERL_INLINE_DIRECTORY
 	# to ~/.cache/public-inbox/inline-c if it exists and Inline::C works
@@ -80,9 +79,8 @@ sub add_alt ($$) {
 	# to refer to $V2INBOX_DIR/git/$EPOCH.git/objects
 	#
 	# See https://bugs.debian.org/975607
-	if (open(my $fh, '<', "$objdir/info/alternates")) {
-		chomp(my @abs_alt = grep m!^/!, PublicInbox::IO::read_all $fh);
-		$gcf2->add_alternate($_) for @abs_alt;
+	if (my $s = PublicInbox::IO::try_cat("$objdir/info/alternates")) {
+		$gcf2->add_alternate($_) for ($s =~ m!^(/[^\n]+)\n!gms);
 	}
 	$gcf2->add_alternate($objdir);
 	1;
@@ -92,8 +90,9 @@ sub have_unlinked_files () {
 	# FIXME: port gcf2-like over to git.git so we won't need to
 	# deal with libgit2
 	return 1 if $^O ne 'linux';
-	open my $fh, '<', "/proc/$$/maps" or return;
-	while (<$fh>) { return 1 if /\.(?:idx|pack) \(deleted\)$/ }
+	if (my $s = PublicInbox::IO::try_cat("/proc/$$/maps")) {
+		return 1 if /\.(?:idx|pack) \(deleted\)/s;
+	}
 	undef;
 }
 

[PATCH 3/3] treewide: more autodie safety fixes for older Perl

[Eric Wong]

Avoid mixing autodie use in different scopes since it's likely
to cause problems like it did in Gcf2.  While none of these
fix known problems with test cases, it's likely worthwhile to
avoid it anyways to avoid future surprises.

For Process::IO, we'll add some additional tests in t/io.t
to ensure we don't get unintended exceptions for try_cat.
---
 lib/PublicInbox/IO.pm           |  2 ++
 lib/PublicInbox/LEI.pm          |  7 +++----
 lib/PublicInbox/TestCommon.pm   |  5 ++---
 lib/PublicInbox/XapHelperCxx.pm | 18 ++++++++----------
 t/extsearch.t                   |  3 +--
 t/io.t                          | 10 +++++++++-
 6 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/lib/PublicInbox/IO.pm b/lib/PublicInbox/IO.pm
index 11ce9be1..63ae3ef4 100644
--- a/lib/PublicInbox/IO.pm
+++ b/lib/PublicInbox/IO.pm
@@ -9,6 +9,8 @@ use PublicInbox::DS qw(awaitpid);
 our @EXPORT_OK = qw(poll_in read_all try_cat write_file);
 use Carp qw(croak);
 use IO::Poll qw(POLLIN);
+# don't autodie in top-level for Perl 5.16.3 (and maybe newer versions)
+# we have our own ->close, so we scope autodie into each sub
 
 sub waitcb { # awaitpid callback
 	my ($pid, $errref, $cb, @args) = @_;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 69065ce7..460aed40 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -9,7 +9,7 @@ package PublicInbox::LEI;
 use v5.12;
 use parent qw(PublicInbox::DS PublicInbox::LeiExternal
 	PublicInbox::LeiQuery);
-use autodie qw(bind chdir fork open socket socketpair syswrite unlink);
+use autodie qw(bind chdir fork open pipe socket socketpair syswrite unlink);
 use Getopt::Long ();
 use Socket qw(AF_UNIX SOCK_SEQPACKET pack_sockaddr_un);
 use Errno qw(EPIPE EAGAIN ECONNREFUSED ENOENT ECONNRESET);
@@ -1099,8 +1099,8 @@ sub start_pager {
 	$new_env->{LESS} //= 'FRX';
 	$new_env->{LV} //= '-c';
 	$new_env->{MORE} = $new_env->{LESS} if $^O eq 'freebsd';
-	pipe(my ($r, $wpager)) or return warn "pipe: $!";
-	my $rdr = { 0 => $r, 1 => $self->{1}, 2 => $self->{2} };
+	my $rdr = { 1 => $self->{1}, 2 => $self->{2} };
+	CORE::pipe($rdr->{0}, my $wpager) or return warn "pipe: $!";
 	my $pgr = [ undef, @$rdr{1, 2} ];
 	my $env = $self->{env};
 	if ($self->{sock}) { # lei(1) process runs it
@@ -1580,7 +1580,6 @@ sub slurp_stdin {
 	my $in = $lei->{0};
 	if (-t $in) { # run cat via script/lei and read from it
 		$in = undef;
-		use autodie qw(pipe);
 		pipe($in, my $wr);
 		say { $lei->{2} } '# enter query, Ctrl-D when done';
 		send_exec_cmd($lei, [ $lei->{0}, $wr ], ['cat'], {});
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index a5546905..8bfa30f2 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -16,7 +16,7 @@ our @EXPORT;
 my $lei_loud = $ENV{TEST_LEI_ERR_LOUD};
 my $tail_cmd = $ENV{TAIL};
 our ($lei_opt, $lei_out, $lei_err);
-use autodie qw(chdir close fcntl open opendir seek unlink);
+use autodie qw(chdir close fcntl mkdir open opendir seek unlink);
 
 $_ = File::Spec->rel2abs($_) for (grep(!m!^/!, @INC));
 
@@ -670,7 +670,6 @@ sub test_lei {
 SKIP: {
 	my ($cb) = pop @_;
 	my $test_opt = shift // {};
-	use autodie qw(mkdir);
 	require_git(2.6, 1);
 	my $mods = $test_opt->{mods} // [ 'lei' ];
 	require_mods(@$mods, 2);
@@ -801,7 +800,7 @@ sub create_coderepo ($$;@) {
 	my ($db) = (PublicInbox::Import::default_branch() =~ m!([^/]+)\z!);
 	my $dir = "t/data-gen/$base.$ident-$db";
 	my $new = !-d $dir;
-	if ($new && !mkdir($dir)) {
+	if ($new && !CORE::mkdir($dir)) {
 		my $err = $!;
 		-d $dir or xbail "mkdir($dir): $err";
 	}
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index e516b111..1250c964 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -8,10 +8,11 @@
 package PublicInbox::XapHelperCxx;
 use v5.12;
 use PublicInbox::Spawn qw(run_die run_qx which);
-use PublicInbox::IO qw(read_all write_file);
+use PublicInbox::IO qw(read_all try_cat write_file);
 use PublicInbox::Search;
 use Fcntl qw(SEEK_SET);
 use Config;
+use autodie;
 my $cxx = which($ENV{CXX} // 'c++');
 my $dir = substr("$cxx-$Config{archname}", 1); # drop leading '/'
 $dir =~ tr!/!-!;
@@ -39,25 +40,22 @@ EOM
 }
 
 sub needs_rebuild () {
-	open my $fh, '<', "$dir/XFLAGS" or return 1;
-	chomp(my $prev = read_all($fh));
+	my $prev = try_cat("$dir/XFLAGS") or return 1;
+	chomp $prev;
 	return 1 if $prev ne $xflags;
 
-	open $fh, '<', "$dir/xap_modversion" or return 1;
-	chomp($prev = read_all($fh));
-	$prev or return 1;
+	$prev = try_cat("$dir/xap_modversion") or return 1;
+	chomp $prev;
 
 	$xap_modversion = xap_cfg('--modversion');
 	$xap_modversion ne $prev;
 }
 
 sub build () {
-	if (!-d $dir) {
-		my $err;
-		mkdir($dir) or $err = $!;
+	if (!-d $dir && !CORE::mkdir($dir)) {
+		my $err = $!;
 		die "mkdir($dir): $err" if !-d $dir;
 	}
-	use autodie;
 	require PublicInbox::CodeSearch;
 	require PublicInbox::Lock;
 	require PublicInbox::OnDestroy;
diff --git a/t/extsearch.t b/t/extsearch.t
index 1a1eb350..090f6db5 100644
--- a/t/extsearch.t
+++ b/t/extsearch.t
@@ -7,7 +7,7 @@ use PublicInbox::Config;
 use PublicInbox::InboxWritable;
 require_git(2.6);
 require_mods(qw(json DBD::SQLite Xapian));
-use autodie qw(open rename truncate);
+use autodie qw(open rename truncate unlink);
 require PublicInbox::Search;
 use_ok 'PublicInbox::ExtSearch';
 use_ok 'PublicInbox::ExtSearchIdx';
@@ -577,7 +577,6 @@ test_lei(sub {
 	is_deeply($dst, \@md1,
 		"convert from extindex w/ or w/o `extindex' prefix");
 
-	use autodie qw(unlink);
 	my @o = glob "$home/extindex/ei*/over.sqlite*";
 	unlink(@o);
 	ok(!lei('convert', '-o', "$home/fail", "extindex:$d"));
diff --git a/t/io.t b/t/io.t
index 3ea00ed8..91f93526 100644
--- a/t/io.t
+++ b/t/io.t
@@ -7,7 +7,7 @@ my $tmpdir = tmpdir;
 use_ok 'PublicInbox::IO';
 use PublicInbox::Spawn qw(which run_qx);
 
-# only test failures
+# test failures:
 SKIP: {
 my $strace = strace_inject;
 my $env = { PERL5LIB => join(':', @INC) };
@@ -24,4 +24,12 @@ like($err, qr/\bclose\b/, 'close error noted');
 is(-s $dst, 0, 'file created and empty after EIO');
 } # /SKIP
 
+PublicInbox::IO::write_file '>:unix', "$tmpdir/f", "HI\n";
+is(-s "$tmpdir/f", 3, 'write_file works w/ IO layer');
+PublicInbox::IO::write_file '>>', "$tmpdir/f", "HI\n";
+is(-s "$tmpdir/f", 6, 'write_file can append');
+
+is PublicInbox::IO::try_cat("$tmpdir/non-existent"), '',
+	"try_cat on non-existent file returns `'";
+
 done_testing;

[PATCH 4/3] xap_helper_cxx: accept leading spaces from pkg-config

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> Avoid mixing autodie use in different scopes since it's likely
> to cause problems like it did in Gcf2.  While none of these
> fix known problems with test cases, it's likely worthwhile to
> avoid it anyways to avoid future surprises.

>  lib/PublicInbox/XapHelperCxx.pm | 18 ++++++++----------

That XapHelperCxx change was totally necessary for running the
C++ build on CentOS 7.x (but the test is auto-skipped on any
build failure), as is this one:

--------8<--------
Subject: [PATCH] xap_helper_cxx: accept leading spaces from pkg-config

pkg-config 0.27.1 and xapian14-core-devel (1.4.24-1.el7) on
CentOS 7.x will print a leading space when running
`pkg-config --libs --cflags xapian-core'.  This leading
space creates an empty string when `split' with /\s+/ as
a pattern.  Instead, use the documented ' ' (SP) character
to put split into "awk mode" which eats leading (and
redundant) spaces and tabs.
---
 lib/PublicInbox/XapHelperCxx.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 1250c964..9e819546 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -86,7 +86,7 @@ sub build () {
 	# distributed packages.
 	$^O eq 'netbsd' and $fl =~ s/(\A|[ \t])\-L([^ \t]+)([ \t]|\z)/
 				"$1-L$2 -Wl,-rpath=$2$3"/egsx;
-	my @xflags = split(/\s+/, "$fl $xflags");
+	my @xflags = split(' ', "$fl $xflags"); # ' ' awk-mode eats leading WS
 	my @cflags = grep(!/\A-(?:Wl|l|L)/, @xflags);
 	run_die([$cxx, '-c', "$prog.cpp", @cflags]);
 	run_die([$cxx, '-o', "$prog.tmp", "$prog.o", @xflags]);