* [PATCH 1/4] cindex: rework path canonicalization check
2024-11-12 20:34 [PATCH 0/4] File::Spec-related things Eric Wong
@ 2024-11-12 20:34 ` Eric Wong
2024-11-12 20:34 ` [PATCH 2/4] www_coderepo: drop unused File::Spec import Eric Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2024-11-12 20:34 UTC (permalink / raw)
To: meta
While reading the code, I noticed inadvertant `$_' use when the
loop iterator is `$d'. Using `$_' here would result in
uninitialized variable access. I've yet to hit this case in
real-world access.
Furthermore, we can use a single pass to canonicalize existing
directories instead of relying on a grep block, first.
Finally, favor File::Spec::Functions since `->' method dispatch
is slower than normal subroutine calls by a small amount even
when both the package and method names are static and known
early in advance..
---
lib/PublicInbox/CodeSearchIdx.pm | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index ff3db8ba..13533a00 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -56,7 +56,7 @@ use PublicInbox::PktOp;
use PublicInbox::IPC qw(nproc_shards);
use POSIX qw(WNOHANG SEEK_SET strftime);
use File::Path ();
-use File::Spec ();
+use File::Spec::Functions qw(canonpath);
use List::Util qw(max);
use PublicInbox::SHA qw(sha256_hex sha_all);
use PublicInbox::Search qw(xap_terms);
@@ -1341,15 +1341,15 @@ sub cidx_run { # main entry point
delete $REINDEX->{lock_path};
$REINDEX->dbh;
}
- my @nc = grep { File::Spec->canonpath($_) ne $_ } @{$self->{git_dirs}};
- if (@nc) {
- warn "E: BUG? paths in $self->{cidx_dir} not canonicalized:\n";
- for my $d (@{$self->{git_dirs}}) {
- my $c = File::Spec->canonpath($_);
- warn "E: $d => $c\n";
- $d = $c;
- }
- warn "E: canonicalized and attempting to continue\n";
+ my $cwarn;
+ for my $d (@{$self->{git_dirs}}) {
+ my $c = canonpath $d;
+ next if $c eq $d;
+ $cwarn = warn <<EOM if !$cwarn;
+E: BUG? paths in $self->{cidx_dir} not canonicalized:
+EOM
+ warn "E: $d => $c\n";
+ $d = $c;
}
if (defined(my $excl = $self->{-opt}->{exclude})) {
my $re = '(?:'.join('\\z|', map {
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/4] hval: use File::Spec::Functions::abs2rel
2024-11-12 20:34 [PATCH 0/4] File::Spec-related things Eric Wong
2024-11-12 20:34 ` [PATCH 1/4] cindex: rework path canonicalization check Eric Wong
2024-11-12 20:34 ` [PATCH 2/4] www_coderepo: drop unused File::Spec import Eric Wong
@ 2024-11-12 20:34 ` Eric Wong
2024-11-12 20:34 ` [PATCH 4/4] lei_mirror: favor File::Spec::Functions Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2024-11-12 20:34 UTC (permalink / raw)
To: meta
Function calls are faster than method dispatch and abs2rel() may
be called hundreds/thousands of times when listing associated
inboxes||coderepos.
---
lib/PublicInbox/Hval.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/Hval.pm b/lib/PublicInbox/Hval.pm
index 963dbb71..d44b6562 100644
--- a/lib/PublicInbox/Hval.pm
+++ b/lib/PublicInbox/Hval.pm
@@ -13,7 +13,7 @@ our @EXPORT_OK = qw/ascii_html obfuscate_addrs to_filename src_escape
to_attr prurl mid_href fmt_ts ts2str utf8_maybe/;
use POSIX qw(strftime);
my $enc_ascii = find_encoding('us-ascii');
-use File::Spec;
+use File::Spec::Functions qw(abs2rel);
# safe-ish acceptable filename pattern for portability
our $FN = '[a-zA-Z0-9][a-zA-Z0-9_\-\.]+[a-zA-Z0-9]'; # needs \z anchor
@@ -76,7 +76,7 @@ sub prurl ($$) {
} elsif ($dslash < 0 && substr($u, 0, 1) ne '/' &&
substr(my $path = $env->{PATH_INFO}, 0, 1) eq '/') {
# this won't touch the FS at all:
- File::Spec->abs2rel("/$u", $path);
+ abs2rel "/$u", $path;
} else {
$u;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/4] lei_mirror: favor File::Spec::Functions
2024-11-12 20:34 [PATCH 0/4] File::Spec-related things Eric Wong
` (2 preceding siblings ...)
2024-11-12 20:34 ` [PATCH 3/4] hval: use File::Spec::Functions::abs2rel Eric Wong
@ 2024-11-12 20:34 ` Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2024-11-12 20:34 UTC (permalink / raw)
To: meta
Functions calls are preferable over `->' method dispatch in
tight loops. This can be the case when scanning alternates in
v1_done(). Since we're at it, replace all other `File::Spec->'
method dispatches with function calls since function calls can
be used to validate function prototypes at compile time.
---
lib/PublicInbox/LeiMirror.pm | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index fb0295da..f87cdc51 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -11,7 +11,7 @@ use PublicInbox::Spawn qw(spawn run_wait run_die run_qx);
use PublicInbox::IO qw(write_file);
use File::Path ();
use File::Temp ();
-use File::Spec ();
+use File::Spec::Functions qw(abs2rel rel2abs splitpath);
use Fcntl qw(SEEK_SET O_CREAT O_EXCL O_WRONLY);
use Carp qw(croak);
use URI;
@@ -502,9 +502,9 @@ EOM
EOM
}
if (!$self->{dry_run}) {
- my $alt = File::Spec->rel2abs("$dir/objects");
+ my $alt = rel2abs "$dir/objects";
my $o = "$self->{cur_dst}/objects";
- my $l = File::Spec->abs2rel($alt, File::Spec->rel2abs($o));
+ my $l = abs2rel $alt, rel2abs($o);
open my $fh, '+>>', my $f = "$o/info/alternates";
seek($fh, 0, SEEK_SET); # Perl did SEEK_END when it saw '>>'
my $seen = grep /\A\Q$l\E\n/, PublicInbox::IO::read_all $fh;
@@ -780,15 +780,14 @@ sub update_ent {
start_cmd($self, $cmd, { 2 => $self->{lei}->{2} }) if $cmd;
}
if (my $symlinks = $self->{-ent}->{symlinks}) {
- my $top = File::Spec->rel2abs($self->{dst});
+ my $top = rel2abs $self->{dst};
push @{$self->{-new_symlinks}}, @$symlinks;
for my $p (@$symlinks) {
my $ln = "$top/$p";
$ln =~ tr!/!/!s;
- my (undef, $dn, $bn) = File::Spec->splitpath($ln);
+ my (undef, $dn, $bn) = splitpath $ln;
File::Path::mkpath($dn);
- my $tgt = "$top/$key";
- $tgt = File::Spec->abs2rel($tgt, $dn);
+ my $tgt = abs2rel "$top/$key", $dn;
if (lstat($ln)) {
if (-l _) {
next if readlink($ln) eq $tgt;
@@ -828,12 +827,12 @@ sub v1_done { # called via OnDestroy
update_ent($self) if $self->{-ent};
my $o = "$dst/objects";
if (CORE::open(my $fh, '<', my $fn = "$o/info/alternates")) {;
- my $base = File::Spec->rel2abs($o);
+ my $base = rel2abs $o;
my @l = <$fh>;
my $ft;
for (@l) {
next unless m!\A/!;
- $_ = File::Spec->abs2rel($_, $base);
+ $_ = abs2rel $_, $base;
$ft //= File::Temp->new(TEMPLATE => '.XXXX',
DIR => "$o/info");
}
@@ -1123,7 +1122,7 @@ EOM
}
$self->{chg}->{nr_chg} += scalar(@remote) + scalar(@local);
return if !defined($f) || $self->{dry_run};
- my (undef, $dn, $bn) = File::Spec->splitpath($f);
+ my (undef, $dn, $bn) = splitpath $f;
my $new = join("\n", @list, '');
atomic_write($dn, $bn, $new) if $new ne $old;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread