[PATCH] treewide: simplify File::Path mkpath/make_path callers

[PATCH] treewide: simplify File::Path mkpath/make_path callers

[Eric Wong]

File::Path already accounts for the existence of directories,
handles races from redundant mkdir(2), and croaks on
unrecoverable errors.  So there's no point in doing any
of that on our end.

Furthermore, avoiding the overhead of loading File::Path doesn't
seem worth it to save 20-60ms given the overhead of loading
our other code.  Instead, try to reduce optree overhead on
our code, instead, since File::Path gets used in a bunch of
places.

We'll also favor the newer make_path for multi-directory
invocations to avoid bloating our own optree to create an
arrayref, but mkpath is one fewer subroutine call within
File::Path itself, right now.
---
 lib/PublicInbox/Emergency.pm  | 11 ++---------
 lib/PublicInbox/Import.pm     |  2 +-
 lib/PublicInbox/LEI.pm        |  4 ++--
 lib/PublicInbox/LeiMirror.pm  |  2 +-
 lib/PublicInbox/LeiToMail.pm  |  9 ++-------
 lib/PublicInbox/SharedKV.pm   |  4 ++--
 lib/PublicInbox/TestCommon.pm |  2 +-
 t/solver_git.t                |  2 +-
 xt/imapd-mbsync-oimap.t       |  6 +++---
 9 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/lib/PublicInbox/Emergency.pm b/lib/PublicInbox/Emergency.pm
index 5a1ed1d7..56c58dd1 100644
--- a/lib/PublicInbox/Emergency.pm
+++ b/lib/PublicInbox/Emergency.pm
@@ -9,18 +9,11 @@ use Fcntl qw(:DEFAULT SEEK_SET);
 use Sys::Hostname qw(hostname);
 use IO::Handle; # ->flush
 use Errno qw(EEXIST);
+use File::Path ();
 
 sub new {
 	my ($class, $dir) = @_;
-
-	foreach (qw(new tmp cur)) {
-		my $d = "$dir/$_";
-		next if -d $d;
-		require File::Path;
-		if (!File::Path::mkpath($d) && !-d $d) {
-			die "failed to mkpath($d): $!\n";
-		}
-	}
+	File::Path::make_path(map { $dir.$_ } qw(/tmp /new /cur));
 	bless { dir => $dir, t => 0 }, $class;
 }
 
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 04192174..39719bcb 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -464,7 +464,7 @@ sub init_bare {
 	my ($dir, $head) = @_; # or self
 	$dir = $dir->{git}->{git_dir} if ref($dir);
 	require File::Path;
-	File::Path::mkpath([ map { "$dir/$_" } qw(objects/info refs/heads) ]);
+	File::Path::make_path(map { $dir.$_ } qw(/objects/info /refs/heads));
 	$INIT_FILES[1] //= 'ref: '.default_branch."\n";
 	my @fn_contents = @INIT_FILES;
 	$fn_contents[1] = "ref: refs/heads/$head\n" if defined $head;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index d05b20de..b83de91d 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -24,7 +24,7 @@ use PublicInbox::Eml;
 use PublicInbox::Import;
 use PublicInbox::ContentHash qw(git_sha);
 use Time::HiRes qw(stat); # ctime comparisons for config cache
-use File::Path qw(mkpath);
+use File::Path ();
 use File::Spec;
 use Sys::Syslog qw(openlog syslog closelog);
 our $quit = \&CORE::exit;
@@ -852,7 +852,7 @@ sub _lei_cfg ($;$) {
 			return bless {}, 'PublicInbox::Config';
 		}
 		my ($cfg_dir) = ($f =~ m!(.*?/)[^/]+\z!);
-		-d $cfg_dir or mkpath($cfg_dir) or die "mkpath($cfg_dir): $!\n";
+		File::Path::mkpath($cfg_dir);
 		open my $fh, '>>', $f or die "open($f): $!\n";
 		@st = stat($fh) or die "fstat($f): $!\n";
 		$cur_st = pack('dd', $st[10], $st[7]);
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index e4aa620e..87311198 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -913,7 +913,7 @@ failed to extract epoch number from $src
 	# filter out the epochs we skipped
 	$self->{chg}->{manifest} = 1 if $m && delete(@$m{@skip});
 
-	(!$self->{dry_run} && !-d $dst) and File::Path::mkpath($dst);
+	$self->{dry_run} or File::Path::mkpath($dst);
 
 	$lei->{opt}->{'inbox-config'} =~ /\A(?:always|v2)\z/s and
 		_get_txt_start($task, '_/text/config/raw', $fini);
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 31eba794..e357ee00 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -462,13 +462,8 @@ sub new {
 sub _pre_augment_maildir {
 	my ($self, $lei) = @_;
 	my $dst = $lei->{ovv}->{dst};
-	for my $x (qw(tmp new cur)) {
-		my $d = $dst.$x;
-		next if -d $d;
-		require File::Path;
-		File::Path::mkpath($d);
-		-d $d or die "$d is not a directory";
-	}
+	require File::Path;
+	File::Path::make_path(map { $dst.$_ } qw(tmp new cur));
 	# for utime, so no opendir
 	open $self->{poke_dh}, '<', "${dst}cur" or die "open ${dst}cur: $!";
 }
diff --git a/lib/PublicInbox/SharedKV.pm b/lib/PublicInbox/SharedKV.pm
index 90ccf2b4..89ab3f74 100644
--- a/lib/PublicInbox/SharedKV.pm
+++ b/lib/PublicInbox/SharedKV.pm
@@ -11,7 +11,7 @@ use parent qw(PublicInbox::Lock);
 use File::Temp qw(tempdir);
 use DBI qw(:sql_types); # SQL_BLOB
 use PublicInbox::Spawn;
-use File::Path qw(rmtree make_path);
+use File::Path qw(rmtree);
 
 sub dbh {
 	my ($self, $lock) = @_;
@@ -43,7 +43,7 @@ CREATE TABLE IF NOT EXISTS kv (
 sub new {
 	my ($cls, $dir, $base, $opt) = @_;
 	my $self = bless { opt => $opt }, $cls;
-	make_path($dir) if defined($dir) && !-d $dir;
+	File::Path::mkpath($dir) if defined($dir);
 	$dir //= $self->{"tmp$$.$self"} = tempdir("skv.$$-XXXX", TMPDIR => 1);
 	$base //= '';
 	my $f = $self->{filename} = "$dir/$base.sqlite3";
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 8a34e45a..fe7af452 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -607,7 +607,7 @@ SKIP: {
 	$lei_opt = { 1 => \$lei_out, 2 => \$lei_err };
 	my ($daemon_pid, $for_destroy, $daemon_xrd);
 	my $tmpdir = $test_opt->{tmpdir};
-	File::Path::mkpath($tmpdir) if (defined $tmpdir && !-d $tmpdir);
+	File::Path::mkpath($tmpdir) if defined $tmpdir;
 	($tmpdir, $for_destroy) = tmpdir unless $tmpdir;
 	state $persist_xrd = $ENV{TEST_LEI_DAEMON_PERSIST_DIR};
 	SKIP: {
diff --git a/t/solver_git.t b/t/solver_git.t
index e8d9feb9..02d90e5d 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -30,7 +30,7 @@ my $ibx = create_inbox 'v2', version => 2,
 	$im->add($patch2) or BAIL_OUT;
 };
 my $md = "$tmpdir/md";
-File::Path::mkpath([map { $md.$_ } (qw(/ /cur /new /tmp))]);
+File::Path::make_path(map { $md.$_ } (qw(/cur /new /tmp)));
 symlink(abs_path('t/solve/0001-simple-mod.patch'), "$md/cur/foo:2,") or
 	xbail "symlink: $!";
 
diff --git a/xt/imapd-mbsync-oimap.t b/xt/imapd-mbsync-oimap.t
index b0281105..f99779a1 100644
--- a/xt/imapd-mbsync-oimap.t
+++ b/xt/imapd-mbsync-oimap.t
@@ -4,7 +4,7 @@
 # ensure mbsync and offlineimap compatibility
 use strict;
 use v5.10.1;
-use File::Path qw(mkpath);
+use File::Path qw(make_path);
 use PublicInbox::TestCommon;
 use PublicInbox::Spawn qw(spawn);
 require_mods(qw(-imapd));
@@ -41,7 +41,7 @@ my ($host, $port) = ($sock->sockhost, $sock->sockport);
 my %pids;
 
 SKIP: {
-	mkpath([map { "$tmpdir/oimapdir/$_" } qw(cur new tmp)]);
+	make_path(map { "$tmpdir/oimapdir/$_" } qw(cur new tmp));
 	my $oimap = require_cmd('offlineimap', 1) or
 		skip 'no offlineimap(1)', 1;
 	open my $fh, '>', "$tmpdir/.offlineimaprc" or BAIL_OUT "open: $!";
@@ -78,7 +78,7 @@ EOF
 }
 
 SKIP: {
-	mkpath([map { "$tmpdir/mbsyncdir/test/$_" } qw(cur new tmp)]);
+	make_path(map { "$tmpdir/mbsyncdir/test/$_" } qw(cur new tmp));
 	my $mbsync = require_cmd('mbsync', 1) or skip 'no mbsync(1)', 1;
 	open my $fh, '>', "$tmpdir/.mbsyncrc" or BAIL_OUT "open: $!";
 	print $fh <<EOF or BAIL_OUT "print: $!";