Re: [PATCH] v2: fill alternates with old epochs on init from mirrors

[ebiederm@xmission.com (Eric W. Biederman)]

Eric Wong <e@80x24.org> writes:

> "Eric W. Biederman" <ebiederm@xmission.com> wrote:
>> Is that the correct fix?  Am I write in thinking that public-inbox-init
>> is failing to do that?  Or am I missing something?
>
> Yes, filling in alternates is the correct fix.  Sorry about that,
> you've found a bug which I failed to test :x
>
> Below patch should fix it, thanks for the report:
>
> -------8<------
> Subject: [PATCH] v2: fill alternates with old epochs on init from mirrors
>
> For v2 repositories with multiple epochs, we must not forget
> about earlier epochs in clones.  Ensure we update the alternates
> file with all known epochs up to the current one.
>
> Reported-by: Eric W. Biederman <ebiederm@xmission.com>
>   https://public-inbox.org/meta/871scj2vzi.fsf@xmission.com/

Tested-by: "Eric W. Biederman" <ebiederm@xmission.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>


I just reran my test case and objects/info/alternatives now lists
all of the repos.

Thanky you for the quick fix.


> ---
>  lib/PublicInbox/V2Writable.pm | 12 +++++++++---
>  t/v2mirror.t                  | 32 ++++++++++++++++++++++++--------
>  2 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
> index d8d75ec..412eb6a 100644
> --- a/lib/PublicInbox/V2Writable.pm
> +++ b/lib/PublicInbox/V2Writable.pm
> @@ -495,15 +495,21 @@ sub git_init {
>  	PublicInbox::Import::run_die(\@cmd);
>  
>  	my $alt = "$all/objects/info/alternates";
> -	my $new_obj_dir = "../../git/$epoch.git/objects";
>  	my %alts;
> +	my @add;
>  	if (-e $alt) {
>  		open(my $fh, '<', $alt) or die "open < $alt: $!\n";
>  		%alts = map { chomp; $_ => 1 } (<$fh>);
>  	}
> -	return $git_dir if $alts{$new_obj_dir};
> +	foreach my $i (0..$epoch) {
> +		my $dir = "../../git/$i.git/objects";
> +		push @add, $dir if !$alts{$dir} && -d "$pfx/$i.git";
> +	}
> +	return $git_dir unless @add;
>  	open my $fh, '>>', $alt or die "open >> $alt: $!\n";
> -	print $fh "$new_obj_dir\n" or die "print >> $alt: $!\n";
> +	foreach my $dir (@add) {
> +		print $fh "$dir\n" or die "print >> $alt: $!\n";
> +	}
>  	close $fh or die "close $alt: $!\n";
>  	$git_dir
>  }
> diff --git a/t/v2mirror.t b/t/v2mirror.t
> index 4cd369b..c0c329c 100644
> --- a/t/v2mirror.t
> +++ b/t/v2mirror.t
> @@ -45,11 +45,16 @@ To: You <you@example.com>
>  Subject: a
>  Date: Thu, 01 Jan 1970 00:00:00 +0000
>  
> +my $old_rotate_bytes = $v2w->{rotate_bytes};
> +$v2w->{rotate_bytes} = 500; # force rotating
>  for my $i (1..9) {
>  	$mime->header_set('Message-ID', "<$i\@example.com>");
>  	$mime->header_set('Subject', "subject = $i");
>  	ok($v2w->add($mime), "add msg $i OK");
>  }
> +
> +my $epoch_max = $v2w->{epoch_max};
> +ok($epoch_max > 0, "multiple epochs");
>  $v2w->done;
>  
>  my %opts = (
> @@ -79,11 +84,14 @@ ok(defined $pid, 'forked httpd process successfully');
>  my ($host, $port) = ($sock->sockhost, $sock->sockport);
>  $sock = undef;
>  
> -my @cmd = (qw(git clone --mirror -q), "http://$host:$port/v2/0",
> -	"$tmpdir/m/git/0.git");
> +my @cmd;
> +foreach my $i (0..$epoch_max) {
> +	@cmd = (qw(git clone --mirror -q), "http://$host:$port/v2/$i",
> +		"$tmpdir/m/git/$i.git");
>  
> -is(system(@cmd), 0, 'cloned OK');
> -ok(-d "$tmpdir/m/git/0.git", 'mirror OK');;
> +	is(system(@cmd), 0, 'cloned OK');
> +	ok(-d "$tmpdir/m/git/$i.git", 'mirror OK');
> +}
>  
>  @cmd = ("$script-init", '-V2', 'm', "$tmpdir/m", 'http://example.com/m',
>  	'alt@example.com');
> @@ -94,14 +102,23 @@ my $mibx = { mainrepo => "$tmpdir/m", address => 'alt@example.com' };
>  $mibx = PublicInbox::Inbox->new($mibx);
>  is_deeply([$mibx->mm->minmax], [$ibx->mm->minmax], 'index synched minmax');
>  
> +$v2w->{rotate_bytes} = $old_rotate_bytes;
>  for my $i (10..15) {
>  	$mime->header_set('Message-ID', "<$i\@example.com>");
>  	$mime->header_set('Subject', "subject = $i");
>  	ok($v2w->add($mime), "add msg $i OK");
>  }
>  $v2w->barrier;
> -is(system('git', "--git-dir=$tmpdir/m/git/0.git", 'fetch', '-q'), 0,
> -	'fetch successful');
> +
> +sub fetch_each_epoch {
> +	foreach my $i (0..$epoch_max) {
> +		my $dir = "$tmpdir/m/git/$i.git";
> +		is(system('git', "--git-dir=$dir", 'fetch', '-q'), 0,
> +			'fetch successful');
> +	}
> +}
> +
> +fetch_each_epoch();
>  
>  my $mset = $mibx->search->reopen->query('m:15@example.com', {mset => 1});
>  is(scalar($mset->items), 0, 'new message not found in mirror, yet');
> @@ -131,8 +148,7 @@ like($to_purge, qr/\A[a-f0-9]{40,}\z/, 'read blob to be purged');
>  $mset = $ibx->search->reopen->query('m:10@example.com', {mset => 1});
>  is(scalar($mset->items), 0, 'purged message gone from origin');
>  
> -is(system('git', "--git-dir=$tmpdir/m/git/0.git", 'fetch', '-q'), 0,
> -	'fetch successful');
> +fetch_each_epoch();
>  {
>  	open my $err, '+>', "$tmpdir/index-err" or die "open: $!";
>  	my $ipid = fork;