From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 0E28F1F628 for ; Wed, 22 Feb 2023 17:25:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1677086756; bh=sxtyUzUcsZXo2LzcDVilSPtxztWtqd3n1ZpG4SNAhs8=; h=From:To:Subject:Date:From; b=3bVeenM02Jlk1B30Zk49D+HeplWZUAKteHcgTpxF26Gf1MSsPlr+ivI6PG84UTaDG Y9KbyC/QiB6J71UCeDkndbd4+lkT5IouXx4Q+PpAsuDnsi1YTjvZ2LXxekIgcv8w3E IGRMviNpl6Talc5Iwn4LnbrU6kup4aGUAmYm2vBQ= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] treewide: simplify File::Path mkpath/make_path callers Date: Wed, 22 Feb 2023 17:25:55 +0000 Message-Id: <20230222172555.822216-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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 <