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(-)
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; }
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
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 {
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).