[PATCH 0/5] some minor cleanups and doc updates

[PATCH 0/5] some minor cleanups and doc updates

[Eric Wong]

Actual features coming, later, but some cleanups and internal
doc updates I found along the way.  I'm a tiny bit worried
about PATCH 3/5 breaking somebody's setup because of old
versions of examples/public-inbox.psgi, but considering how
little adoption this project had in 2016, maybe it's not a
big deal...

Eric Wong (5):
  wwwstatic: getline: die on missing psgix.io
  http: update comment about psgix.io usage
  githttpbackend: remove ancient compatibility check
  filter/base: export REJECT as a constant
  cgit: type declaration for PublicInbox::Git

 lib/PublicInbox/Cgit.pm           |  3 ++-
 lib/PublicInbox/Filter/Base.pm    |  2 ++
 lib/PublicInbox/GitHTTPBackend.pm |  3 ---
 lib/PublicInbox/HTTP.pm           |  2 +-
 lib/PublicInbox/InboxWritable.pm  |  3 +--
 lib/PublicInbox/WatchMaildir.pm   |  3 +--
 lib/PublicInbox/WwwStatic.pm      | 20 +++++++++-----------
 script/public-inbox-purge         |  4 +---
 8 files changed, 17 insertions(+), 23 deletions(-)

[PATCH 1/5] wwwstatic: getline: die on missing psgix.io

[Eric Wong]

"psgix." extensions aren't guaranteed, so make we should
try and support some theoretical generic PSGI servers
without "psgix.io" on errors by die-ing.

While we're at it, make the error handling path more obvious by
sharing more code between the EOF and errno ($!) cases.
---
 lib/PublicInbox/WwwStatic.pm | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/WwwStatic.pm b/lib/PublicInbox/WwwStatic.pm
index 76e50c78..58db58b4 100644
--- a/lib/PublicInbox/WwwStatic.pm
+++ b/lib/PublicInbox/WwwStatic.pm
@@ -77,25 +77,23 @@ sub response {
 # called by PSGI servers:
 sub getline {
 	my ($self) = @_;
-	my $len = $self->{len};
-	return if $len == 0;
+	my $len = $self->{len} or return; # undef, tells server we're done
 	my $n = delete($self->{initial_rd}) // 8192;
 	$n = $len if $len < $n;
 	my $r = sysread($self->{in}, my $buf, $n);
-	if (!defined $r) {
-		$self->{env}->{'psgi.errors'}->print(
-			"$self->{path} read error: $!\n");
-	} elsif ($r > 0) { # success!
+	if (defined $r && $r > 0) { # success!
 		$self->{len} = $len - $r;
 		return $buf;
-	} else {
-		$self->{env}->{'psgi.errors'}->print(
-			"$self->{path} EOF with $len bytes left\n");
 	}
+	my $m = defined $r ? "EOF with $len bytes left" : "read error: $!";
+	my $env = $self->{env};
+	$env->{'psgi.errors'}->print("$self->{path} $m\n");
 
 	# drop the client on error
-	if (my $io = $self->{env}->{'psgix.io'}) {
-		$io->close; # this is PublicInbox::DS::close
+	if (my $io = $env->{'psgix.io'}) {
+		$io->close; # this is likely PublicInbox::DS::close
+	} else { # for some PSGI servers w/o psgix.io
+		die "dropping client socket\n";
 	}
 	undef;
 }

[PATCH 2/5] http: update comment about psgix.io usage

[Eric Wong]

We've been using async_pass for a while.
---
 lib/PublicInbox/HTTP.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index e350daaf..d2f04ba5 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -164,7 +164,7 @@ sub app_dispatch {
 	}
 	# note: NOT $self->{sock}, we want our close (+ PublicInbox::DS::close),
 	# to do proper cleanup:
-	$env->{'psgix.io'} = $self; # only for ->close
+	$env->{'psgix.io'} = $self; # for ->close or async_pass
 	my $res = Plack::Util::run_app($self->{httpd}->{app}, $env);
 	eval {
 		if (ref($res) eq 'CODE') {

[PATCH 3/5] githttpbackend: remove ancient compatibility check

[Eric Wong]

The ref() call could be hitting memory leaks on Perl 5.16.x.

It's been 3 years (2016-12-25) since 292ca34140489da2
("githttpbackend: simplify compatibility code") back when
this project was barely known and probably nobody used
examples/public-inbox.psgi...
---
 lib/PublicInbox/GitHTTPBackend.pm | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index 8dd27a75..8883ec34 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -47,9 +47,6 @@ sub r ($;$) {
 sub serve {
 	my ($env, $git, $path) = @_;
 
-	# XXX compatibility... ugh, can we stop supporting this?
-	$git = PublicInbox::Git->new($git) unless ref($git);
-
 	# Documentation/technical/http-protocol.txt in git.git
 	# requires one and exactly one query parameter:
 	if ($env->{QUERY_STRING} =~ /\Aservice=git-[A-Za-z0-9_]+-pack\z/ ||

[PATCH 4/5] filter/base: export REJECT as a constant

[Eric Wong]

And update callers to use it, as it makes the code a bit cleaner.
Probably irrelvant, but it should be faster, too, as
"perl -I lib -w -MO=Deparse $FILE" shows REJECT() calls are
constant-folded.
---
 lib/PublicInbox/Filter/Base.pm   | 2 ++
 lib/PublicInbox/InboxWritable.pm | 3 +--
 lib/PublicInbox/WatchMaildir.pm  | 3 +--
 script/public-inbox-purge        | 4 +---
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Filter/Base.pm b/lib/PublicInbox/Filter/Base.pm
index 7a0c720f..81adab2a 100644
--- a/lib/PublicInbox/Filter/Base.pm
+++ b/lib/PublicInbox/Filter/Base.pm
@@ -6,6 +6,8 @@ package PublicInbox::Filter::Base;
 use strict;
 use warnings;
 use PublicInbox::MsgIter;
+use parent qw(Exporter);
+our @EXPORT_OK = qw(REJECT); # we may export IGNORE if/when needed
 
 sub No ($) { "*** We only accept plain-text mail, No $_[0] ***" }
 
diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
index d8391251..228e786c 100644
--- a/lib/PublicInbox/InboxWritable.pm
+++ b/lib/PublicInbox/InboxWritable.pm
@@ -7,8 +7,7 @@ use strict;
 use warnings;
 use base qw(PublicInbox::Inbox);
 use PublicInbox::Import;
-use PublicInbox::Filter::Base;
-*REJECT = *PublicInbox::Filter::Base::REJECT;
+use PublicInbox::Filter::Base qw(REJECT);
 
 use constant {
 	PERM_UMASK => 0,
diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 003baec9..7803b58a 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -10,9 +10,8 @@ use PublicInbox::MIME;
 use PublicInbox::Spawn qw(spawn);
 use PublicInbox::InboxWritable;
 use File::Temp 0.19 ();
-use PublicInbox::Filter::Base;
+use PublicInbox::Filter::Base qw(REJECT);
 use PublicInbox::Spamcheck;
-*REJECT = *PublicInbox::Filter::Base::REJECT;
 *maildir_path_load = *PublicInbox::InboxWritable::maildir_path_load;
 
 sub new {
diff --git a/script/public-inbox-purge b/script/public-inbox-purge
index f103ccf7..22b56a8e 100755
--- a/script/public-inbox-purge
+++ b/script/public-inbox-purge
@@ -9,12 +9,10 @@ use warnings;
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 use PublicInbox::AdminEdit;
 PublicInbox::Admin::check_require('-index');
-use PublicInbox::Filter::Base;
+use PublicInbox::Filter::Base qw(REJECT);
 use PublicInbox::MIME;
 require PublicInbox::V2Writable;
 
-*REJECT = \&PublicInbox::Filter::Base::REJECT;
-
 my $usage = "$0 [--all] [INBOX_DIRS] </path/to/message";
 my $opt = { verbose => 1, all => 0, -min_inbox_version => 2 };
 GetOptions($opt, @PublicInbox::AdminEdit::OPT) or

[PATCH 5/5] cgit: type declaration for PublicInbox::Git

[Eric Wong]

AFAIK this doesn't do anything for Perl internally since
PublicInbox::Git doesn't "use fields", but it makes it easier for
humans readers to follow and ensure we're not passing unblessed
or non-ref scalars to PublicInbox::GitHTTPBackend::serve.
---
 lib/PublicInbox/Cgit.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Cgit.pm b/lib/PublicInbox/Cgit.pm
index ab4065bd..36239438 100644
--- a/lib/PublicInbox/Cgit.pm
+++ b/lib/PublicInbox/Cgit.pm
@@ -8,6 +8,7 @@
 package PublicInbox::Cgit;
 use strict;
 use PublicInbox::GitHTTPBackend;
+use PublicInbox::Git;
 # not bothering with Exporter for a one-off
 *r = *PublicInbox::GitHTTPBackend::r;
 *input_prepare = *PublicInbox::GitHTTPBackend::input_prepare;
@@ -109,7 +110,7 @@ sub call {
 	# handle requests without spawning cgit iff possible:
 	if ($path_info =~ m!\A/(.+?)/($PublicInbox::GitHTTPBackend::ANY)\z!ox) {
 		my ($nick, $path) = ($1, $2);
-		if (my $git = $self->{"\0$nick"}) {
+		if (my PublicInbox::Git $git = $self->{"\0$nick"}) {
 			return serve($env, $git, $path);
 		}
 	} elsif ($path_info =~ m!$self->{static}! &&