From: Eric Wong <e@80x24.org>
Date: Mon, 30 Sep 2024 21:30:06 +0000 (+0000)
Subject: www: improve handling of missing coderepos
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dbaf5742861e7748af3319a241306d5ede903d26;p=thirdparty%2Fpublic-inbox.git

www: improve handling of missing coderepos

Git coderepos may appear and disappear during the lifetime of
an -httpd or -netd instance.  This happens quite frequently on
on my git.kernel.org mirror and is and clutters up stderr, so
we'll validate the existance of a git directory before trying
to serve anything.

Whether or not this should be the case for inboxes is yet-to-be
decided, especially since there's inbox-specific information
inside in PI_CONFIG (e.g. address, newsgroup) and we only use a
project listing for coderepos.
---

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index d76deca40..442759dec 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -375,7 +375,9 @@ sub valid_dir ($$) {
 sub fill_coderepo {
 	my ($self, $nick) = @_;
 	my $pfx = "coderepo.$nick";
-	my $git = PublicInbox::Git->new(valid_dir($self, "$pfx.dir") // return);
+	my $git_dir = valid_dir($self, "$pfx.dir") // return;
+	-e $git_dir // return;
+	my $git = PublicInbox::Git->new($git_dir);
 	if (defined(my $cgits = $self->{"$pfx.cgiturl"})) {
 		$git->{cgit_url} = $cgits = _array($cgits);
 		$self->{"$pfx.cgiturl"} = $cgits;
@@ -701,13 +703,14 @@ sub glob2re ($) {
 
 sub get_coderepo {
 	my ($self, $nick) = @_;
-	$self->{-coderepos}->{$nick} // do {
+	my $git = $self->{-coderepos}->{$nick} // do {
 		defined($self->{-cgit_scan_path}) ? do {
 			apply_cgit_scan_path($self);
 			my $cr = fill_coderepo($self, $nick);
 			$cr ? ($self->{-coderepos}->{$nick} = $cr) : undef;
 		} : undef;
 	};
+	$git && -e $git->{git_dir} ? $git : undef;
 }
 
 1;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index c43f98632..b3f39adf8 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -250,7 +250,9 @@ sub cat_async_step ($$) {
 	my $cmd = ref($req) ? $$req : $req;
 	# ->fail may be called via Gcf2Client.pm
 	my $info = $self->{-bc} && substr($cmd, 0, 5) eq 'info ';
-	if ($head =~ /^([0-9a-f]{40,}) (\S+) ([0-9]+)$/) {
+	if (!defined $head) {
+		$self->fail("E: $self->{git_dir} gone? (\$!=$!)");
+	} elsif ($head =~ /^([0-9a-f]{40,}) (\S+) ([0-9]+)$/) {
 		($oid, $type, $size) = ($1, $2, $3 + 0);
 		unless ($info) { # --batch-command
 			$bref = $self->{sock}->my_bufread($size + 1) or
diff --git a/t/config.t b/t/config.t
index c41a42d36..956ad35c4 100644
--- a/t/config.t
+++ b/t/config.t
@@ -7,7 +7,7 @@ use_ok 'PublicInbox';
 ok(defined(eval('$PublicInbox::VERSION')), 'VERSION defined');
 use_ok 'PublicInbox::Config';
 my ($tmpdir, $for_destroy) = tmpdir();
-use autodie qw(open close);
+use autodie qw(close mkdir open);
 my $validate_git_behavior = $ENV{TEST_VALIDATE_GIT_BEHAVIOR};
 
 {
@@ -240,8 +240,9 @@ for my $s (@valid) {
 	inboxdir = /path/to/foo
 	coderepo = project
 [coderepo "project"]
-	dir = /path/to/project.git
+	dir = $tmpdir/project.git
 EOF
+	mkdir "$tmpdir/project.git"; # must exist for ->fill_coderepo
 	my $t1 = $cfg->lookup_name('test1');
 	my $t2 = $cfg->lookup_name('test2');
 	ok $cfg->repo_objs($t1)->[0], 'coderepo parsed';
diff --git a/t/solver_git.t b/t/solver_git.t
index 694e2671f..6cc5eeba2 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -6,8 +6,11 @@ use PublicInbox::TestCommon;
 use Cwd qw(abs_path);
 require_git v2.6;
 use PublicInbox::ContentHash qw(git_sha);
-use PublicInbox::Spawn qw(run_qx);
-require_mods(qw(DBD::SQLite Xapian));
+use PublicInbox::Spawn qw(run_qx which);
+use File::Path qw(remove_tree);
+use PublicInbox::IO qw(write_file);
+use autodie qw(symlink unlink);
+require_mods qw(DBD::SQLite Xapian);
 require PublicInbox::SolverGit;
 my $rdr = { 2 => \(my $null) };
 my $git_dir = xqx([qw(git rev-parse --git-common-dir)], undef, $rdr);
@@ -22,6 +25,8 @@ my $patch2 = eml_load 't/solve/0002-rename-with-modifications.patch';
 my $patch2_oid = git_sha(1, $patch2)->hexdigest;
 
 my ($tmpdir, $for_destroy) = tmpdir();
+my $gone_repo = "$tmpdir/to-be-deleted.git";
+
 my $ibx = create_inbox 'v2', version => 2,
 			indexlevel => 'medium', sub {
 	my ($im) = @_;
@@ -287,6 +292,8 @@ SKIP: {
 [coderepo "binfoo"]
 	dir = $binfoo
 	cgiturl = http://example.com/binfoo
+[coderepo "goner"]
+	dir = $gone_repo
 EOF
 	close $cfgfh or die;
 	my $exp_digest;
@@ -450,9 +457,32 @@ EOF
 			my $t = XML::TreePP->new->parse($res->content);
 			ok(scalar @{$t->{feed}->{entry}}, 'got tag entries');
 		}
+
+		# test disappearing repos
+		$res = $cb->(GET('/goner/'));
+		is $res->code, 200, 'coderepo summary for goner repo';
+		$res = $cb->(GET("/goner/$oid{'iso-8859-1'}/s/"));
+		is $res->code, 200, 'goner repo OID /s/ still available';
+		$stderr_empty->();
+		remove_tree $gone_repo;
+
+		$res = $cb->(GET('/goner/'));
+		is $res->code, 404, '/goner/ repo summary';
+		$stderr_empty->();
+
+		$res = $cb->(GET("/goner/$oid{'iso-8859-1'}/s/"));
+		is $res->code, 404, 'gone repo OID /s/';
+		$stderr_empty->();
 	};
+
+	state $cp = which('cp');
+	$cp or xbail "`cp' not available (WTF!?)";
+	xsys_e $cp, qw(-Rp), $binfoo, $gone_repo;
+
 	test_psgi(sub { $www->call(@_) }, $client);
 	my $env = { PI_CONFIG => $cfgpath, TMPDIR => $tmpdir };
+
+	xsys_e $cp, qw(-Rp), $binfoo, $gone_repo; # for test_httpd $client
 	test_httpd($env, $client, 7, sub {
 	SKIP: {
 		require_cmd('curl', 1) or skip 'no curl', 1;