[PATCH 0/2] www_coderepo: graceful dead branch handling

[PATCH 0/2] www_coderepo: graceful dead branch handling

[Eric Wong]

At least make stderr quieter (cgit doesn't spew to stderr on
dead branches, so we shouldn't, either).

Eric Wong (2):
  www_coderepo: quiet 404s on Atom feeds for dead branches
  www_coderepo: handle unborn/dead branches in summary

 lib/PublicInbox/RepoAtom.pm    |  3 ++-
 lib/PublicInbox/WwwCoderepo.pm | 15 ++++++++-------
 t/solver_git.t                 | 23 ++++++++++++-----------
 3 files changed, 22 insertions(+), 19 deletions(-)

[PATCH 1/2] www_coderepo: quiet 404s on Atom feeds for dead branches

[Eric Wong]

No need to clutter up logs when a request hits a dead branch.
---
 lib/PublicInbox/RepoAtom.pm    | 3 ++-
 lib/PublicInbox/WwwCoderepo.pm | 1 +
 t/solver_git.t                 | 5 ++++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/RepoAtom.pm b/lib/PublicInbox/RepoAtom.pm
index 44883ab4..c89d4551 100644
--- a/lib/PublicInbox/RepoAtom.pm
+++ b/lib/PublicInbox/RepoAtom.pm
@@ -120,7 +120,8 @@ sub srv_atom {
 	# else: let git decide based on HEAD if $tip isn't defined
 	push @cmd, '--';
 	push @cmd, $path if $path ne '';
-	my $qsp = PublicInbox::Qspawn->new(\@cmd);
+	my $qsp = PublicInbox::Qspawn->new(\@cmd, undef,
+					{ quiet => 1, 2 => $ctx->{lh} });
 	$qsp->psgi_return($ctx->{env}, undef, \&atom_ok, $ctx);
 }
 
diff --git a/lib/PublicInbox/WwwCoderepo.pm b/lib/PublicInbox/WwwCoderepo.pm
index 7a2cb80b..8a490b6c 100644
--- a/lib/PublicInbox/WwwCoderepo.pm
+++ b/lib/PublicInbox/WwwCoderepo.pm
@@ -308,6 +308,7 @@ sub srv { # endpoint called by PublicInbox::WWW
 		PublicInbox::RepoSnapshot::srv($ctx, $2) // r(404);
 	} elsif ($path_info =~ m!\A/(.+?)/atom/(.*)\z! and
 			($ctx->{git} = $cr->{$1})) {
+		$ctx->{lh} = $self->{log_fh};
 		PublicInbox::RepoAtom::srv_atom($ctx, $2) // r(404);
 	} elsif ($path_info =~ m!\A/(.+?)/tags\.atom\z! and
 			($ctx->{git} = $cr->{$1})) {
diff --git a/t/solver_git.t b/t/solver_git.t
index 0090bc06..79672398 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -379,7 +379,10 @@ EOF
 			ok($t->{feed}->{entry}, 'got entry');
 
 			$res = $cb->(GET('/public-inbox/atom/README.md'));
-			is($res->code, 404, '404 on non-existent file Atom feed');
+			is($res->code, 404, '404 on missing file Atom feed');
+
+			$res = $cb->(GET('/public-inbox/atom/?h=gone'));
+			is($res->code, 404, '404 on missing Atom feed branch');
 		}
 
 		$res = $cb->(GET('/public-inbox/tree/'));

[PATCH 2/2] www_coderepo: handle unborn/dead branches in summary

[Eric Wong]

We need to account for `git log' showing nothing for invalid
branches and continue to render properly.  We'll also quiet down
`git log' stderr to avoid cluttering stderr, too.
---
 lib/PublicInbox/WwwCoderepo.pm | 14 +++++++-------
 t/solver_git.t                 | 18 ++++++++----------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/lib/PublicInbox/WwwCoderepo.pm b/lib/PublicInbox/WwwCoderepo.pm
index 8a490b6c..629fad09 100644
--- a/lib/PublicInbox/WwwCoderepo.pm
+++ b/lib/PublicInbox/WwwCoderepo.pm
@@ -123,17 +123,16 @@ sub _refs_tags_link {
 sub summary_finish {
 	my ($ctx) = @_;
 	my $wcb = delete($ctx->{env}->{'qspawn.wcb'}) or return; # already done
-	my @x = split(/\n\n/sm, delete($ctx->{-each_refs}));
+	my @x = split(/\n\n/sm, delete($ctx->{-each_refs}), 3);
 	PublicInbox::WwwStream::html_init($ctx);
 	my $zfh = $ctx->zfh;
 
 	# git log
-	my @r = split(/\n/s, pop(@x) // '');
+	my @r = split(/\n/s, pop(@x));
 	my $last = scalar(@r) > $ctx->{wcr}->{summary_log} ? pop(@r) : undef;
 	my $tip_html = '';
-	if (defined(my $tip = $ctx->{qp}->{h})) {
-		$tip_html .= ' '.ascii_html($tip).' --';
-	}
+	my $tip = $ctx->{qp}->{h};
+	$tip_html .= ' '.ascii_html($tip).' --' if defined $tip;
 	print $zfh <<EOM;
 <pre><a id=log>\$</a> git log --pretty=format:'%h %s (%cs)%d'$tip_html
 EOM
@@ -146,7 +145,7 @@ EOM
 			" (", $cs, ")\n";
 		print $zfh "\t(", ascii_html($d), ")\n" if $d;
 	}
-	print $zfh "# no commits, yet\n" if !@r;
+	print $zfh '# no commits in `', ($tip//'HEAD'),"', yet\n\n" if !@r;
 	print $zfh "...\n" if $last;
 
 	# README
@@ -218,7 +217,8 @@ sub summary {
 		qq(git log -$nl --pretty=format:'%d %H %h %cs %s' "\$@" --));
 	push @cmd, '--', $tip if defined($tip);
 	my $qsp = PublicInbox::Qspawn->new(\@cmd,
-		{ GIT_DIR => $ctx->{git}->{git_dir} });
+		{ GIT_DIR => $ctx->{git}->{git_dir} },
+		{ quiet => 1, 2 => $self->{log_fh} });
 	$qsp->{qsp_err} = \($ctx->{-qsp_err} = '');
 	$tip //= 'HEAD';
 	my @try = ("$tip:README", "$tip:README.md"); # TODO: configurable
diff --git a/t/solver_git.t b/t/solver_git.t
index 79672398..c65d9785 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -330,18 +330,16 @@ EOF
 		defined($ENV{PLACK_TEST_EXTERNALSERVER_URI}) or
 			open STDERR, '>&', $olderr or xbail "open: $!";
 		is($res->code, 200, 'coderepo summary (binfoo)');
-		if (ok(-s "$tmpdir/stderr.log")) {
-			open my $fh, '<', "$tmpdir/stderr.log" or xbail $!;
-			my $s = do { local $/; <$fh> };
-			open $fh, '>', "$tmpdir/stderr.log" or xbail $!;
-			ok($s =~ s/^fatal: your current branch.*?\n//sm,
-				'got current branch warning');
-			ok($s =~ s/^.*? exit status=[1-9]+\n\z//sm,
-				'got exit status warning');
-			is($s, '', 'no unexpected warnings on empty coderepo');
-		}
+		ok(!-s "$tmpdir/stderr.log");
 		$res = $cb->(GET('/public-inbox/'));
 		is($res->code, 200, 'coderepo summary (public-inbox)');
+
+		my $tip = 'invalid-'.int(rand(0xdeadbeef));
+		$res = $cb->(GET('/public-inbox/?h='.$tip));
+		is($res->code, 200, 'coderepo summary on dead branch');
+		like($res->content, qr/no commits in `\Q$tip\E', yet/,
+			'lack of commits noted');
+
 		$res = $cb->(GET('/public-inbox'));
 		is($res->code, 301, 'redirected');
 

Re: [PATCH 2/2] www_coderepo: handle unborn/dead branches in summary

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> diff --git a/lib/PublicInbox/WwwCoderepo.pm b/lib/PublicInbox/WwwCoderepo.pm
> index 8a490b6c..629fad09 100644
> --- a/lib/PublicInbox/WwwCoderepo.pm
> +++ b/lib/PublicInbox/WwwCoderepo.pm
> @@ -218,7 +217,8 @@ sub summary {
>  		qq(git log -$nl --pretty=format:'%d %H %h %cs %s' "\$@" --));
>  	push @cmd, '--', $tip if defined($tip);

Looks like I need to squash this in for FreeBSD /bin/sh, too:

diff --git a/lib/PublicInbox/WwwCoderepo.pm b/lib/PublicInbox/WwwCoderepo.pm
index 629fad09..52ab6e48 100644
--- a/lib/PublicInbox/WwwCoderepo.pm
+++ b/lib/PublicInbox/WwwCoderepo.pm
@@ -215,7 +215,7 @@ sub summary {
 		"$EACH_REF --count=$nb refs/heads; echo && " .
 		"$EACH_REF --count=$nt refs/tags; echo && " .
 		qq(git log -$nl --pretty=format:'%d %H %h %cs %s' "\$@" --));
-	push @cmd, '--', $tip if defined($tip);
+	push @cmd, 'git', $tip if defined($tip);
 	my $qsp = PublicInbox::Qspawn->new(\@cmd,
 		{ GIT_DIR => $ctx->{git}->{git_dir} },
 		{ quiet => 1, 2 => $self->{log_fh} });

dash (on Debian) can handle the '--' as the command-name,
but /bin/sh seems to try interpreting it as a CLI switch.