cindex: respect existing permissions

For internal ($GIT_DIR/public-inbox-cindex) Xapian DBs, we can
rely on core.sharedRepository.  For external ones, we'll just
rely on existing permissions if the directory already exists.

diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index 21c43973dd7b479a3c59fc1a029c67f8478c5304..704baa9c2d3e98ba71ce91a59ab7d37d33fe07f0 100644 (file)
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -710,8 +710,35 @@ sub init_tmp_git_dir ($) {
        $TMP_GIT->{-tmp} = $tmp;
 }
 
+sub prep_umask ($) {
+       my ($self) = @_;
+       my $um;
+       my $cur = umask;
+       if ($self->{-internal}) { # respect core.sharedRepository
+               @{$self->{git_dirs}} == 1 or die 'BUG: only for GIT_DIR';
+               # yuck, FIXME move umask handling out of inbox-specific stuff
+               require PublicInbox::InboxWritable;
+               my $git = PublicInbox::Git->new($self->{git_dirs}->[0]);
+               chomp($um = $git->qx('config', 'core.sharedRepository') // '');
+               $um = PublicInbox::InboxWritable::_git_config_perm(undef, $um);
+               $um = PublicInbox::InboxWritable::_umask_for($um);
+               umask == $um or progress($self, 'umask from git: ',
+                                               sprintf('0%03o', $um));
+       } elsif (-d $self->{cidx_dir}) { # respect existing perms
+               my @st = stat(_);
+               $um = (~$st[2] & 0777);
+               umask == $um or progress($self, 'using umask from ',
+                                               $self->{cidx_dir}, ': ',
+                                               sprintf('0%03o', $um));
+       }
+       defined($um) ?
+               PublicInbox::OnDestroy->new(\&CORE::umask, umask($um)) :
+               undef;
+}
+
 sub cidx_run { # main entry point
        my ($self) = @_;
+       my $restore_umask = prep_umask($self);
        local $self->{todo} = [];
        local $DEFER = $self->{todo};
        local $SIGSET = PublicInbox::DS::block_signals();
@@ -800,7 +827,7 @@ sub shard_done_wait { # awaitpid cb via ipc_worker_reap
        ++$self->{shard_err} if defined($self->{shard_err});
 }
 
-sub with_umask { # TODO
+sub with_umask { # TODO get rid of this treewide and rely on OnDestroy
        my ($self, $cb, @arg) = @_;
        $cb->(@arg);
 }

diff --git a/t/cindex.t b/t/cindex.t
index eb66b2e6f6a41fce1bf4b9c80d59b4dad6ce0837..9da0ba692ff9fc57997ca7eb171976114ff3f9dd 100644 (file)
--- a/t/cindex.t
+++ b/t/cindex.t
@@ -12,9 +12,10 @@ my $pwd = getcwd();
 
 # I reworked CodeSearchIdx->shard_worker to handle empty trees
 # in the initial commit generated by cvs2svn for xapian.git
-create_coderepo 'empty-tree-root', tmpdir => "$tmp/wt0", sub {
+create_coderepo 'empty-tree-root-0600', tmpdir => "$tmp/wt0", sub {
        xsys_e([qw(/bin/sh -c), <<'EOM']);
 git init -q &&
+git config core.sharedRepository 0600
 tree=$(git mktree </dev/null) &&
 head=$(git symbolic-ref HEAD) &&
 cmt=$(echo 'empty root' | git commit-tree $tree) &&
@@ -27,8 +28,14 @@ EOM
 }; # /create_coderepo
 
 ok(run_script([qw(-cindex --dangerous -q), "$tmp/wt0"]), 'cindex internal');
-ok(-e "$tmp/wt0/.git/public-inbox-cindex/cidx.lock", 'internal dir created');
-
+{
+       my $exists = -e "$tmp/wt0/.git/public-inbox-cindex/cidx.lock";
+       my @st = stat(_);
+       ok($exists, 'internal dir created');
+       is($st[2] & 0600, 0600, 'mode respects core.sharedRepository');
+       @st = stat("$tmp/wt0/.git/public-inbox-cindex");
+       is($st[2] & 0700, 0700, 'dir mode respects core.sharedRepository');
+}
 
 # it's possible for git to emit NUL characters in diffs
 # (see c4201214cbf10636e2c1ab9131573f735b42c8d4 in linux.git)
@@ -115,4 +122,13 @@ if ('--prune') {
                'hit stays pruned since GIT_DIR was previously pruned');
 }
 
+File::Path::remove_tree("$tmp/ext");
+ok(mkdir("$tmp/ext", 0707), 'create $tmp/ext with odd permissions');
+ok(run_script([qw(-cindex --dangerous -q -d), "$tmp/ext", $zp]),
+       'external on existing dir');
+{
+       my @st = stat("$tmp/ext/cidx.lock");
+       is($st[2] & 0777, 0604, 'created lock respects odd permissions');
+}
+
 done_testing;