Bug 368250: collectstats.pl creates files with wrong ownership

r/a=mkanat

2 files changed, 41 insertions, 20 deletions

diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 15106dab9..c5215ecfa 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -46,6 +46,7 @@ our @EXPORT = qw(
     update_filesystem
     create_htaccess
     fix_all_file_permissions
+    fix_dir_permissions
     fix_file_permissions
 );
 
@@ -645,6 +646,26 @@ sub _update_old_charts {
     } 
 }
 
+sub fix_dir_permissions {
+    my ($dir) = @_;
+    return if ON_WINDOWS;
+    # Note that _get_owner_and_group is always silent here.
+    my ($owner_id, $group_id) = _get_owner_and_group();
+
+    my $perms;
+    my $fs = FILESYSTEM();
+    if ($perms = $fs->{recurse_dirs}->{$dir}) {
+        _fix_perms_recursively($dir, $owner_id, $group_id, $perms);
+    }
+    elsif ($perms = $fs->{all_dirs}->{$dir}) {
+        _fix_perms($dir, $owner_id, $group_id, $perms);
+    }
+    else {
+        # Do nothing. We know nothing about this directory.
+        warn "Unknown directory $dir";
+    }
+}
+
 sub fix_file_permissions {
     my ($file) = @_;
     return if ON_WINDOWS;
@@ -843,6 +864,12 @@ Params:      C<$output> - C<true> if you want this function to print
 
 Returns:     nothing
 
+=item C<fix_dir_permissions>
+
+Given the name of a directory, its permissions will be fixed according to
+how they are supposed to be set in Bugzilla's current configuration.
+If it fails to set the permissions, a warning will be printed to STDERR.
+
 =item C<fix_file_permissions>
 
 Given the name of a file, its permissions will be fixed according to
diff --git a/collectstats.pl b/collectstats.pl
index 007669fad..be51d9b8d 100755
--- a/collectstats.pl
+++ b/collectstats.pl
@@ -41,6 +41,7 @@ use Bugzilla::Search;
 use Bugzilla::User;
 use Bugzilla::Product;
 use Bugzilla::Field;
+use Bugzilla::Install::Filesystem qw(fix_dir_permissions);
 
 my %switch;
 GetOptions(\%switch, 'help|h', 'regenerate');
@@ -139,32 +140,28 @@ my $tstart = time;
 my @myproducts = Bugzilla::Product->get_all;
 unshift(@myproducts, "-All-");
 
-foreach (@myproducts) {
-    my $dir = "$datadir/mining";
-
-    &check_data_dir ($dir);
+my $dir = "$datadir/mining";
+if (!-d $dir) {
+    mkdir $dir or die "mkdir $dir failed: $!";
+    fix_dir_permissions($dir);
+}
 
+foreach (@myproducts) {
     if ($switch{'regenerate'}) {
         regenerate_stats($dir, $_, \%bug_resolution, \%bug_status, \%removed);
     } else {
         &collect_stats($dir, $_);
     }
 }
+# Fix permissions for all files in mining/.
+fix_dir_permissions($dir);
+
 my $tend = time;
 # Uncomment the following line for performance testing.
-#print "Total time taken " . delta_time($tstart, $tend) . "\n";
+#say "Total time taken " . delta_time($tstart, $tend);
 
 CollectSeriesData();
 
-sub check_data_dir {
-    my $dir = shift;
-
-    if (! -d $dir) {
-        mkdir $dir, 0755;
-        chmod 0755, $dir;
-    }
-}
-
 sub collect_stats {
     my $dir = shift;
     my $product = shift;
@@ -250,7 +247,6 @@ FIN
     }
     print DATA (join '|', @row) . "\n";
     close DATA;
-    chmod 0644, $file;
 }
 
 sub get_old_data {
@@ -406,14 +402,12 @@ FIN
             foreach (@resolutions) { print DATA "|$bugcount{$_}"; }
             print DATA "\n";
         }
-        
+
         # Finish up output feedback for this product.
         my $tend = time;
-        print "\rRegenerating $product \[100.0\%] - " .
-            delta_time($tstart, $tend) . "\n";
-            
+        say "\rRegenerating $product \[100.0\%] - " . delta_time($tstart, $tend);
+
         close DATA;
-        chmod 0640, $file;
     }
 }