From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS37560 197.231.220.0/22 X-Spam-Status: No, score=-3.2 required=3.0 tests=AWL,BAYES_00,RCVD_IN_XBL, SPF_FAIL,SPF_HELO_FAIL shortcircuit=no autolearn=no autolearn_force=no version=3.4.0 Received: from 80x24.org (exit1.ipredator.se [197.231.221.211]) by dcvr.yhbt.net (Postfix) with ESMTP id 8640F1FAE7 for ; Fri, 30 Mar 2018 20:53:54 +0000 (UTC) From: "Eric Wong (Contractor, The Linux Foundation)" To: meta@public-inbox.org Subject: [PATCH 4/4] v2: respect core.sharedRepository in git configs Date: Fri, 30 Mar 2018 20:53:37 +0000 Message-Id: <20180330205337.19025-5-e@80x24.org> In-Reply-To: <20180330205337.19025-1-e@80x24.org> References: <20180330205337.19025-1-e@80x24.org> List-Id: Ensure -convert and -compact do not make repositories unreadable on live servers. --- lib/PublicInbox/InboxWritable.pm | 5 +- lib/PublicInbox/V2Writable.pm | 26 ++++---- script/public-inbox-compact | 110 +++++++++++++++++-------------- script/public-inbox-convert | 23 +++++-- t/convert-compact.t | 43 ++++++++++++ 5 files changed, 139 insertions(+), 68 deletions(-) diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm index 92b9016..5c11a36 100644 --- a/lib/PublicInbox/InboxWritable.pm +++ b/lib/PublicInbox/InboxWritable.pm @@ -168,10 +168,7 @@ sub import_mbox { sub _read_git_config_perm { my ($self) = @_; my @cmd = qw(config); - if (($self->{version} ||= 1) == 2) { - push @cmd, "--file=$self->{mainrepo}/all.git/config"; - } - chomp(my $perm = $self->git->qx(@cmd, 'core.sharedRepository')); + chomp(my $perm = $self->git->qx('config', 'core.sharedRepository')); $perm; } diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index 34f13e2..c4368cc 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -50,6 +50,7 @@ sub new { } $nparts = nproc() if ($nparts == 0); + $v2ibx = PublicInbox::InboxWritable->new($v2ibx); my $self = { -inbox => $v2ibx, im => undef, # PublicInbox::Import @@ -193,20 +194,23 @@ sub idx_init { # frequently activated. delete $ibx->{$_} foreach (qw(git mm search)); - $self->lock_acquire; + $ibx->umask_prepare; + $ibx->with_umask(sub { + $self->lock_acquire; - # first time initialization, first we create the skeleton pipe: - my $skel = $self->{skel} = PublicInbox::SearchIdxSkeleton->new($self); + # first time initialization, first we create the skeleton pipe: + my $skel = PublicInbox::SearchIdxSkeleton->new($self); + $self->{skel} = $skel; - # need to create all parts before initializing msgmap FD - my $max = $self->{partitions} - 1; - my $idx = $self->{idx_parts} = []; - for my $i (0..$max) { - push @$idx, PublicInbox::SearchIdxPart->new($self, $i, $skel); - } + # need to create all parts before initializing msgmap FD + my $max = $self->{partitions} - 1; + @{$self->{idx_parts}} = map { + PublicInbox::SearchIdxPart->new($self, $_, $skel); + } (0..$max); - # Now that all subprocesses are up, we can open the FD for SQLite: - $skel->_msgmap_init->{dbh}->begin_work; + # Now that all subprocesses are up, we can open the FD for SQLite: + $skel->_msgmap_init->{dbh}->begin_work; + }); } sub purge_oids { diff --git a/script/public-inbox-compact b/script/public-inbox-compact index 016873d..79cd039 100755 --- a/script/public-inbox-compact +++ b/script/public-inbox-compact @@ -15,6 +15,7 @@ my $usage = "Usage: public-inbox-compact REPO_DIR\n"; my $dir = shift or die $usage; my $config = PublicInbox::Config->new; my $ibx; +$dir = abs_path($dir); $config->each_inbox(sub { $ibx = $_[0] if abs_path($_[0]->{mainrepo}) eq $dir }); @@ -29,66 +30,79 @@ unless ($ibx) { $ibx = PublicInbox::Inbox->new($ibx); } my $v = ($ibx->{version} || 1); +$ibx = PublicInbox::InboxWritable->new($ibx); +$ibx->umask_prepare; + +sub commit_changes ($$$) { + my ($im, $old, $new) = @_; + my @st = stat($old) or die "failed to stat($old): $!\n"; + rename($old, "$new/old") or die "rename $old => $new/old: $!\n"; + chmod($st[2] & 07777, $new) or die "chmod $old: $!\n"; + rename($new, $old) or die "rename $new => $old: $!\n"; + $im->lock_release; + remove_tree("$old/old") or die "failed to remove $old/old: $!\n"; +} + if ($v == 2) { require PublicInbox::V2Writable; my $v2w = PublicInbox::V2Writable->new($ibx); my $xap_v = 'xap'.PublicInbox::Search::SCHEMA_VERSION; - my $xroot = "$ibx->{mainrepo}/$xap_v"; - opendir my $dh, $xroot or die "Failed to opendir $xroot: $!\n"; - $v2w->lock_acquire; - my $new = tempdir(CLEANUP => 1, DIR => $ibx->{mainrepo}); - my @parts; - my $skel; - while (defined(my $dn = readdir($dh))) { - if ($dn =~ /\A\d+\z/) { - push @parts, "$xroot/$dn"; - } elsif ($dn eq 'skel') { - $skel = "$xroot/$dn"; - } elsif ($dn eq '.' || $dn eq '..') { + my $old = "$dir/$xap_v"; + opendir my $dh, $old or die "Failed to opendir $old: $!\n"; + my $new = tempdir('compact-XXXXXXXX', CLEANUP => 1, DIR => $dir); + $ibx->with_umask(sub { + $v2w->lock_acquire; + my @parts; + my $skel; + while (defined(my $dn = readdir($dh))) { + if ($dn =~ /\A\d+\z/) { + push @parts, "$old/$dn"; + } elsif ($dn eq 'skel') { + $skel = "$old/$dn"; + } elsif ($dn eq '.' || $dn eq '..') { + } else { + warn "W: skipping unknown Xapian DB: $old/$dn\n" + } + } + close $dh; + my %pids; + + if (@parts) { + my $pid = spawn(['xapian-compact', @parts, "$new/0" ]); + defined $pid or die "compact failed: $?\n"; + $pids{$pid} = 'xapian-compact (parts)'; } else { - warn "W: skipping unknown Xapian DB: $xroot/$dn\n"; + warn "No parts found in $old\n"; } - } - close $dh; - my %pids; - if (@parts) { - my $pid = spawn([ qw(xapian-compact), @parts, "$new/0" ]); - defined $pid or die "compact failed: $?\n"; - $pids{$pid} = 'xapian-compact (parts)'; - } else { - warn "No parts found in $xroot\n"; - } - if (defined $skel) { - my $pid = spawn([ qw(xapian-compact), $skel, "$new/skel" ]); - defined $pid or die "compact failed: $?\n"; - $pids{$pid} = 'xapian-compact (skel)'; - } else { - warn "$xroot/skel missing\n"; - } - die "No xapian-compact processes running\n" unless scalar keys %pids; - while (scalar keys %pids) { - my $pid = waitpid(-1, 0); - my $desc = delete $pids{$pid}; - die "$desc failed: $?\n" if $?; - } - rename($xroot, "$new/old") or die "rename $xroot => $new/old: $!\n"; - rename($new, $xroot) or die "rename $new => $xroot: $!\n"; - $v2w->lock_release; - remove_tree("$xroot/old") or die "failed to remove $xroot/old: $!\n"; + if (defined $skel) { + my $pid = spawn(['xapian-compact', $skel, "$new/skel"]); + defined $pid or die "compact failed: $?\n"; + $pids{$pid} = 'xapian-compact (skel)'; + } else { + warn "$old/skel missing\n"; + } + scalar keys %pids or + die "No xapian-compact processes running\n"; + while (scalar keys %pids) { + my $pid = waitpid(-1, 0); + my $desc = delete $pids{$pid}; + die "$desc failed: $?\n" if $?; + } + commit_changes($v2w, $old, $new); + }); } elsif ($v == 1) { require PublicInbox::Import; my $im = PublicInbox::Import->new($ibx->git, undef, undef, $ibx); my $xap_v = 'xapian'.PublicInbox::Search::SCHEMA_VERSION; - my $v1_root = "$ibx->{mainrepo}/public-inbox"; + my $v1_root = "$dir/public-inbox"; my $old = "$v1_root/$xap_v"; -d $old or die "$old does not exist\n"; - my $new = tempdir(CLEANUP => 1, DIR => $v1_root); - $im->lock_acquire; - PublicInbox::Import::run_die([ qw(xapian-compact), $old, $new ]); - rename($old, "$new/old") or die "rename $old => $new: $!\n"; - rename($new, $old) or die "rename $new => $old: $!\n"; - $im->lock_release; - remove_tree("$old/old") or die "failed to remove $old/old: $!\n"; + my $new = tempdir('compact-XXXXXXXX', CLEANUP => 1, DIR => $v1_root); + $ibx->with_umask(sub { + $im->lock_acquire; + PublicInbox::Import::run_die(['xapian-compact', $old, $new]); + commit_changes($im, $old, $new); + }); } else { die "Unsupported inbox version: $v\n"; } diff --git a/script/public-inbox-convert b/script/public-inbox-convert index e6fb4f5..f58bf27 100755 --- a/script/public-inbox-convert +++ b/script/public-inbox-convert @@ -5,9 +5,10 @@ use strict; use warnings; use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev); use PublicInbox::MIME; -use PublicInbox::Inbox; +use PublicInbox::InboxWritable; use PublicInbox::Config; use PublicInbox::V2Writable; +use PublicInbox::Import; use PublicInbox::Spawn qw(spawn); use Cwd 'abs_path'; my $usage = "Usage: public-inbox-convert OLD NEW\n"; @@ -39,16 +40,28 @@ unless ($old) { }; $old = PublicInbox::Inbox->new($old); } +$old = PublicInbox::InboxWritable->new($old); if (($old->{version} || 1) >= 2) { die "Only conversion from v1 inboxes is supported\n"; } my $new = { %$old }; delete $new->{altid}; # TODO: support altid for v2 -$new->{mainrepo} = $new_dir; +$new->{mainrepo} = abs_path($new_dir); $new->{version} = 2; -$new = PublicInbox::Inbox->new($new); -my $v2w = PublicInbox::V2Writable->new($new, 1); -$v2w->init_inbox($jobs); +$new = PublicInbox::InboxWritable->new($new); +my $v2w; +$old->umask_prepare; +$old->with_umask(sub { + local $ENV{GIT_CONFIG} = "$old->{mainrepo}/config"; + $v2w = PublicInbox::V2Writable->new($new, 1); + $v2w->init_inbox($jobs); + chomp(my $sr = $old->git->qx('config', 'core.sharedRepository')); + if ($sr ne '') { + PublicInbox::Import::run_die(['git', 'config', + "--file=$new->{mainrepo}/all.git/config", + 'core.sharedRepository', $sr]); + } +}); my $state = ''; my ($prev, $from); my $head = $old->{ref_head} || 'HEAD'; diff --git a/t/convert-compact.t b/t/convert-compact.t index 922ec9c..e51eadc 100644 --- a/t/convert-compact.t +++ b/t/convert-compact.t @@ -21,6 +21,9 @@ my $ibx = { ok(PublicInbox::Import::run_die([qw(git init --bare -q), $ibx->{mainrepo}]), 'initialized v1 repo'); +ok(umask(077), 'set restrictive umask'); +ok(PublicInbox::Import::run_die([qw(git) , "--git-dir=$ibx->{mainrepo}", + qw(config core.sharedRepository 0644)]), 'set sharedRepository'); $ibx = PublicInbox::Inbox->new($ibx); my $im = PublicInbox::Import->new($ibx->git, undef, undef, $ibx); my $mime = PublicInbox::MIME->create( @@ -36,6 +39,18 @@ my $mime = PublicInbox::MIME->create( ok($im->add($mime), 'added one message'); $im->done; PublicInbox::SearchIdx->new($ibx, 1)->index_sync; + +is(((stat("$ibx->{mainrepo}/public-inbox"))[2]) & 07777, 0755, + 'sharedRepository respected for v1'); +is(((stat("$ibx->{mainrepo}/public-inbox/msgmap.sqlite3"))[2]) & 07777, 0644, + 'sharedRepository respected for v1 msgmap'); +my @xdir = glob("$ibx->{mainrepo}/public-inbox/xap*/*"); +foreach (@xdir) { + my @st = stat($_); + is($st[2] & 07777, -f _ ? 0644 : 0755, + 'sharedRepository respected on file after convert'); +} + local $ENV{PATH} = "blib/script:$ENV{PATH}"; open my $err, '>>', "$tmpdir/err.log" or die "open: err.log $!\n"; open my $out, '>>', "$tmpdir/out.log" or die "open: out.log $!\n"; @@ -44,8 +59,19 @@ my $rdr = { 1 => fileno($out), 2 => fileno($err) }; my $cmd = [ 'public-inbox-compact', $ibx->{mainrepo} ]; ok(PublicInbox::Import::run_die($cmd, undef, $rdr), 'v1 compact works'); +@xdir = glob("$ibx->{mainrepo}/public-inbox/xap*"); +is(scalar(@xdir), 1, 'got one xapian directory after compact'); +is(((stat($xdir[0]))[2]) & 07777, 0755, + 'sharedRepository respected on v1 compact'); + $cmd = [ 'public-inbox-convert', $ibx->{mainrepo}, "$tmpdir/v2" ]; ok(PublicInbox::Import::run_die($cmd, undef, $rdr), 'convert works'); +@xdir = glob("$tmpdir/v2/xap*/*"); +foreach (@xdir) { + my @st = stat($_); + is($st[2] & 07777, -f _ ? 0644 : 0755, + 'sharedRepository respected after convert'); +} $cmd = [ 'public-inbox-compact', "$tmpdir/v2" ]; my $env = { NPROC => 2 }; @@ -54,4 +80,21 @@ $ibx->{mainrepo} = "$tmpdir/v2"; my $v2w = PublicInbox::V2Writable->new($ibx); is($v2w->{partitions}, 1, "only one partition in compacted repo"); +@xdir = glob("$tmpdir/v2/xap*/*"); +foreach (@xdir) { + my @st = stat($_); + is($st[2] & 07777, -f _ ? 0644 : 0755, + 'sharedRepository respected after v2 compact'); +} +is(((stat("$tmpdir/v2/msgmap.sqlite3"))[2]) & 07777, 0644, + 'sharedRepository respected for v2 msgmap'); + +@xdir = (glob("$tmpdir/v2/git/*.git/objects/*/*"), + glob("$tmpdir/v2/git/*.git/objects/pack/*")); +foreach (@xdir) { + my @st = stat($_); + is($st[2] & 07777, -f _ ? 0444 : 0755, + 'sharedRepository respected after v2 compact'); +} + done_testing(); -- EW