[PATCH] fetch: drop 304 Not Modified support, simplify comparisons

[PATCH] fetch: drop 304 Not Modified support, simplify comparisons

[Eric Wong]

Timestamp comparisons only have 1 second granularity, which
isn't nearly enough for our test cases, and probably not for
real world use for "git send-email" bursts and fast SMTP
servers.

We'll continue to check modification times inside the manifest,
though, in case an extremely rare SHA-1 collision is found...
---
 lib/PublicInbox/Fetch.pm     |  7 +++----
 lib/PublicInbox/LeiMirror.pm |  2 +-
 t/lei-mirror.t               | 14 +++++++++-----
 t/v2mirror.t                 |  4 ----
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index 4d501108..6a6daee6 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -53,7 +53,6 @@ sub do_manifest ($$$) {
 			PublicInbox::LeiMirror::decode_manifest($fh, $mf, $mf)
 		};
 		$lei->err($@) if $@;
-		push @opt, '-z', $mf if defined($m0);
 	}
 	my $curl_cmd = $lei->{curl}->for_uri($lei, $muri, @opt);
 	my $opt = {};
@@ -64,11 +63,11 @@ sub do_manifest ($$$) {
 		$lei->child_error($cerr, "@$curl_cmd failed");
 		return;
 	}
-	return [ 304 ] if !-s $ft; # 304 Not Modified via curl -z
 	my $m1 = PublicInbox::LeiMirror::decode_manifest($ft, $fn, $muri);
 	my $mdiff = { %$m1 };
 
-	# filter out unchanged entries
+	# filter out unchanged entries.  We check modified, too, since
+	# fingerprints are SHA-1, so there's a teeny chance they'll collide
 	while (my ($k, $v0) = each %{$m0 // {}}) {
 		my $cur = $m1->{$k} // next;
 		my $f0 = $v0->{fingerprint} // next;
@@ -77,6 +76,7 @@ sub do_manifest ($$$) {
 		my $t1 = $cur->{modified} // next;
 		delete($mdiff->{$k}) if $f0 eq $f1 && $t0 == $t1;
 	}
+	return unless keys %$mdiff;
 	my (undef, $v1_path, @v2_epochs) =
 		PublicInbox::LeiMirror::deduce_epochs($mdiff, $ibx_uri->path);
 	[ 200, $v1_path, \@v2_epochs, $muri, $ft, $mf ];
@@ -118,7 +118,6 @@ EOM
 	$lei->qerr("# inbox URL: $ibx_uri/");
 	my $res = do_manifest($lei, $dir, $ibx_uri) or return;
 	my ($code, $v1_path, $v2_epochs, $muri, $ft, $mf) = @$res;
-	return if $code == 304;
 	if ($code == 404) {
 		# any pre-manifest.js.gz instances running? Just fetch all
 		# existing ones and unconditionally try cloning the next
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 254848c9..bc2e749c 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -181,7 +181,7 @@ sub index_cloned_inbox {
 
 sub run_reap {
 	my ($lei, $cmd, $opt) = @_;
-	$lei->qerr("# @$cmd");
+	$lei->qerr("# @$cmd" . ($opt->{-C} ? " (in $opt->{-C})" : ''));
 	$opt->{pgid} = 0 if $lei->{sock};
 	my $pid = spawn($cmd, undef, $opt);
 	my $reap = PublicInbox::OnDestroy->new($lei->can('sigint_reap'), $pid);
diff --git a/t/lei-mirror.t b/t/lei-mirror.t
index 7db49e15..4f4c49c1 100644
--- a/t/lei-mirror.t
+++ b/t/lei-mirror.t
@@ -91,30 +91,34 @@ SKIP: {
 	my $opt = { -C => $d, 2 => \(my $err) };
 	ok(!run_script([qw(-clone -q), "$http/404"], undef, $opt), '404 fails');
 	ok(!-d "$d/404", 'destination not created');
-	delete $opt->{2};
 
 	ok(run_script([qw(-clone -q -C), $d, "$http/t2"], undef, $opt),
 		'-clone succeeds on v2');
 	ok(-d "$d/t2/git/0.git", 'epoch cloned');
 	ok(-f "$d/t2/manifest.js.gz", 'manifest saved');
 	ok(!-e "$d/t2/mirror.done", 'no leftover mirror.done');
-	ok(run_script([qw(-fetch -q -C), "$d/t2"], undef, $opt),
+	ok(run_script([qw(-fetch -C), "$d/t2"], undef, $opt),
 		'-fetch succeeds w/ manifest.js.gz');
+	unlike($err, qr/git fetch/, 'no fetch done w/ manifest');
 	unlink("$d/t2/manifest.js.gz") or xbail "unlink $!";
-	ok(run_script([qw(-fetch -q -C), "$d/t2"], undef, $opt),
+	ok(run_script([qw(-fetch -C), "$d/t2"], undef, $opt),
 		'-fetch succeeds w/o manifest.js.gz');
+	like($err, qr/git fetch/, 'fetch forced w/o manifest');
 
 	ok(run_script([qw(-clone -q -C), $d, "$http/t1"], undef, $opt),
 		'cloning v1 works');
 	ok(-d "$d/t1", 'v1 cloned');
 	ok(!-e "$d/t1/mirror.done", 'no leftover file');
-	ok(run_script([qw(-fetch -q -C), "$d/t1"], undef, $opt),
+	ok(-f "$d/t1/manifest.js.gz", 'manifest saved');
+	ok(run_script([qw(-fetch -C), "$d/t1"], undef, $opt),
 		'fetching v1 works');
+	unlike($err, qr/git fetch/, 'no fetch done w/ manifest');
 	unlink("$d/t1/manifest.js.gz") or xbail "unlink $!";
 	my $before = [ glob("$d/t1/*") ];
-	ok(run_script([qw(-fetch -q -C), "$d/t1"], undef, $opt),
+	ok(run_script([qw(-fetch -C), "$d/t1"], undef, $opt),
 		'fetching v1 works w/o manifest.js.gz');
 	unlink("$d/t1/FETCH_HEAD"); # git internal
+	like($err, qr/git fetch/, 'no fetch done w/ manifest');
 	ok(unlink("$d/t1/manifest.js.gz"), 'manifest created');
 	my $after = [ glob("$d/t1/*") ];
 	is_deeply($before, $after, 'no new files created');
diff --git a/t/v2mirror.t b/t/v2mirror.t
index b0075fcc..2bb3238b 100644
--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -99,10 +99,6 @@ $ibx->cleanup;
 
 my @new_epochs;
 my $fetch_each_epoch = sub {
-	my $mf = "$tmpdir/m/manifest.js.gz";
-	if (my @st = stat($mf)) {
-		utime($st[8], $st[9] - 1, $mf) or xbail "utime $mf: $!";
-	}
 	my %before = map { $_ => 1 } glob("$tmpdir/m/git/*");
 	run_script([qw(-fetch -q)], undef, {-C => "$tmpdir/m"}) or
 		xbail '-fetch fail';