From 68e182a29404407680bee482f99ee24dd9c3c7cb Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 30 Dec 2022 10:59:39 +0000 Subject: [PATCH] clone: fix --post-update-hook behavior Only run hooks if we've done a fetch (which may be a no-op), and add some tests to ensure it works as advertised with and without --objstore= --- lib/PublicInbox/LeiMirror.pm | 29 +++++++++++++++-------------- t/clone-coderepo.t | 26 ++++++++++++++++++-------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm index bfbfd843..34a4bd1c 100644 --- a/lib/PublicInbox/LeiMirror.pm +++ b/lib/PublicInbox/LeiMirror.pm @@ -24,6 +24,7 @@ use POSIX qw(strftime); our $LIVE; # pid => callback our $FGRP_TODO; # objstore -> [ fgrp mirror objects ] our $TODO; # reference => [ non-fgrp mirror objects ] +our @PUH; # post-update hooks sub keep_going ($) { $LIVE && (!$_[0]->{lei}->{child_error} || @@ -299,7 +300,7 @@ sub fgrp_update { my $cmd = [ 'git', "--git-dir=$fgrp->{cur_dst}", qw(update-ref --stdin -z) ]; my $lei = $fgrp->{lei}; - my $pack = PublicInbox::OnDestroy->new($$, \&pack_dst, $fgrp); + my $pack = PublicInbox::OnDestroy->new($$, \&satellite_done, $fgrp); start_cmd($fgrp, $cmd, { 0 => $r, 2 => $lei->{2} }, $pack); close $r or die "close(r): $!"; return if $fgrp->{dry_run}; @@ -319,9 +320,10 @@ sub fgrp_update { close($w) or warn "E: close(update-ref --stdin): $! (need git 1.8.5+)\n"; } -sub pack_dst { # packs lightweight satellite repos +sub satellite_done { my ($fgrp) = @_; pack_refs($fgrp, $fgrp->{cur_dst}); + run_puh($fgrp); } sub pack_refs { @@ -513,7 +515,8 @@ sub resume_fetch { my $cmd = [ @{$self->{-torsocks}}, @git, fetch_args($self->{lei}, $opt), $rn ]; push @$cmd, '-P' if $self->{lei}->{prune}; # --prune-tags implied - start_cmd($self, $cmd, $opt, $fini); + my $run_puh = PublicInbox::OnDestroy->new($$, \&run_puh, $self, $fini); + start_cmd($self, $cmd, $opt, $run_puh); } sub fgrp_enqueue { @@ -562,7 +565,8 @@ sub clone_v1 { "$self->{dst}$ref"; } } - start_cmd($self, $cmd, $opt, $fini); + start_cmd($self, $cmd, $opt, PublicInbox::OnDestroy->new($$, + \&run_puh, $self, $fini)); } if (!$self->{-is_epoch} && $lei->{opt}->{'inbox-config'} =~ /\A(?:always|v1)\z/s) { @@ -683,17 +687,17 @@ sub atomic_write ($$$) { sub run_next_puh { my ($self) = @_; - my $puh = shift @{$self->{-puh_todo}} // return; + my $puh = shift @{$self->{-puh_todo}} // return delete($self->{-fini}); my $fini = PublicInbox::OnDestroy->new($$, \&run_next_puh, $self); my $cmd = [ @$puh, ($self->{cur_dst} // $self->{dst}) ]; my $opt = +{ map { $_ => $self->{lei}->{$_} } (0..2) }; start_cmd($self, $cmd, undef, $opt, $fini); } -sub run_post_update_hooks { - my ($self) = @_; - my $puh = $self->{-puh} // return; - @{$self->{-puh_todo}} = @$puh; +sub run_puh { + my ($self, $fini) = @_; + $self->{-fini} = $fini; + @{$self->{-puh_todo}} = @PUH; run_next_puh($self); } @@ -788,7 +792,6 @@ sub v1_done { # called via OnDestroy } eval { set_description($self) }; warn $@ if $@; - run_post_update_hooks($self); return if ($self->{-is_epoch} || $self->{lei}->{opt}->{'inbox-config'} ne 'always'); write_makefile($dst, 1); @@ -1181,12 +1184,10 @@ sub do_mirror { # via wq_io_do or public-inbox-clone $self->{dry_run} = 1 if $lei->{opt}->{'dry-run'}; umask($lei->{client_umask}) if defined $lei->{client_umask}; $self->{-initial_clone} = 1 if !-d $self->{dst}; + local @PUH; if (defined(my $puh = $lei->{opt}->{'post-update-hook'})) { require Text::ParseWords; - for (@$puh) { - my $pfx = [ Text::ParseWords::shellwords($_) ]; - push @{$self->{-puh}}, $pfx; - } + @PUH = map { [ Text::ParseWords::shellwords($_) ] } @$puh; } eval { my $ic = $lei->{opt}->{'inbox-config'} //= 'always'; diff --git a/t/clone-coderepo.t b/t/clone-coderepo.t index 94721207..66eaa030 100644 --- a/t/clone-coderepo.t +++ b/t/clone-coderepo.t @@ -127,26 +127,36 @@ is(PublicInbox::Git::try_cat($dst_pl), "a.git\nb.git\n", like($err, qr/no longer exist.*\bgone\.git\b/s, 'gone.git noted'); } -{ +my $test_puh = sub { + my (@clone_arg) = @_; my $x = [qw(-clone --inbox-config=never --manifest= --project-list= - --objstore= -p), $url, "$tmpdir/dst", + -q -p), $url, "$tmpdir/dst", @clone_arg, '--post-update-hook=./t/clone-coderepo-puh1.sh', '--post-update-hook=./t/clone-coderepo-puh2.sh' ]; my $log = "$tmpdir/puh.log"; my $env = { CLONE_CODEREPO_TEST_OUT => $log }; - ok(run_script($x, $env), 'no-op clone w/ post-update-hook'); - ok(!-e $log, 'hooks not run on no-op'); remove_tree("$tmpdir/dst"); - ok(run_script($x, $env), 'fresh clone w/ post-update-hook'); - ok(-e $log, 'hooks run on fresh clone'); + ok(run_script($x, $env), "fresh clone @clone_arg w/ post-update-hook"); + ok(-e $log, "hooks run on fresh clone @clone_arg"); open my $lh, '<', $log or xbail "open $log: $!"; chomp(my @l = readline($lh)); - is(scalar(@l), 4, '4 lines written by hooks'); + is(scalar(@l), 4, "4 lines written by hooks on @clone_arg"); for my $r (qw(a b)) { is_xdeeply(['uno', 'dos'], [ (map { s/ .+//; $_ } grep(m!/$r\.git\z!, @l)) ], "$r.git hooks ran in order") or diag explain(\@l); } -} + unlink($log) or xbail "unlink: $!"; + ok(run_script($x, $env), "no-op clone @clone_arg w/ post-update-hook"); + ok(!-e $log, "hooks not run on no-op @clone_arg"); +}; +$test_puh->(); +ok(!-e "$tmpdir/dst/objstore", 'no objstore, yet'); + +my $fgrp = 'fgrp'; +$m->{'/a.git'}->{forkgroup} = $m->{'/b.git'}->{forkgroup} = $fgrp; +$set_manifest->($m); +$test_puh->('--objstore='); +ok(-e "$tmpdir/dst/objstore", 'objstore created'); done_testing; -- 2.44.0