[PATCH 0/6] v5.12 + autodie cleanups

[PATCH 0/6] v5.12 + autodie cleanups

[Eric Wong]

Some yak shaving while testing long-running `lei imports'...

Eric Wong (6):
  index: use v5.12, remove outdated comment
  over: use autodie for open
  lei_blob: use autodie for open + seek
  admin: autodie chdir + open
  xapcmd: use autodie for numerous syscalls
  spamc: autodie for open + sysseek

 lib/PublicInbox/Admin.pm           |  7 +++----
 lib/PublicInbox/LeiBlob.pm         | 11 ++++++-----
 lib/PublicInbox/Over.pm            |  3 ++-
 lib/PublicInbox/Spamcheck/Spamc.pm | 10 ++++------
 lib/PublicInbox/Xapcmd.pm          | 12 +++++-------
 script/public-inbox-index          |  7 ++-----
 6 files changed, 22 insertions(+), 28 deletions(-)

[PATCH 1/6] index: use v5.12, remove outdated comment

[Eric Wong]

There's no unicode_strings-dependent code in this script, so
v5.12 is safe.  The comment about libeatmydata shouldn't be
necessary for -index any longer since we support --no-fsync
nowadays and Xapian 1.4+ is fairly widespread.
---
 script/public-inbox-index | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/script/public-inbox-index b/script/public-inbox-index
index a13e44bf..82baa7b4 100755
--- a/script/public-inbox-index
+++ b/script/public-inbox-index
@@ -1,12 +1,9 @@
 #!perl -w
-# Copyright (C) 2015-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 # Basic tool to create a Xapian search index for a public-inbox.
-# Usage with libeatmydata <https://www.flamingspork.com/projects/libeatmydata/>
-# highly recommended: eatmydata public-inbox-index INBOX_DIR
 
-use strict;
-use v5.10.1;
+use v5.12;
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 my $help = <<EOF; # the following should fit w/o scrolling in 80x24 term:
 usage: public-inbox-index [options] INBOX_DIR

[PATCH 2/6] over: use autodie for open

[Eric Wong]

autodie improves the consistency of error messages in
most places, so we use it here since there's no detail
lost.
---
 lib/PublicInbox/Over.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index 3b7d49f5..f68964c2 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -13,6 +13,7 @@ use PublicInbox::Smsg;
 use Compress::Zlib qw(uncompress);
 use constant DEFAULT_LIMIT => 1000;
 use List::Util (); # for max
+use autodie qw(open);
 
 sub dbh_new {
 	my ($self, $rw) = @_;
@@ -22,7 +23,7 @@ sub dbh_new {
 			require PublicInbox::Syscall;
 			my ($dir) = ($f =~ m!(.+)/[^/]+\z!);
 			PublicInbox::Syscall::nodatacow_dir($dir);
-			open my $fh, '+>>', $f or die "failed to open $f: $!";
+			open my $fh, '+>>', $f;
 		} else {
 			$self->{filename} = $f; # die on stat() below:
 		}

[PATCH 3/6] lei_blob: use autodie for open + seek

[Eric Wong]

The numerous open() calls are less noisy on our end and more
consistent.  Our seek() call needed error checking anyways, and
autodie provides it.
---
 lib/PublicInbox/LeiBlob.pm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 31936c36..1c5ee302 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -7,6 +7,7 @@ package PublicInbox::LeiBlob;
 use strict;
 use v5.10.1;
 use parent qw(PublicInbox::IPC);
+use autodie qw(open seek);
 use PublicInbox::Spawn qw(run_wait run_qx which);
 use PublicInbox::DS;
 use PublicInbox::Eml;
@@ -22,7 +23,7 @@ sub get_git_dir ($$) {
 	if (defined($lei->{opt}->{cwd})) { # --cwd used, report errors
 		$opt->{2} = $lei->{2};
 	} else { # implicit --cwd, quiet errors
-		open $opt->{2}, '>', '/dev/null' or die "open /dev/null: $!";
+		open $opt->{2}, '>', '/dev/null';
 	}
 	chomp(my $git_dir = run_qx($cmd, {GIT_DIR => undef}, $opt));
 	$? ? undef : $git_dir;
@@ -57,7 +58,7 @@ sub do_solve_blob { # via wq_do
 		$x =~ tr/-/_/;
 		$hints->{$x} = $v;
 	}
-	open my $log, '+>', \(my $log_buf = '') or die "PerlIO::scalar: $!";
+	open my $log, '+>', \(my $log_buf = '');
 	$lei->{log_buf} = \$log_buf;
 	my $git = $lei->{ale}->git;
 	my @rmt = map {
@@ -114,9 +115,9 @@ sub lei_blob {
 		}
 		my $rdr = {};
 		if ($opt->{mail}) {
-			open $rdr->{2}, '+>', undef or die "open: $!";
+			open $rdr->{2}, '+>', undef;
 		} else {
-			open $rdr->{2}, '>', '/dev/null' or die "open: $!";
+			open $rdr->{2}, '>', '/dev/null';
 		}
 		my $cmd = $lei->ale->git->cmd('cat-file', 'blob', $blob);
 		my $cerr;
@@ -139,7 +140,7 @@ sub lei_blob {
 					extract_attach($lei, $blob, $bref) :
 					$lei->out($$bref);
 		if ($opt->{mail}) {
-			seek($rdr->{2}, 0, 0);
+			seek $rdr->{2}, 0, 0; # regular file (see above)
 			return $lei->child_error($cerr, read_all($rdr->{2}));
 		} # else: fall through to solver below
 	}

[PATCH 4/6] admin: autodie chdir + open

[Eric Wong]

autodie gives us more consistent error messages and reduces
visual noise on our end.  We can also open() directly into a
hash entry without relying on a temporary variable.
---
 lib/PublicInbox/Admin.pm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm
index bb5d3653..fb745cf8 100644
--- a/lib/PublicInbox/Admin.pm
+++ b/lib/PublicInbox/Admin.pm
@@ -6,6 +6,7 @@
 package PublicInbox::Admin;
 use v5.12;
 use parent qw(Exporter);
+use autodie qw(chdir open);
 our @EXPORT_OK = qw(setup_signals fmt_localtime);
 use PublicInbox::Config;
 use PublicInbox::Inbox;
@@ -313,9 +314,7 @@ sub progress_prepare ($;$) {
 		$opt->{quiet} = !$opt->{verbose};
 	}
 	if ($opt->{quiet}) {
-		open my $null, '>', '/dev/null' or
-			die "failed to open /dev/null: $!\n";
-		$opt->{1} = $null; # suitable for spawn() redirect
+		open $opt->{1}, '>', '/dev/null'; # suitable for spawn() redirect
 	} else {
 		$opt->{verbose} ||= 1;
 		$dst //= \*STDERR;
@@ -378,7 +377,7 @@ sub do_chdir ($) {
 	my $chdir = $_[0] // return;
 	for my $d (@$chdir) {
 		next if $d eq ''; # same as git(1)
-		chdir $d or die "cd $d: $!";
+		chdir $d;
 	}
 }
 

[PATCH 5/6] xapcmd: use autodie for numerous syscalls

[Eric Wong]

autodie allows us to simplify a multi-line statement for
conditional rename() and improve error message consistency
with the rest of our codebase.

syswrite() error checking was missing before and now exists
trivially due to autodie.
---
 lib/PublicInbox/Xapcmd.pm | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 9a148ae4..4af50ea8 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -2,6 +2,7 @@
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 package PublicInbox::Xapcmd;
 use v5.12;
+use autodie qw(chmod opendir rename syswrite);
 use PublicInbox::Spawn qw(which popen_rd);
 use PublicInbox::Syscall;
 use PublicInbox::Admin qw(setup_signals);
@@ -61,12 +62,9 @@ sub commit_changes ($$$$) {
 			next;
 		}
 
-		chmod($mode & 07777, $new) or die "chmod($new): $!\n";
-		if ($have_old) {
-			rename($old, "$new/old") or
-					die "rename $old => $new/old: $!\n";
-		}
-		rename($new, $old) or die "rename $new => $old: $!\n";
+		chmod $mode & 07777, $new;
+		rename $old, "$new/old" if $have_old;
+		rename $new, $old;
 		push @old_shard, "$old/old" if $have_old;
 	}
 
@@ -219,7 +217,7 @@ sub prepare_run {
 		PublicInbox::Syscall::nodatacow_dir($wip->dirname);
 		push @queue, [ $old, $wip ];
 	} elsif (defined $old) {
-		opendir my $dh, $old or die "Failed to opendir $old: $!\n";
+		opendir my $dh, $old;
 		my @old_shards;
 		while (defined(my $dn = readdir($dh))) {
 			if ($dn =~ /\A[0-9]+\z/) {

[PATCH 6/6] spamc: autodie for open + sysseek

[Eric Wong]

autodie allows us to shorten some lines and generate
error messages which are more consistent with the rest
of the codebase.
---
 lib/PublicInbox/Spamcheck/Spamc.pm | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm
index b4f95e2b..71474b8f 100644
--- a/lib/PublicInbox/Spamcheck/Spamc.pm
+++ b/lib/PublicInbox/Spamcheck/Spamc.pm
@@ -4,6 +4,7 @@
 # Default spam filter class for wrapping spamc(1)
 package PublicInbox::Spamcheck::Spamc;
 use v5.12;
+use autodie qw(open sysseek);
 use PublicInbox::Spawn qw(run_qx run_wait);
 use IO::Handle;
 use Fcntl qw(SEEK_SET);
@@ -48,8 +49,7 @@ sub _learn {
 sub _devnull {
 	my ($self) = @_;
 	$self->{-devnull} //= do {
-		open my $fh, '+>', '/dev/null' or
-				die "failed to open /dev/null: $!";
+		open my $fh, '+>', '/dev/null';
 		$fh
 	}
 }
@@ -60,13 +60,11 @@ sub _msg_to_fh {
 		my $fd = eval { fileno($msg) };
 		return $msg if defined($fd) && $fd >= 0;
 
-		open(my $tmpfh, '+>', undef) or die "failed to open: $!";
+		open my $tmpfh, '+>', undef;
 		$tmpfh->autoflush(1);
 		$msg = \($msg->as_string) if $ref ne 'SCALAR';
 		print $tmpfh $$msg or die "failed to print: $!";
-		sysseek($tmpfh, 0, SEEK_SET) or
-			die "sysseek(fh) failed: $!";
-
+		sysseek $tmpfh, 0, SEEK_SET;
 		return $tmpfh;
 	}
 	$msg;