[PATCH 0/3] www: configurable cgit fallback for coderepos

[PATCH 0/3] www: configurable cgit fallback for coderepos

[Eric Wong]

Since WwwCoderepo may not be to the tastes of everyone
(but certainly will be for me :>), we'll continue allowing (and
defaulting) to using cgit, first, for backwards compatibility.

IOW, we try cgit, and fall back to WwwCoderepo if cgit returns 404.

publicinbox.cgit=fallback swaps them: try WwwCoderepo, and fall
back to cgit if WwwCoderepo returns 404

In the future, publicinbox.cgit=rewrite will 301 cgit URLs to
equivalent WwwCoderepo URLs.

Eric Wong (3):
  www: do not call ->coderepo->srv on sub ref
  www: cgit: fall back to WwwCoderepo on 404s
  www: support publicinbox.cgit knob

 Documentation/public-inbox-config.pod | 21 ++++++++++++++++++
 lib/PublicInbox/Cgit.pm               |  5 +++--
 lib/PublicInbox/GitHTTPBackend.pm     | 19 +++++++++++++---
 lib/PublicInbox/Qspawn.pm             |  8 +++----
 lib/PublicInbox/WWW.pm                | 31 ++++++++++++++++-----------
 5 files changed, 62 insertions(+), 22 deletions(-)

[PATCH 1/3] www: do not call ->coderepo->srv on sub ref

[Eric Wong]

The PublicInbox::Cgit wrapper will return a sub-ref for most
responses, so ensure we don't try to treat it as an array-ref.
---
 lib/PublicInbox/WWW.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index d0e20fb5..470510ae 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -197,8 +197,9 @@ sub news_cgit_fallback ($) {
 	my $www = $ctx->{www};
 	my $env = $ctx->{env};
 	my $res = $www->news_www->call($env);
-	$res = $www->cgit->call($env) if $res->[0] == 404;
-	$res = $www->coderepo->srv($ctx) if $res->[0] == 404;
+	$res = $www->cgit->call($env, $ctx) if $res->[0] == 404;
+	ref($res) eq 'ARRAY' && $res->[0] == 404 and
+		$res = $www->coderepo->srv($ctx);
 	$res;
 }
 

[PATCH 2/3] www: cgit: fall back to WwwCoderepo on 404s

[Eric Wong]

We can't rely on 3-element array response when calling
WwwCoderepo for ViewVCS endpoints since that uses Qspawn
internally.  Thus, we have to allow two Qspawn objects to run in
parallel and ensure `qspawn.wcb' only gets called once, so we
end up duplicating the entire $ctx to ensure this.
---
 lib/PublicInbox/Cgit.pm           |  4 ++--
 lib/PublicInbox/GitHTTPBackend.pm | 19 ++++++++++++++++---
 lib/PublicInbox/Qspawn.pm         |  8 ++++----
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/Cgit.pm b/lib/PublicInbox/Cgit.pm
index 1112d9f8..298663c7 100644
--- a/lib/PublicInbox/Cgit.pm
+++ b/lib/PublicInbox/Cgit.pm
@@ -83,7 +83,7 @@ my @PASS_ENV = qw(
 my $parse_cgi_headers = \&PublicInbox::GitHTTPBackend::parse_cgi_headers;
 
 sub call {
-	my ($self, $env) = @_;
+	my ($self, $env, $ctx) = @_; # $ctx is optional, used by WWW
 	my $path_info = $env->{PATH_INFO};
 	my $cgit_data;
 
@@ -109,7 +109,7 @@ sub call {
 	my $rdr = input_prepare($env) or return r(500);
 	my $qsp = PublicInbox::Qspawn->new($self->{cmd}, $cgi_env, $rdr);
 	my $limiter = $self->{pi_cfg}->limiter('-cgit');
-	$qsp->psgi_return($env, $limiter, $parse_cgi_headers);
+	$qsp->psgi_return($env, $limiter, $parse_cgi_headers, $ctx);
 }
 
 1;
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index ba3a8f20..61a13560 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # when no endpoints match, fallback to this and serve a static file
@@ -132,7 +132,7 @@ sub input_prepare {
 }
 
 sub parse_cgi_headers {
-	my ($r, $bref) = @_;
+	my ($r, $bref, $ctx) = @_;
 	return r(500) unless defined $r && $r >= 0;
 	$$bref =~ s/\A(.*?)\r?\n\r?\n//s or return $r == 0 ? r(500) : undef;
 	my $h = $1;
@@ -146,7 +146,20 @@ sub parse_cgi_headers {
 			push @h, $k, $v;
 		}
 	}
-	[ $code, \@h ]
+
+	# fallback to WwwCoderepo if cgit 404s.  Duplicating $ctx prevents
+	# ->finalize from the current Qspawn from using qspawn.wcb
+	if ($code == 404 && $ctx->{www} && !$ctx->{_coderepo_tried}++) {
+		my %ctx = %$ctx;
+		$ctx{env} = +{ %{$ctx->{env}} };
+		delete $ctx->{env}->{'qspawn.wcb'};
+		$ctx->{env}->{'plack.skip-deflater'} = 1; # prevent 2x gzip
+		my $res = $ctx->{www}->coderepo->srv(\%ctx);
+		$res->(delete $ctx{env}->{'qspawn.wcb'}) if ref($res) eq 'CODE';
+		$res; # non ARRAY ref for ->psgi_return_init_cb
+	} else {
+		[ $code, \@h ]
+	}
 }
 
 1;
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index cea34fc3..ef9db43e 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -225,19 +225,19 @@ sub psgi_return_init_cb {
 	my ($self) = @_;
 	my $r = rd_hdr($self) or return;
 	my $env = $self->{psgi_env};
-	my $filter = delete $env->{'qspawn.filter'} //
-		PublicInbox::GzipFilter::qsp_maybe($r->[1], $env);
+	my $filter = delete($env->{'qspawn.filter'}) // (ref($r) eq 'ARRAY' ?
+		PublicInbox::GzipFilter::qsp_maybe($r->[1], $env) : undef);
 
 	my $wcb = delete $env->{'qspawn.wcb'};
 	my $async = delete $self->{async}; # PublicInbox::HTTPD::Async
-	if (scalar(@$r) == 3) { # error
+	if (ref($r) ne 'ARRAY' || scalar(@$r) == 3) { # error
 		if ($async) { # calls rpipe->close && ->event_step
 			$async->close; # PublicInbox::HTTPD::Async::close
 		} else {
 			$self->{rpipe}->close;
 			event_step($self);
 		}
-		$wcb->($r);
+		$wcb->($r) if ref($r) eq 'ARRAY';
 	} elsif ($async) {
 		# done reading headers, handoff to read body
 		my $fh = $wcb->($r); # scalar @$r == 2

[PATCH 3/3] www: support publicinbox.cgit knob

[Eric Wong]

For backwards-compatibility, this defaults to `first'.  When set
to `fallback', PublicInbox::WwwCoderepo is favored and cgit is
only used as a fallback.  Eventually, `rewrite' will also be
supported to rewrite cgit URLs to WwwCoderepo ones.

Of course, WwwCoderepo is still missing search and other key
features, but that's being worked on...
---
 Documentation/public-inbox-config.pod | 21 +++++++++++++++++++
 lib/PublicInbox/Cgit.pm               |  1 +
 lib/PublicInbox/WWW.pm                | 30 +++++++++++++++------------
 3 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
index d8504e61..88403100 100644
--- a/Documentation/public-inbox-config.pod
+++ b/Documentation/public-inbox-config.pod
@@ -293,6 +293,27 @@ C<publicinbox.cgitbin>, but may be overridden.
 Default: basename of C<publicinbox.cgitbin>, /var/www/htdocs/cgit/
 or /usr/share/cgit/
 
+=item publicinbox.cgit
+
+=over 8
+
+=item * first
+
+Try using C<cgit> as the first choice, this is the default.
+
+=item * fallback
+
+Fall back to using C<cgit> only if our native, inbox-aware
+git code repository viewer doesn't recognized the URL.
+
+=item * rewrite
+
+Rewrite C<cgit> URLs for our native, inbox-aware code repository viewer.
+This implies C<fallback> for URLs the native viewer does not recognize.
+
+Default: C<first>  (C<cgit> will be used iff C<publicinbox.cgitrc>
+is set and the C<cgit> binary exists).
+
 =item publicinbox.mailEditor
 
 See L<public-inbox-edit(1)>
diff --git a/lib/PublicInbox/Cgit.pm b/lib/PublicInbox/Cgit.pm
index 298663c7..336098ca 100644
--- a/lib/PublicInbox/Cgit.pm
+++ b/lib/PublicInbox/Cgit.pm
@@ -53,6 +53,7 @@ sub locate_cgit ($) {
 sub new {
 	my ($class, $pi_cfg) = @_;
 	my ($cgit_bin, $cgit_data) = locate_cgit($pi_cfg);
+	$cgit_bin // return; # fall back in WWW->cgit
 	my $self = bless {
 		cmd => [ $cgit_bin ],
 		cgit_data => $cgit_data,
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 470510ae..f861b192 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -194,12 +194,19 @@ sub r404 {
 
 sub news_cgit_fallback ($) {
 	my ($ctx) = @_;
-	my $www = $ctx->{www};
-	my $env = $ctx->{env};
-	my $res = $www->news_www->call($env);
-	$res = $www->cgit->call($env, $ctx) if $res->[0] == 404;
+	my $res = $ctx->{www}->news_www->call($ctx->{env});
+
+	$res->[0] == 404 and ($ctx->{www}->{cgit_fallback} //= do {
+		my $c = $ctx->{www}->{pi_cfg}->{'publicinbox.cgit'} // 'first';
+		$c ne 'first' # `fallback' and `rewrite' => true
+	} // 0) and $res = $ctx->{www}->coderepo->srv($ctx);
+
 	ref($res) eq 'ARRAY' && $res->[0] == 404 and
-		$res = $www->coderepo->srv($ctx);
+		$res = $ctx->{www}->cgit->call($ctx->{env}, $ctx);
+
+	ref($res) eq 'ARRAY' && $res->[0] == 404 &&
+			!$ctx->{www}->{cgit_fallback} and
+		$res = $ctx->{www}->coderepo->srv($ctx);
 	$res;
 }
 
@@ -484,17 +491,14 @@ sub news_www {
 
 sub cgit {
 	my ($self) = @_;
-	$self->{cgit} //= do {
-		my $pi_cfg = $self->{pi_cfg};
-
-		if (defined($pi_cfg->{'publicinbox.cgitrc'})) {
+	$self->{cgit} //=
+		(defined($self->{pi_cfg}->{'publicinbox.cgitrc'}) ? do {
 			require PublicInbox::Cgit;
-			PublicInbox::Cgit->new($pi_cfg);
-		} else {
+			PublicInbox::Cgit->new($self->{pi_cfg});
+		} : undef) // do {
 			require Plack::Util;
 			Plack::Util::inline_object(call => sub { r404() });
-		}
-	}
+		};
 }
 
 sub coderepo {

[squash 4/3] manpage fix (was: [PATCH 3/3] www: support publicinbox.cgit knob)

[Eric Wong]

Will squash the following into the docs, since my mind was
nearly passed out by the time I wrote the manpage entry :X
---
 Documentation/public-inbox-config.pod | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
index 88403100..e926a27b 100644
--- a/Documentation/public-inbox-config.pod
+++ b/Documentation/public-inbox-config.pod
@@ -295,6 +295,9 @@ or /usr/share/cgit/
 
 =item publicinbox.cgit
 
+Controls whether or not and how C<cgit> is used for serving coderepos.
+New in public-inbox 2.0.0 (PENDING).
+
 =over 8
 
 =item * first
@@ -311,6 +314,8 @@ git code repository viewer doesn't recognized the URL.
 Rewrite C<cgit> URLs for our native, inbox-aware code repository viewer.
 This implies C<fallback> for URLs the native viewer does not recognize.
 
+=back
+
 Default: C<first>  (C<cgit> will be used iff C<publicinbox.cgitrc>
 is set and the C<cgit> binary exists).