[PATCH 0/3] just-ahead-of-time build improvements

[PATCH 0/3] just-ahead-of-time build improvements

[Eric Wong]

Our just-ahead-of-time builds need to detect upgrades to
libgit2 and libxapian and rebuild+relink as necessary.

This needs to happen for the FUSE-3 component, as well (post-2.0)

Eric Wong (3):
  gcf2: detect libgit2 version changes
  xap_helper: (C++) detect libxapian version changes
  gcf2: switch build phase to use autodie

 lib/PublicInbox/Gcf2.pm         | 63 ++++++++++++++++-----------------
 lib/PublicInbox/XapHelperCxx.pm | 50 +++++++++++++++++++-------
 2 files changed, 69 insertions(+), 44 deletions(-)

[PATCH 1/3] gcf2: detect libgit2 version changes

[Eric Wong]

We need to force Inline::C to rebuild if libgit2 is updated;
otherwise dynamic linking can be broken.  Adding the output
from the `--modversion' of pkg-config(1) along with the existing
`--libs' and `--cflags' output seems appropriate for this task.

To force Inline::C into a rebuild, neither CFLAGSEX nor CPPFLAGS
changes are enough.  Modifying the source string and adding
comments seems like the most obvious way to force a rebuild.

The `-print-file-name=LIBRARY' feature from gcc+clang could also
be used, but that requires parsing the library name from
`pkg-config --libs' output into a library basename appropriate
for `-print-file-name='.  IOW, we'd need to transform:
`-lgit2' => `libgit2.so'; and possibly deal with platforms
which deal with static libraries in the future.

So just use pkg-config, since `pkg-config --modversion' is
roughly 2-3x as fast as `gcc-10 -print-file-name=', and
10-20x faster than clang-11.
---
 lib/PublicInbox/Gcf2.pm | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm
index d13e6b1a..9ca99d00 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Gcf2.pm
@@ -4,8 +4,7 @@
 # backend for a git-cat-file-workalike based on libgit2,
 # other libgit2 stuff may go here, too.
 package PublicInbox::Gcf2;
-use strict;
-use v5.10.1;
+use v5.12;
 use PublicInbox::Spawn qw(which popen_rd); # may set PERL_INLINE_DIRECTORY
 use Fcntl qw(LOCK_EX SEEK_SET);
 use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
@@ -29,40 +28,41 @@ BEGIN {
 	my ($dir) = (__FILE__ =~ m!\A(.+?)/[^/]+\z!);
 	my $ef = "$inline_dir/.public-inbox.pkg-config.err";
 	open my $err, '+>', $ef or die "open($ef): $!";
-	for my $x (qw(libgit2)) {
-		my $rdr = { 2 => $err };
-		my ($l, $pid) = popen_rd([$pc, '--libs', $x], undef, $rdr);
-		$l = do { local $/; <$l> };
-		waitpid($pid, 0);
-		next if $?;
-		(my $c, $pid) = popen_rd([$pc, '--cflags', $x], undef, $rdr);
-		$c = do { local $/; <$c> };
-		waitpid($pid, 0);
-		next if $?;
-
+	my $vals = {};
+	my $rdr = { 2 => $err };
+	my @switches = qw(modversion cflags libs);
+	for my $k (@switches) {
+		my $rd = popen_rd([$pc, "--$k", 'libgit2'], undef, $rdr);
+		chomp(my $val = do { local $/; <$rd> });
+		close($rd) or last; # checks for error and sets $?
+		$vals->{$k} = $val;
+	}
+	if (!$?) {
 		# note: we name C source files .h to prevent
 		# ExtUtils::MakeMaker from automatically trying to
 		# build them.
-		my $f = "$dir/gcf2_$x.h";
+		my $f = "$dir/gcf2_libgit2.h";
 		open(my $src, '<', $f) or die "E: open($f): $!";
-		chomp($l, $c);
 		local $/;
 		defined($c_src = <$src>) or die "read $f: $!";
-		$CFG{LIBS} = $l;
-		$CFG{CCFLAGSEX} = $c;
-		last;
 	}
 	unless ($c_src) {
 		seek($err, 0, SEEK_SET);
 		$err = do { local $/; <$err> };
 		die "E: libgit2 not installed: $err\n";
 	}
+	# append pkg-config results to the source to ensure Inline::C
+	# can rebuild if there's changes (it doesn't seem to detect
+	# $CFG{CCFLAGSEX} nor $CFG{CPPFLAGS} changes)
+	$c_src .= "/* $pc --$_ libgit2 => $vals->{$_} */\n" for @switches;
 	open my $oldout, '>&', \*STDOUT or die "dup(1): $!";
 	open my $olderr, '>&', \*STDERR or die "dup(2): $!";
 	open STDOUT, '>&', $fh or die "1>$f: $!";
 	open STDERR, '>&', $fh or die "2>$f: $!";
 	STDERR->autoflush(1);
 	STDOUT->autoflush(1);
+	$CFG{CCFLAGSEX} = $vals->{cflags};
+	$CFG{LIBS} = $vals->{libs};
 
 	# we use Capitalized and ALLCAPS for compatibility with old Inline::C
 	eval <<'EOM';

[PATCH 2/3] xap_helper_cxx: detect libxapian version changes

[Eric Wong]

As with libgit2, Xapian can be upgraded and break linkage
with the xap_helper binary.  For now, save the result of
`pkg-config --modversion xapian-core' to a file and
compare it with the current output.

Perhaps portable some make(1) can be used for this...
---
 lib/PublicInbox/XapHelperCxx.pm | 50 +++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 1c9a314f..5157fb35 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -7,8 +7,9 @@
 # The resulting executable is not linked to Perl in any way.
 package PublicInbox::XapHelperCxx;
 use v5.12;
-use PublicInbox::Spawn;
+use PublicInbox::Spawn qw(popen_rd);
 use PublicInbox::Search;
+use Fcntl qw(SEEK_SET);
 my $dir = ($ENV{PERL_INLINE_DIRECTORY} //
 	die('BUG: PERL_INLINE_DIRECTORY unset')) . '/cxx';
 my $bin = "$dir/xap_helper";
@@ -20,11 +21,33 @@ $ldflags .= ' -Wl,--compress-debug-sections=zlib' if $^O ne 'openbsd';
 my $xflags = ($ENV{CXXFLAGS} // '-Wall -ggdb3 -O0') . ' ' .
 	($ENV{LDFLAGS} // $ldflags) .
 	qq{ -DTHREADID=}.PublicInbox::Search::THREADID;
+my $xap_modversion;
 
-sub xflags_chg () {
+sub xap_cfg (@) {
+	open my $err, '+>', undef or die "open(undef): $!";
+	my $cmd = [ $ENV{PKG_CONFIG} // 'pkg-config', @_, 'xapian-core' ];
+	my $rd = popen_rd($cmd, undef, { 2 => $err });
+	chomp(my $ret = do { local $/; <$rd> });
+	return $ret if close($rd);
+	seek($err, 0, SEEK_SET) or die "seek: $!";
+	$err = do { local $/; <$err> };
+	die <<EOM;
+@$cmd failed: Xapian devlopment files missing? (\$?=$?)
+$err
+EOM
+}
+
+sub needs_rebuild () {
 	open my $fh, '<', "$dir/XFLAGS" or return 1;
 	chomp(my $prev = <$fh>);
-	$prev ne $xflags;
+	return 1 if $prev ne $xflags;
+
+	open $fh, '<', "$dir/xap_modversion" or return 1;
+	chomp($prev = <$fh>);
+	$prev or return 1;
+
+	$xap_modversion = xap_cfg('--modversion');
+	$xap_modversion ne $prev;
 }
 
 sub build () {
@@ -37,7 +60,6 @@ sub build () {
 	require File::Temp;
 	require PublicInbox::CodeSearch;
 	my ($prog) = ($bin =~ m!/([^/]+)\z!);
-	my $pkg_config = $ENV{PKG_CONFIG} // 'pkg-config';
 	my $tmp = File::Temp->newdir(DIR => $dir) // die "newdir: $!";
 	my $src = "$tmp/$prog.cpp";
 	open my $fh, '>', $src;
@@ -51,9 +73,9 @@ sub build () {
 	print $fh PublicInbox::CodeSearch::generate_cxx();
 	close $fh;
 
-	my $cmd = "$pkg_config --libs --cflags xapian-core";
-	chomp(my $fl = `$cmd`);
-	die "$cmd failed: \$?=$?" if $?;
+	# xap_modversion may be set by needs_rebuild
+	$xap_modversion //= xap_cfg('--modversion');
+	my $fl = xap_cfg(qw(--libs --cflags));
 
 	# Using rpath seems acceptable/encouraged in the NetBSD packaging world
 	# since /usr/pkg/lib isn't searched by the dynamic loader by default.
@@ -64,14 +86,18 @@ sub build () {
 				"$1-L$2 -Wl,-rpath=$2$3"/egsx;
 
 	my $cxx = $ENV{CXX} // 'c++';
-	$cmd = "$cxx $src $fl $xflags -o $tmp/$prog";
+	my $cmd = "$cxx $src $fl $xflags -o $tmp/$prog";
 	system($cmd) and die "$cmd failed: \$?=$?";
-	my $cf = "$tmp/XFLAGS";
-	open $fh, '>', $cf;
+	open $fh, '>', "$tmp/XFLAGS";
 	say $fh $xflags;
 	close $fh;
+
+	open $fh, '>', "$tmp/xap_modversion";
+	say $fh $xap_modversion;
+	close $fh;
+	undef $xap_modversion; # do we ever build() twice?
 	# not quite atomic, but close enough :P
-	rename("$tmp/$_", "$dir/$_") for ($prog, 'XFLAGS');
+	rename("$tmp/$_", "$dir/$_") for ($prog, qw(XFLAGS xap_modversion));
 }
 
 sub check_build () {
@@ -85,7 +111,7 @@ sub check_build () {
 			return build() if $ctime > $bin[10];
 		}
 	}
-	xflags_chg() ? build() : 0;
+	needs_rebuild() ? build() : 0;
 }
 
 sub start (@) {

[PATCH 3/3] gcf2: switch build phase to use autodie

[Eric Wong]

This simplifies much of our code since much of it is
error-handling.
---
 lib/PublicInbox/Gcf2.pm | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm
index 9ca99d00..0f4d2bf0 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Gcf2.pm
@@ -12,29 +12,28 @@ use IO::Handle; # autoflush
 use PublicInbox::Git;
 
 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
 	my $inline_dir = $ENV{PERL_INLINE_DIRECTORY} //
 		die 'PERL_INLINE_DIRECTORY not defined';
-	my $f = "$inline_dir/.public-inbox.lock";
-	open my $fh, '+>', $f or die "open($f): $!";
+	open my $fh, '+>', "$inline_dir/.public-inbox.lock";
 
 	# CentOS 7.x ships Inline 0.53, 0.64+ has built-in locking
-	flock($fh, LOCK_EX) or die "LOCK_EX($f): $!\n";
+	flock($fh, LOCK_EX);
 
 	my $pc = which($ENV{PKG_CONFIG} // 'pkg-config') //
 		die "pkg-config missing for libgit2";
 	my ($dir) = (__FILE__ =~ m!\A(.+?)/[^/]+\z!);
-	my $ef = "$inline_dir/.public-inbox.pkg-config.err";
-	open my $err, '+>', $ef or die "open($ef): $!";
+	open my $err, '+>', "$inline_dir/.public-inbox.pkg-config.err";
 	my $vals = {};
 	my $rdr = { 2 => $err };
 	my @switches = qw(modversion cflags libs);
 	for my $k (@switches) {
 		my $rd = popen_rd([$pc, "--$k", 'libgit2'], undef, $rdr);
 		chomp(my $val = do { local $/; <$rd> });
-		close($rd) or last; # checks for error and sets $?
+		CORE::close($rd) or last; # checks for error and sets $?
 		$vals->{$k} = $val;
 	}
 	if (!$?) {
@@ -42,9 +41,9 @@ BEGIN {
 		# ExtUtils::MakeMaker from automatically trying to
 		# build them.
 		my $f = "$dir/gcf2_libgit2.h";
-		open(my $src, '<', $f) or die "E: open($f): $!";
+		open my $src, '<', $f;
 		local $/;
-		defined($c_src = <$src>) or die "read $f: $!";
+		$c_src = <$src> // die "read $f: $!";
 	}
 	unless ($c_src) {
 		seek($err, 0, SEEK_SET);
@@ -55,23 +54,23 @@ BEGIN {
 	# can rebuild if there's changes (it doesn't seem to detect
 	# $CFG{CCFLAGSEX} nor $CFG{CPPFLAGS} changes)
 	$c_src .= "/* $pc --$_ libgit2 => $vals->{$_} */\n" for @switches;
-	open my $oldout, '>&', \*STDOUT or die "dup(1): $!";
-	open my $olderr, '>&', \*STDERR or die "dup(2): $!";
-	open STDOUT, '>&', $fh or die "1>$f: $!";
-	open STDERR, '>&', $fh or die "2>$f: $!";
+	open my $oldout, '>&', \*STDOUT;
+	open my $olderr, '>&', \*STDERR;
+	open STDOUT, '>&', $fh;
+	open STDERR, '>&', $fh;
 	STDERR->autoflush(1);
 	STDOUT->autoflush(1);
 	$CFG{CCFLAGSEX} = $vals->{cflags};
 	$CFG{LIBS} = $vals->{libs};
 
 	# we use Capitalized and ALLCAPS for compatibility with old Inline::C
-	eval <<'EOM';
+	CORE::eval <<'EOM';
 use Inline C => Config => %CFG, BOOT => q[git_libgit2_init();];
 use Inline C => $c_src, BUILD_NOISY => 1;
 EOM
 	$err = $@;
-	open(STDERR, '>&', $olderr) or warn "restore stderr: $!";
-	open(STDOUT, '>&', $oldout) or warn "restore stdout: $!";
+	open(STDERR, '>&', $olderr);
+	open(STDOUT, '>&', $oldout);
 	if ($err) {
 		seek($fh, 0, SEEK_SET);
 		my @msg = <$fh>;