Bug 430014: Re-write the code hooks system so that it uses modules instead of individual .pl files

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> (module owner) a=mkanat

3 files changed, 125 insertions, 47 deletions

diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 51d258227..d7eb14c24 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -590,7 +590,7 @@ sub update_table_definitions {
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################
 
-    Bugzilla::Hook::process('install-update_db');
+    Bugzilla::Hook::process('install_update_db');
 
     # We do this here because otherwise the foreign key from 
     # products.classification_id to classifications.id will fail
diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index 1fa53de9b..190dbe968 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -26,7 +26,8 @@ package Bugzilla::Install::Requirements;
 use strict;
 
 use Bugzilla::Constants;
-use Bugzilla::Install::Util qw(vers_cmp install_string);
+use Bugzilla::Install::Util qw(vers_cmp install_string 
+                               extension_requirement_packages);
 use List::Util qw(max);
 use Safe;
 use Term::ANSIColor;
@@ -138,9 +139,9 @@ sub REQUIRED_MODULES {
     },
     );
 
-    my $all_modules = _get_extension_requirements(
-        'REQUIRED_MODULES', \@modules);
-    return $all_modules;
+    my $extra_modules = _get_extension_requirements('REQUIRED_MODULES');
+    push(@modules, @$extra_modules);
+    return \@modules;
 };
 
 sub OPTIONAL_MODULES {
@@ -291,9 +292,9 @@ sub OPTIONAL_MODULES {
     },
     );
 
-    my $all_modules = _get_extension_requirements(
-        'OPTIONAL_MODULES', \@modules);
-    return $all_modules;
+    my $extra_modules = _get_extension_requirements('OPTIONAL_MODULES');
+    push(@modules, @$extra_modules);
+    return \@modules;
 };
 
 # This maps features to the files that require that feature in order
@@ -312,31 +313,20 @@ use constant FEATURE_FILES => (
     updates       => ['Bugzilla/Update.pm'],
 );
 
-# This implements the install-requirements hook described in Bugzilla::Hook.
+# This implements the REQUIRED_MODULES and OPTIONAL_MODULES stuff
+# described in in Bugzilla::Extension.
 sub _get_extension_requirements {
-    my ($function, $base_modules) = @_;
-    my @all_modules;
-    # get a list of all extensions
-    my @extensions = glob(bz_locations()->{'extensionsdir'} . "/*");
-    foreach my $extension (@extensions) {
-        my $file = "$extension/code/install-requirements.pl";
-        if (-e $file) {
-            my $safe = new Safe;
-            # This is a very liberal Safe.
-            $safe->permit(qw(:browse require entereval caller));
-            $safe->rdo($file);
-            if ($@) {
-                warn $@;
-                next;
-            }
-            my $modules = eval { &{$safe->varglob($function)}($base_modules) };
-            next unless $modules;
-            push(@all_modules, @$modules);
+    my ($function) = @_;
+
+    my $packages = extension_requirement_packages();
+    my @modules;
+    foreach my $package (@$packages) {
+        if ($package->can($function)) {
+            my $extra_modules = $package->$function;
+            push(@modules, @$extra_modules);
         }
     }
-
-    unshift(@all_modules, @$base_modules);
-    return \@all_modules;
+    return \@modules;
 };
 
 sub check_requirements {
diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm
index d3fb4e5f8..254cc237b 100644
--- a/Bugzilla/Install/Util.pm
+++ b/Bugzilla/Install/Util.pm
@@ -37,6 +37,8 @@ use base qw(Exporter);
 our @EXPORT_OK = qw(
     bin_loc
     get_version_and_os
+    extension_code_files
+    extension_requirement_packages
     indicate_progress
     install_string
     include_languages
@@ -79,6 +81,96 @@ sub get_version_and_os {
              os_ver   => $os_details[3] };
 }
 
+sub _extension_paths {
+    my $dir = bz_locations()->{'extensionsdir'};
+    my @extension_items = glob("$dir/*");
+    my @paths;
+    foreach my $item (@extension_items) {
+        my $basename = basename($item);
+        # Skip CVS directories and any hidden files/dirs.
+        next if ($basename eq 'CVS' or $basename =~ /^\./);
+        if (-d $item) {
+            if (!-e "$item/disabled") {
+                push(@paths, $item);
+            }
+        }
+        elsif ($item =~ /\.pm$/i) {
+            push(@paths, $item);
+        }
+    }
+    return @paths;
+}
+
+sub extension_code_files {
+    my ($requirements_only) = @_;
+    my @files;
+    foreach my $path (_extension_paths()) {
+        my @load_files;
+        if (-d $path) {
+            my $extension_file = "$path/Extension.pm";
+            my $config_file    = "$path/Config.pm";
+            if (-e $extension_file) {
+                push(@load_files, $extension_file);
+            }
+            if (-e $config_file) {
+                push(@load_files, $config_file);
+            }
+
+            # Don't load Extension.pm if we just want Config.pm and
+            # we found both.
+            if ($requirements_only and scalar(@load_files) == 2) {
+                shift(@load_files);
+            }
+        }
+        else {
+            push(@load_files, $path);
+        }
+        next if !scalar(@load_files);
+        # We know that these paths are safe, because they came from
+        # extensionsdir and we checked them specifically for their format.
+        # Also, the only thing we ever do with them is pass them to "require".
+        trick_taint($_) foreach @load_files;
+        push(@files, \@load_files);
+    }
+    return \@files;
+}
+
+# Used by _get_extension_requirements in Bugzilla::Install::Requirements.
+sub extension_requirement_packages {
+    # If we're in a .cgi script or some time that's not the requirements phase,
+    # just use Bugzilla->extensions. This avoids running the below code during
+    # a normal Bugzilla page, which is important because the below code
+    # doesn't actually function right if it runs after 
+    # Bugzilla::Extension->load_all (because stuff has already been loaded).
+    # (This matters because almost every page calls Bugzilla->feature, which
+    # calls OPTIONAL_MODULES, which calls this method.)
+    if (eval { Bugzilla->extensions }) {
+        return Bugzilla->extensions;
+    }
+    my $packages = _cache()->{extension_requirement_packages};
+    return $packages if $packages;
+    $packages = [];
+    my %package_map;
+    
+    my $extension_files = extension_code_files('requirements only');
+    foreach my $file_set (@$extension_files) {
+        my $file = shift @$file_set;
+        my $name = require $file;
+        if ($name =~ /^\d+$/) {
+            die install_string('extension_must_return_name',
+                               { file => $file, returned => $name });
+        }
+        my $package = "Bugzilla::Extension::$name";
+        $package_map{$file} = $package;
+        push(@$packages, $package);
+    }
+    _cache()->{extension_requirement_packages} = $packages;
+    # Used by Bugzilla::Extension->load if it's called after this method
+    # (which only happens during checksetup.pl, currently).
+    _cache()->{extension_requirement_package_map} = \%package_map;
+    return $packages;
+}
+
 sub indicate_progress {
     my ($params) = @_;
     my $current = $params->{current};
@@ -93,8 +185,8 @@ sub indicate_progress {
 
 sub install_string {
     my ($string_id, $vars) = @_;
-    _cache()->{template_include_path} ||= template_include_path();
-    my $path = _cache()->{template_include_path};
+    _cache()->{install_string_path} ||= template_include_path();
+    my $path = _cache()->{install_string_path};
     
     my $string_template;
     # Find the first template that defines this string.
@@ -134,10 +226,10 @@ sub include_languages {
     # function in Bugzilla->request_cache. This is done to improve the
     # performance of the template processing.
     my $to_be_cached = 0;
-    if (exists $ENV{'SERVER_SOFTWARE'} and not @_) {
-        my $cache = Bugzilla->request_cache;
+    if (not @_) {
+        my $cache = _cache();
         if (exists $cache->{include_languages}) {
-            return @{$cache->{include_languages}}
+            return @{ $cache->{include_languages} };
         }
         $to_be_cached = 1;
     }
@@ -202,8 +294,7 @@ sub include_languages {
     # Cache the result if we are in CGI mode and called without parameter
     # (see the comment at the top of this function).
     if ($to_be_cached) {
-        my $cache = Bugzilla->request_cache;
-        $cache->{include_languages} = \@usedlanguages;
+        _cache()->{include_languages} = \@usedlanguages;
     }
 
     return @usedlanguages;
@@ -241,12 +332,8 @@ sub template_base_directories {
     # First, we add extension template directories, because extension templates
     # override standard templates. Extensions may be localized in the same way
     # that Bugzilla templates are localized.
-    my @template_dirs;
-    my @extensions = glob(bz_locations()->{'extensionsdir'} . "/*");
-    foreach my $extension (@extensions) {
-        next if (-e "$extension/disabled" or !-d "$extension/template");
-        push(@template_dirs, "$extension/template");
-    }
+    my @extensions = grep { -d "$_/template" } _extension_paths();
+    my @template_dirs = map { "$_/template" } @extensions;
     push(@template_dirs, bz_locations()->{'templatedir'});
     return \@template_dirs;
 }
@@ -384,12 +471,13 @@ sub init_console {
 }
 
 # This is like request_cache, but it's used only by installation code
-# for setup.cgi and things like that.
+# for checksetup.pl and things like that.
 our $_cache = {};
 sub _cache {
-    if ($ENV{MOD_PERL}) {
-        require Apache2::RequestUtil;
-        return Apache2::RequestUtil->request->pnotes();
+    # If the normal request_cache is available (which happens any time
+    # after the requirements phase) then we should use that.
+    if (eval { Bugzilla->request_cache; }) {
+        return Bugzilla->request_cache;
     }
     return $_cache;
 }