[PATCH 0/4] various cleanups around http

[PATCH 0/4] various cleanups around http

[Eric Wong]

Eric Wong (4):
      githttpbackend: minor readability improvement
      githttpbackend: simplify compatibility code
      githttpbackend: minor cleanups to improve readability
      httpd/async: improve variable naming

 examples/public-inbox.psgi        |  1 +
 lib/PublicInbox/GitHTTPBackend.pm | 41 +++++++++++++++++----------------------
 lib/PublicInbox/HTTPD/Async.pm    | 14 ++++++-------
 3 files changed, 26 insertions(+), 30 deletions(-)

[PATCH 1/4] githttpbackend: minor readability improvement

[Eric Wong]

Use a more meaningful variable name for the Qspawn
object, since this module is the reference for its
use.
---
 lib/PublicInbox/GitHTTPBackend.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index a069fd9..86b8ebc 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -206,11 +206,11 @@ sub serve_smart {
 	}
 	$env{GIT_HTTP_EXPORT_ALL} = '1';
 	$env{PATH_TRANSLATED} = "$git_dir/$path";
-	my %rdr = ( 0 => fileno($in) );
-	my $x = PublicInbox::Qspawn->new([qw(git http-backend)], \%env, \%rdr);
+	my $rdr = { 0 => fileno($in) };
+	my $qsp = PublicInbox::Qspawn->new([qw(git http-backend)], \%env, $rdr);
 	my ($fh, $rpipe);
 	my $end = sub {
-		if (my $err = $x->finish) {
+		if (my $err = $qsp->finish) {
 			err($env, "git http-backend ($git_dir): $err");
 		}
 		$fh->close if $fh; # async-only
@@ -258,7 +258,7 @@ sub serve_smart {
 		# holding the input here is a waste of FDs and memory
 		$env->{'psgi.input'} = undef;
 
-		$x->start($limiter, sub { # may run later, much later...
+		$qsp->start($limiter, sub { # may run later, much later...
 			($rpipe) = @_;
 			$in = undef;
 			if ($async) {
-- 
EW

[PATCH 2/4] githttpbackend: simplify compatibility code

[Eric Wong]

Fewer conditionals means theres fewer code paths to test
and makes things easier-to-read.
---
 examples/public-inbox.psgi        |  1 +
 lib/PublicInbox/GitHTTPBackend.pm | 15 ++++++---------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/examples/public-inbox.psgi b/examples/public-inbox.psgi
index e97f917..98cde92 100644
--- a/examples/public-inbox.psgi
+++ b/examples/public-inbox.psgi
@@ -14,6 +14,7 @@ my $www = PublicInbox::WWW->new;
 
 # share the public-inbox code itself:
 my $src = $ENV{SRC_GIT_DIR}; # '/path/to/public-inbox.git'
+$src = PublicInbox::Git->new($src) if defined $src;
 
 builder {
 	eval {
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index 86b8ebc..4ad3fd1 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -46,6 +46,9 @@ 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-\w+-pack\z/ ||
@@ -98,7 +101,7 @@ sub serve_dumb {
 		return r(404);
 	}
 
-	my $f = (ref $git ? $git->{git_dir} : $git) . '/' . $path;
+	my $f = $git->{git_dir} . '/' . $path;
 	return r(404) unless -f $f && -r _; # just in case it's a FIFO :P
 	my $size = -s _;
 
@@ -196,14 +199,8 @@ sub serve_smart {
 		my $val = $env->{$name};
 		$env{$name} = $val if defined $val;
 	}
-	my ($git_dir, $limiter);
-	if (ref $git) {
-		$limiter = $git->{-httpbackend_limiter} || $default_limiter;
-		$git_dir = $git->{git_dir};
-	} else {
-		$limiter = $default_limiter;
-		$git_dir = $git;
-	}
+	my $limiter = $git->{-httpbackend_limiter} || $default_limiter;
+	my $git_dir = $git->{git_dir};
 	$env{GIT_HTTP_EXPORT_ALL} = '1';
 	$env{PATH_TRANSLATED} = "$git_dir/$path";
 	my $rdr = { 0 => fileno($in) };
-- 
EW

[PATCH 3/4] githttpbackend: minor cleanups to improve readability

[Eric Wong]

Fewer returns improves readability and the diffstat agrees.
---
 lib/PublicInbox/GitHTTPBackend.pm | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index 4ad3fd1..1fa5e30 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -225,7 +225,6 @@ sub serve_smart {
 	};
 	my $res;
 	my $async = $env->{'pi-httpd.async'}; # XXX unstable API
-	my $io = $env->{'psgix.io'};
 	my $cb = sub {
 		my $r = $rd_hdr->() or return;
 		$rd_hdr = undef;
@@ -236,17 +235,16 @@ sub serve_smart {
 				$rpipe->close;
 				$end->();
 			}
-			return $res->($r);
-		}
-		if ($async) {
+			$res->($r);
+		} elsif ($async) {
 			$fh = $res->($r);
-			return $async->async_pass($io, $fh, \$buf);
+			$async->async_pass($env->{'psgix.io'}, $fh, \$buf);
+		} else { # for synchronous PSGI servers
+			require PublicInbox::GetlineBody;
+			$r->[2] = PublicInbox::GetlineBody->new($rpipe, $end,
+								$buf);
+			$res->($r);
 		}
-
-		# for synchronous PSGI servers
-		require PublicInbox::GetlineBody;
-		$r->[2] = PublicInbox::GetlineBody->new($rpipe, $end, $buf);
-		$res->($r);
 	};
 	sub {
 		($res) = @_;
-- 
EW

[PATCH 4/4] httpd/async: improve variable naming

[Eric Wong]

We only refer to PublicInbox::HTTP objects here, so '$io'
was a bad name.
---
 lib/PublicInbox/HTTPD/Async.pm | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index 68514f5..79951ca 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -30,10 +30,10 @@ sub restart_read_cb ($) {
 }
 
 sub async_pass {
-	my ($self, $io, $fh, $bref) = @_;
-	# In case the client HTTP connection ($io) dies, it
+	my ($self, $http, $fh, $bref) = @_;
+	# In case the client HTTP connection ($http) dies, it
 	# will automatically close this ($self) object.
-	$io->{forward} = $self;
+	$http->{forward} = $self;
 	$fh->write($$bref);
 	my $restart_read = restart_read_cb($self);
 	weaken($self);
@@ -41,10 +41,10 @@ sub async_pass {
 		my $r = sysread($self->{sock}, $$bref, 8192);
 		if ($r) {
 			$fh->write($$bref);
-			return if $io->{closed};
-			if ($io->{write_buf_size}) {
+			return if $http->{closed};
+			if ($http->{write_buf_size}) {
 				$self->watch_read(0);
-				$io->write($restart_read); # D::S::write
+				$http->write($restart_read); # D::S::write
 			}
 			# stay in watch_read, but let other clients
 			# get some work done, too.
@@ -55,7 +55,7 @@ sub async_pass {
 
 		# Done! Error handling will happen in $fh->close
 		# called by the {cleanup} handler
-		$io->{forward} = undef;
+		$http->{forward} = undef;
 		$self->close;
 	}
 }
-- 
EW