* [PATCH 0/5] misc cleanups and bugfixes
@ 2020-01-10 9:14 Eric Wong
2020-01-10 9:14 ` [PATCH 1/5] inbox: use PublicInbox::Git::host_prefix_url for base_url Eric Wong
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
Some minor things, probably unlikely to matter a lot
but we can trim some code a little.
Eric Wong (5):
inbox: use PublicInbox::Git::host_prefix_url for base_url
allow HTTP_HOST to be '0' via defined() checks
git: ->modified uses cat_async
git: remove ->commit_title method
spawn (and thus popen_rd) die on failure
lib/PublicInbox/Config.pm | 2 +-
lib/PublicInbox/Git.pm | 24 +++++++++++-------------
lib/PublicInbox/HTTP.pm | 2 +-
lib/PublicInbox/Import.pm | 2 --
lib/PublicInbox/Inbox.pm | 30 +++++++++++++-----------------
lib/PublicInbox/SearchIdx.pm | 1 -
lib/PublicInbox/Spamcheck/Spamc.pm | 1 -
lib/PublicInbox/Spawn.pm | 1 -
lib/PublicInbox/TestCommon.pm | 1 -
lib/PublicInbox/V2Writable.pm | 1 -
lib/PublicInbox/WatchMaildir.pm | 1 -
lib/PublicInbox/WwwListing.pm | 4 +---
t/check-www-inbox.perl | 1 -
t/solver_git.t | 8 +-------
14 files changed, 28 insertions(+), 51 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/5] inbox: use PublicInbox::Git::host_prefix_url for base_url
2020-01-10 9:14 [PATCH 0/5] misc cleanups and bugfixes Eric Wong
@ 2020-01-10 9:14 ` Eric Wong
2020-01-10 9:14 ` [PATCH 2/5] allow HTTP_HOST to be '0' via defined() checks Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
Better not to duplicate the same logic across different classes.
Also, our git wrapper class is a strange place for
host_prefix_url, but it needs to be usable for coderepos, so
it's there, for now...
---
lib/PublicInbox/Inbox.pm | 30 +++++++++++++-----------------
t/solver_git.t | 2 +-
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index a3cdcbc0..ff800965 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -240,26 +240,22 @@ sub cloneurl {
}
sub base_url {
- my ($self, $env) = @_;
- my $scheme;
- if ($env && ($scheme = $env->{'psgi.url_scheme'})) { # PSGI env
- my $host_port = $env->{HTTP_HOST} ||
- "$env->{SERVER_NAME}:$env->{SERVER_PORT}";
- my $url = "$scheme://$host_port". ($env->{SCRIPT_NAME} || '/');
+ my ($self, $env) = @_; # env - PSGI env
+ if ($env) {
+ my $url = PublicInbox::Git::host_prefix_url($env, '');
# for mount in Plack::Builder
$url .= '/' if $url !~ m!/\z!;
- $url .= $self->{name} . '/';
- } else {
- # either called from a non-PSGI environment (e.g. NNTP/POP3)
- $self->{-base_url} ||= do {
- my $url = $self->{url}->[0] or return undef;
- # expand protocol-relative URLs to HTTPS if we're
- # not inside a web server
- $url = "https:$url" if $url =~ m!\A//!;
- $url .= '/' if $url !~ m!/\z!;
- $url;
- };
+ return $url .= $self->{name} . '/';
}
+ # called from a non-PSGI environment (e.g. NNTP/POP3):
+ $self->{-base_url} ||= do {
+ my $url = $self->{url}->[0] or return undef;
+ # expand protocol-relative URLs to HTTPS if we're
+ # not inside a web server
+ $url = "https:$url" if $url =~ m!\A//!;
+ $url .= '/' if $url !~ m!/\z!;
+ $url;
+ };
}
sub nntp_url {
diff --git a/t/solver_git.t b/t/solver_git.t
index 55746994..7c5619a7 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -48,7 +48,7 @@ $ibx->{-repo_objs} = [ $git ];
my $res;
my $solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
open my $log, '+>>', "$inboxdir/solve.log" or die "open: $!";
-my $psgi_env = { 'psgi.errors' => *STDERR };
+my $psgi_env = { 'psgi.errors' => *STDERR, 'psgi.url_scheme' => 'http' };
$solver->solve($psgi_env, $log, '69df7d5', {});
ok($res, 'solved a blob!');
my $wt_git = $res->[0];
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/5] allow HTTP_HOST to be '0' via defined() checks
2020-01-10 9:14 [PATCH 0/5] misc cleanups and bugfixes Eric Wong
2020-01-10 9:14 ` [PATCH 1/5] inbox: use PublicInbox::Git::host_prefix_url for base_url Eric Wong
@ 2020-01-10 9:14 ` Eric Wong
2020-01-10 9:14 ` [PATCH 3/5] git: ->modified uses cat_async Eric Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
'0' is a valid value for HTTP_HOST, and maybe some folks
will want to hit that as port 80 where the HTTP client won't
send the ":$PORT" suffix.
---
lib/PublicInbox/Git.pm | 2 +-
lib/PublicInbox/HTTP.pm | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 6a527f82..2aaf1866 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -300,7 +300,7 @@ sub host_prefix_url ($$) {
my ($env, $url) = @_;
return $url if index($url, '//') >= 0;
my $scheme = $env->{'psgi.url_scheme'};
- my $host_port = $env->{HTTP_HOST} ||
+ my $host_port = $env->{HTTP_HOST} //
"$env->{SERVER_NAME}:$env->{SERVER_PORT}";
"$scheme://$host_port". ($env->{SCRIPT_NAME} || '/') . $url;
}
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index a6ec1d0d..071251c6 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -155,7 +155,7 @@ sub app_dispatch {
my $env = $self->{env};
$env->{REMOTE_ADDR} = $self->{remote_addr};
$env->{REMOTE_PORT} = $self->{remote_port};
- if (my $host = $env->{HTTP_HOST}) {
+ if (defined(my $host = $env->{HTTP_HOST})) {
$host =~ s/:([0-9]+)\z// and $env->{SERVER_PORT} = $1;
$env->{SERVER_NAME} = $host;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/5] git: ->modified uses cat_async
2020-01-10 9:14 [PATCH 0/5] misc cleanups and bugfixes Eric Wong
2020-01-10 9:14 ` [PATCH 1/5] inbox: use PublicInbox::Git::host_prefix_url for base_url Eric Wong
2020-01-10 9:14 ` [PATCH 2/5] allow HTTP_HOST to be '0' via defined() checks Eric Wong
@ 2020-01-10 9:14 ` Eric Wong
2020-01-10 9:14 ` [PATCH 4/5] git: remove ->commit_title method Eric Wong
2020-01-10 9:14 ` [PATCH 5/5] spawn (and thus popen_rd) die on failure Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
While v1 inboxes are typically only a single branch, coderepos
will have many branches and being able to pipeline requests
to "git cat-file --batch" can help us mask seek times.
---
lib/PublicInbox/Git.pm | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 2aaf1866..9d0f660b 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -339,6 +339,15 @@ sub commit_title ($$) {
($$buf =~ /\r?\n\r?\n([^\r\n]+)\r?\n?/)[0]
}
+sub extract_cmt_time {
+ my ($bref, undef, undef, undef, $modified) = @_;
+
+ if ($$bref =~ /^committer .*?> ([0-9]+) [\+\-]?[0-9]+/sm) {
+ my $cmt_time = $1 + 0;
+ $$modified = $cmt_time if $cmt_time > $$modified;
+ }
+}
+
# returns the modified time of a git repo, same as the "modified" field
# of a grokmirror manifest
sub modified ($) {
@@ -346,14 +355,13 @@ sub modified ($) {
my $modified = 0;
my $fh = popen($self, qw(rev-parse --branches));
defined $fh or return $modified;
+ cat_async_begin($self);
local $/ = "\n";
foreach my $oid (<$fh>) {
chomp $oid;
- my $buf = cat_file($self, $oid) or next;
- $$buf =~ /^committer .*?> ([0-9]+) [\+\-]?[0-9]+/sm or next;
- my $cmt_time = $1 + 0;
- $modified = $cmt_time if $cmt_time > $modified;
+ cat_async($self, $oid, \&extract_cmt_time, \$modified);
}
+ cat_async_wait($self);
$modified || time;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/5] git: remove ->commit_title method
2020-01-10 9:14 [PATCH 0/5] misc cleanups and bugfixes Eric Wong
` (2 preceding siblings ...)
2020-01-10 9:14 ` [PATCH 3/5] git: ->modified uses cat_async Eric Wong
@ 2020-01-10 9:14 ` Eric Wong
2020-01-10 9:14 ` [PATCH 5/5] spawn (and thus popen_rd) die on failure Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
We haven't used it in SolverGit, yet, and I'll be reworking it
to work with ->cat_async, instead.
---
lib/PublicInbox/Git.pm | 7 -------
t/solver_git.t | 6 ------
2 files changed, 13 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 9d0f660b..f3b7a0a0 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -332,13 +332,6 @@ sub cat_async ($$$;$) {
push(@$inflight, [ $cb, $arg ]);
}
-sub commit_title ($$) {
- my ($self, $oid) = @_; # PublicInbox::Git, $sha1hex
- my $buf = cat_file($self, $oid) or return;
- utf8::decode($$buf);
- ($$buf =~ /\r?\n\r?\n([^\r\n]+)\r?\n?/)[0]
-}
-
sub extract_cmt_time {
my ($bref, undef, undef, undef, $modified) = @_;
diff --git a/t/solver_git.t b/t/solver_git.t
index 7c5619a7..67ae02e6 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -38,12 +38,6 @@ $deliver_patch->('t/solve/0001-simple-mod.patch');
my $v1_0_0_tag = 'cb7c42b1e15577ed2215356a2bf925aef59cdd8d';
my $git = PublicInbox::Git->new($git_dir);
-is('public-inbox 1.0.0',
- $git->commit_title($v1_0_0_tag),
- 'commit_title works on 1.0.0');
-
-is(undef, $git->commit_title('impossible'), 'undef on impossible object');
-
$ibx->{-repo_objs} = [ $git ];
my $res;
my $solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/5] spawn (and thus popen_rd) die on failure
2020-01-10 9:14 [PATCH 0/5] misc cleanups and bugfixes Eric Wong
` (3 preceding siblings ...)
2020-01-10 9:14 ` [PATCH 4/5] git: remove ->commit_title method Eric Wong
@ 2020-01-10 9:14 ` Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
Most spawn and popen_rd callers die on failure to spawn,
anyways, and some are missing checks entirely. This saves
us a bunch of verbose error-checking code in callers.
This also makes popen_rd more consistent, since it already
dies on pipe creation failures.
---
lib/PublicInbox/Config.pm | 2 +-
lib/PublicInbox/Git.pm | 3 ---
lib/PublicInbox/Import.pm | 2 --
lib/PublicInbox/SearchIdx.pm | 1 -
lib/PublicInbox/Spamcheck/Spamc.pm | 1 -
lib/PublicInbox/Spawn.pm | 1 -
lib/PublicInbox/TestCommon.pm | 1 -
lib/PublicInbox/V2Writable.pm | 1 -
lib/PublicInbox/WatchMaildir.pm | 1 -
lib/PublicInbox/WwwListing.pm | 4 +---
t/check-www-inbox.perl | 1 -
11 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index cc8c1eaf..56d146c2 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -158,7 +158,7 @@ sub git_config_dump {
return {} unless -e $file;
my @cmd = (qw/git config -z -l/, "--file=$file");
my $cmd = join(' ', @cmd);
- my $fh = popen_rd(\@cmd) or die "popen_rd failed for $file: $!\n";
+ my $fh = popen_rd(\@cmd);
my $rv = config_fh_parse($fh, "\0", "\n");
close $fh or die "failed to close ($cmd) pipe: $?";
$rv;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index f3b7a0a0..8ee04e17 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -122,7 +122,6 @@ sub _bidi_pipe {
$redir->{2} = $fh;
}
my $p = spawn(\@cmd, undef, $redir);
- defined $p or fail($self, "spawn failed: $!");
$self->{$pid} = $p;
$out_w->autoflush(1);
$self->{$out} = $out_w;
@@ -256,7 +255,6 @@ sub popen {
sub qx {
my ($self, @cmd) = @_;
my $fh = $self->popen(@cmd);
- defined $fh or return;
local $/ = "\n";
return <$fh> if wantarray;
local $/;
@@ -347,7 +345,6 @@ sub modified ($) {
my ($self) = @_;
my $modified = 0;
my $fh = popen($self, qw(rev-parse --branches));
- defined $fh or return $modified;
cat_async_begin($self);
local $/ = "\n";
foreach my $oid (<$fh>) {
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 572e9bb9..6ac43d37 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -68,7 +68,6 @@ sub gfi_start {
--quiet --done --date-format=raw));
my $rdr = { 0 => $out_r, 1 => $in_w };
my $pid = spawn(\@cmd, undef, $rdr);
- die "spawn fast-import failed: $!" unless defined $pid;
$out_w->autoflush(1);
$self->{in} = $in_r;
$self->{out} = $out_w;
@@ -430,7 +429,6 @@ sub add {
sub run_die ($;$$) {
my ($cmd, $env, $rdr) = @_;
my $pid = spawn($cmd, $env, $rdr);
- defined $pid or die "spawning ".join(' ', @$cmd)." failed: $!";
waitpid($pid, 0) == $pid or die join(' ', @$cmd) .' did not finish';
$? == 0 or die join(' ', @$cmd) . " failed: $?\n";
}
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index f14809d2..cb554912 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -666,7 +666,6 @@ sub is_ancestor ($$$) {
my $cmd = [ 'git', "--git-dir=$git->{git_dir}",
qw(merge-base --is-ancestor), $cur, $tip ];
my $pid = spawn($cmd);
- defined $pid or die "spawning ".join(' ', @$cmd)." failed: $!";
waitpid($pid, 0) == $pid or die join(' ', @$cmd) .' did not finish';
$? == 0;
}
diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm
index bb288b16..d9cc47e3 100644
--- a/lib/PublicInbox/Spamcheck/Spamc.pm
+++ b/lib/PublicInbox/Spamcheck/Spamc.pm
@@ -23,7 +23,6 @@ sub spamcheck {
my $rdr = { 0 => _msg_to_fh($self, $msg) };
my ($fh, $pid) = popen_rd($self->{checkcmd}, undef, $rdr);
- defined $pid or die "failed to popen_rd spamc: $!\n";
my $r;
unless (ref $out) {
my $buf = '';
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 1c74a596..b02d5368 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -219,7 +219,6 @@ sub popen_rd {
$opts ||= {};
$opts->{1} = fileno($w);
my $pid = spawn($cmd, $env, $opts);
- return unless defined $pid;
return ($r, $pid) if wantarray;
my $ret = gensym;
tie *$ret, 'PublicInbox::ProcessPipe', $pid, $r;
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index d6d1e939..b3c95612 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -215,7 +215,6 @@ sub run_script ($;$$) {
require PublicInbox::Spawn;
my $cmd = [ key2script($key), @argv ];
my $pid = PublicInbox::Spawn::spawn($cmd, $env, $spawn_opt);
- defined($pid) or die "spawn: $!";
if (defined $pid) {
my $r = waitpid($pid, 0);
defined($r) or die "waitpid: $!";
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 6021de44..51794326 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -777,7 +777,6 @@ sub diff ($$$) {
my $cmd = [ qw(diff -u), $an, $bn ];
print STDERR "# MID conflict <$mid>\n";
my $pid = spawn($cmd, undef, { 1 => 2 });
- defined $pid or die "diff failed to spawn $!";
waitpid($pid, 0) == $pid or die "diff did not finish";
unlink($an, $bn);
}
diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 8a8c1262..dfb987e8 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -7,7 +7,6 @@ package PublicInbox::WatchMaildir;
use strict;
use warnings;
use PublicInbox::MIME;
-use PublicInbox::Spawn qw(spawn);
use PublicInbox::InboxWritable;
use File::Temp 0.19 (); # 0.19 for ->newdir
use PublicInbox::Filter::Base qw(REJECT);
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index a52dba11..8d610037 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -136,9 +136,7 @@ sub fingerprint ($) {
my ($git) = @_;
# TODO: convert to qspawn for fairness when there's
# thousands of repos
- my ($fh, $pid) = $git->popen('show-ref') or
- die "popen($git->{git_dir} show-ref) failed: $!";
-
+ my ($fh, $pid) = $git->popen('show-ref');
my $dig = Digest::SHA->new(1);
while (read($fh, my $buf, 65536)) {
$dig->add($buf);
diff --git a/t/check-www-inbox.perl b/t/check-www-inbox.perl
index db292c50..40209957 100644
--- a/t/check-www-inbox.perl
+++ b/t/check-www-inbox.perl
@@ -48,7 +48,6 @@ my $atom_check = eval {
2 => fileno($err_fh),
};
my $pid = spawn($cmd, undef, $rdr);
- defined $pid or die "spawn failure: $!";
while (waitpid($pid, 0) != $pid) {
next if $!{EINTR};
warn "waitpid(xmlstarlet, $pid) $!";
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-10 9:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 9:14 [PATCH 0/5] misc cleanups and bugfixes Eric Wong
2020-01-10 9:14 ` [PATCH 1/5] inbox: use PublicInbox::Git::host_prefix_url for base_url Eric Wong
2020-01-10 9:14 ` [PATCH 2/5] allow HTTP_HOST to be '0' via defined() checks Eric Wong
2020-01-10 9:14 ` [PATCH 3/5] git: ->modified uses cat_async Eric Wong
2020-01-10 9:14 ` [PATCH 4/5] git: remove ->commit_title method Eric Wong
2020-01-10 9:14 ` [PATCH 5/5] spawn (and thus popen_rd) die on failure Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).