Bug 192054: Bugzilla dependencies upgrade messages in wrong order

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

1 files changed, 65 insertions, 50 deletions

diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index 4c62564d7..07729554c 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -107,6 +107,12 @@ sub OPTIONAL_MODULES {
         feature => 'Graphical Reports, New Charts, Old Charts'
     },
     {
+        package => 'Chart',
+        module  => 'Chart::Base',
+        version => '1.0',
+        feature => 'New Charts, Old Charts'
+    },
+    {
         package => 'Template-GD',
         # This module tells us whether or not Template-GD is installed
         # on Template-Toolkits after 2.14, and still works with 2.14 and lower.
@@ -115,12 +121,6 @@ sub OPTIONAL_MODULES {
         feature => 'Graphical Reports'
     },
     {
-        package => 'Chart',
-        module  => 'Chart::Base',
-        version => '1.0',
-        feature => 'New Charts, Old Charts'
-    },
-    {
         package => 'GDGraph',
         module  => 'GD::Graph',
         version => 0,
@@ -264,7 +264,7 @@ sub check_requirements {
 
     print "\n", install_string('checking_modules'), "\n" if $output;
     my $root = ROOT_USER;
-    my %missing = _check_missing(REQUIRED_MODULES, $output);
+    my $missing = _check_missing(REQUIRED_MODULES, $output);
 
     print "\n", install_string('checking_dbd'), "\n" if $output;
     my $have_one_dbd = 0;
@@ -275,19 +275,19 @@ sub check_requirements {
     }
 
     print "\n", install_string('checking_optional'), "\n" if $output;
-    my %missing_optional = _check_missing(OPTIONAL_MODULES, $output);
+    my $missing_optional = _check_missing(OPTIONAL_MODULES, $output);
 
     # If we're running on Windows, reset the input line terminator so that
     # console input works properly - loading CGI tends to mess it up
     $/ = "\015\012" if ON_WINDOWS;
 
-    my $pass = !scalar(keys %missing) && $have_one_dbd;
+    my $pass = !scalar(@$missing) && $have_one_dbd;
     return {
         pass     => $pass,
         one_dbd  => $have_one_dbd,
-        missing  => \%missing,
-        optional => \%missing_optional,
-        any_missing => !$pass || scalar(keys %missing_optional),
+        missing  => $missing,
+        optional => $missing_optional,
+        any_missing => !$pass || scalar(@$missing_optional),
     };
 }
 
@@ -295,14 +295,14 @@ sub check_requirements {
 sub _check_missing {
     my ($modules, $output) = @_;
 
-    my %missing;
+    my @missing;
     foreach my $module (@$modules) {
         unless (have_vers($module, $output)) {
-            $missing{$module->{package}} = $module;
+            push(@missing, $module);
         }
     }
 
-    return %missing;
+    return \@missing;
 }
 
 sub print_module_instructions {
@@ -337,7 +337,7 @@ EOT
     }
 
     # Required Modules
-    if (my %missing = %{$check_results->{missing}}) {
+    if (my @missing = @{$check_results->{missing}}) {
         print <<EOT;
 ***********************************************************************
 * REQUIRED MODULES                                                    *
@@ -351,8 +351,8 @@ EOT
 EOT
 
         print "COMMANDS:\n\n";
-        foreach my $package (keys %missing) {
-            my $command = install_command($missing{$package});
+        foreach my $package (@missing) {
+            my $command = install_command($package);
             print "    $command\n";
         }
         print "\n";
@@ -386,7 +386,7 @@ EOT
 
     return unless $output;
 
-    if (my %missing = %{$check_results->{optional}}) {
+    if (my @missing = @{$check_results->{optional}}) {
         print <<EOT;
 **********************************************************************
 * OPTIONAL MODULES                                                   *
@@ -402,12 +402,8 @@ EOT
 **********************************************************************
 
 EOT
-        # We want to sort them so that they are ordered by feature.
-        my @missing_names = sort {$missing{$a}->{feature} 
-                                  cmp $missing{$b}->{feature}} (keys %missing);
-
         # Now we have to determine how large the table cols will be.
-        my $longest_name = max(map(length($_), @missing_names));
+        my $longest_name = max(map(length($_->{package}), @missing));
 
         # The first column header is at least 11 characters long.
         $longest_name = 11 if $longest_name < 11;
@@ -420,16 +416,16 @@ EOT
         printf "* \%${longest_name}s * %-${remaining_space}s *\n",
                'MODULE NAME', 'ENABLES FEATURE(S)';
         print '*' x 71 . "\n";
-        foreach my $name (@missing_names) {
+        foreach my $package (@missing) {
             printf "* \%${longest_name}s * %-${remaining_space}s *\n",
-                   $name, $missing{$name}->{feature};
+                   $package->{package}, $package->{feature};
         }
         print '*' x 71 . "\n";
 
         print "COMMANDS TO INSTALL:\n\n";
-        foreach my $module (@missing_names) {
-            my $command = install_command($missing{$module});
-            printf "%15s: $command\n", $module;
+        foreach my $module (@missing) {
+            my $command = install_command($module);
+            printf "%15s: $command\n", $module->{package};
         }
     }
 }
@@ -549,7 +545,6 @@ sub install_command {
     return sprintf $command, $package;
 }
 
-
 1;
 
 __END__
@@ -581,26 +576,46 @@ represent the name of the module and the version that we require.
 
 =over 4
 
-=item C<check_requirements($output)>
-
- Description: This checks what optional or required perl modules
-              are installed, like C<checksetup.pl> does.
-
- Params:      C<$output> - C<true> if you want the function to print
-                           out information about what it's doing,
-                           and the versions of everything installed.
-                           If you don't pass the minimum requirements,
-                           the will always print out something, 
-                           regardless of this parameter.
-
- Returns:    A hashref containing three values:
-             C<pass> - Whether or not we have all the mandatory 
-                       requirements.
-             C<missing> - A hash showing which mandatory requirements
-                          are missing. The key is the module name,
-                          and the value is the version we require.
-             C<optional> - Which optional modules are installed and
-                           up-to-date enough for Bugzilla.
+=item C<check_requirements>
+
+=over
+
+=item B<Description>
+
+This checks what optional or required perl modules are installed, like
+C<checksetup.pl> does.
+
+=item B<Params>
+
+=over
+
+=item C<$output> - C<true> if you want the function to print out information
+about what it's doing, and the versions of everything installed.
+
+=back
+
+=item B<Returns>
+
+A hashref containing these values:
+
+=over
+
+=item C<pass> - Whether or not we have all the mandatory requirements.
+
+=item C<missing> - An arrayref containing any required modules that
+are not installed or that are not up-to-date. Each item in the array is
+a hashref in the format of items from L</REQUIRED_MODULES>.
+
+=item C<optional> - The same as C<missing>, but for optional modules.
+
+=item C<have_one_dbd> - True if at least one C<DBD::> module is installed.
+
+=item C<any_missing> - True if there are any missing modules, even optional
+modules.
+
+=back
+
+=back
 
 =item C<check_graphviz($output)>