From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 9DA5C1F8C6 for ; Sun, 12 Sep 2021 11:58:15 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] fetch: drop 304 Not Modified support, simplify comparisons Date: Sun, 12 Sep 2021 11:58:15 +0000 Message-Id: <20210912115815.22861-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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';