Bug 473883: Re-arrange Perl module instructions so that required modules are listed at the end

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

2 files changed, 62 insertions, 59 deletions

diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index bb6189d7a..a7ac7fc19 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -363,15 +363,43 @@ sub _get_activestate_build_id {
 sub print_module_instructions {
     my ($check_results, $output) = @_;
 
-    # We only print these notes if we have to.
+    # First we print the long explanatory messages.
+
+    if (scalar @{$check_results->{missing}}) {
+        print install_string('modules_message_required');
+    }
+
+    if (!$check_results->{one_dbd}) {
+        print install_string('modules_message_db');
+    }
+
+    if (my @missing = @{$check_results->{optional}} and $output) {
+        print install_string('modules_message_optional');
+        # Now we have to determine how large the table cols will be.
+        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;
+
+        # The table is TABLE_WIDTH characters long. There are seven mandatory
+        # characters (* and space) in the string. So, we have a total
+        # of TABLE_WIDTH - 7 characters to work with.
+        my $remaining_space = (TABLE_WIDTH - 7) - $longest_name;
+        print '*' x TABLE_WIDTH . "\n";
+        printf "* \%${longest_name}s * %-${remaining_space}s *\n",
+               'MODULE NAME', 'ENABLES FEATURE(S)';
+        print '*' x TABLE_WIDTH . "\n";
+        foreach my $package (@missing) {
+            printf "* \%${longest_name}s * %-${remaining_space}s *\n",
+                   $package->{package}, $package->{feature};
+        }
+    }
+
+    # We only print the PPM repository note if we have to.
     if ((!$output && @{$check_results->{missing}})
         || ($output && $check_results->{any_missing}))
     {
-        
         if (ON_WINDOWS) {
-            print "\n", install_string('run_as_root', { root => ROOT_USER }),
-                  "\n\n";
-
             my $perl_ver = sprintf('%vd', $^V);
             
             # URL when running Perl 5.8.x.
@@ -386,60 +414,38 @@ sub print_module_instructions {
             if (_get_activestate_build_id() < 819) {
                 print install_string('ppm_repo_up');
             }
-            print "*" x TABLE_WIDTH . "\n";
         }
+
+        # If any output was required, we want to close the "table"
+        print "*" x TABLE_WIDTH . "\n";
     }
 
-    # Required Modules
-    if (my @missing = @{$check_results->{missing}}) {
-        print install_string('modules_message_required') . "\n";
-        foreach my $package (@missing) {
-            my $command = install_command($package);
-            print "    $command\n";
+    # And now we print the actual installation commands.
+
+    if (my @missing = @{$check_results->{optional}} and $output) {
+        print install_string('commands_optional') . "\n\n";
+        foreach my $module (@missing) {
+            my $command = install_command($module);
+            printf "%15s: $command\n", $module->{package};
         }
         print "\n";
     }
 
     if (!$check_results->{one_dbd}) {
-        print install_string('modules_message_db') . "\n";
-
+        print install_string('commands_dbd') . "\n";
         my %db_modules = %{DB_MODULE()};
         foreach my $db (keys %db_modules) {
             my $command = install_command($db_modules{$db}->{dbd});
             printf "%10s: \%s\n", $db_modules{$db}->{name}, $command;
-            print ' ' x 12, install_string('min_version_required'),
-                  $db_modules{$db}->{dbd}->{version}, "\n";
         }
         print "\n";
     }
 
-    return unless $output;
-
-    if (my @missing = @{$check_results->{optional}}) {
-        print install_string('modules_message_optional');
-        # Now we have to determine how large the table cols will be.
-        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;
-
-        # The table is TABLE_WIDTH characters long. There are seven mandatory
-        # characters (* and space) in the string. So, we have a total
-        # of TABLE_WIDTH - 7 characters to work with.
-        my $remaining_space = (TABLE_WIDTH - 7) - $longest_name;
-        printf "* \%${longest_name}s * %-${remaining_space}s *\n",
-               'MODULE NAME', 'ENABLES FEATURE(S)';
-        print '*' x TABLE_WIDTH . "\n";
+    if (my @missing = @{$check_results->{missing}}) {
+        print install_string('commands_required') . "\n";
         foreach my $package (@missing) {
-            printf "* \%${longest_name}s * %-${remaining_space}s *\n",
-                   $package->{package}, $package->{feature};
-        }
-        print '*' x TABLE_WIDTH . "\n";
-
-        print install_string('commands_to_install') . "\n\n";
-        foreach my $module (@missing) {
-            my $command = install_command($module);
-            printf "%15s: $command\n", $module->{package};
+            my $command = install_command($package);
+            print "    $command\n";
         }
     }
 
diff --git a/template/en/default/setup/strings.txt.pl b/template/en/default/setup/strings.txt.pl
index 38464807c..d2a846d51 100644
--- a/template/en/default/setup/strings.txt.pl
+++ b/template/en/default/setup/strings.txt.pl
@@ -33,7 +33,15 @@
     checking_dbd      => 'Checking available perl DBD modules...',
     checking_optional => 'The following Perl modules are optional:',
     checking_modules  => 'Checking perl modules...',
-    commands_to_install => 'COMMANDS TO INSTALL:',
+    commands_dbd      => <<EOT,
+YOU MUST RUN ONE OF THE FOLLOWING COMMANDS (depending on which database
+you use):
+EOT
+    commands_optional => 'COMMANDS TO INSTALL OPTIONAL MODULES:',
+    commands_required => <<EOT,
+COMMANDS TO INSTALL REQUIRED MODULES (You *must* run all these commands
+and then re-run checksetup.pl):
+EOT
     done => 'done.',
     header => "* This is Bugzilla ##bz_ver## on perl ##perl_ver##\n"
             . "* Running on ##os_name## ##os_ver##",
@@ -64,12 +72,8 @@ EOT
 ***********************************************************************
 * In order to access your database, Bugzilla requires that the        *
 * correct "DBD" module be installed for the database that you are     *
-* running.                                                            *
-*                                                                     *
-* Pick and run the correct command below for the database that you    *
-* plan to use with Bugzilla.                                          *
-***********************************************************************
-COMMANDS:
+* running. See below for the correct command to run to install the    *
+* appropriate module for your database.                               *
 EOT
     modules_message_optional => <<EOT,
 ***********************************************************************
@@ -80,10 +84,8 @@ EOT
 * features.                                                           *
 *                                                                     *
 * The optional modules you do not have installed are listed below,    *
-* with the name of the feature they enable. If you want to install    *
-* one of these modules, just run the appropriate command in the       *
-* "COMMANDS TO INSTALL" section.                                      *
-***********************************************************************
+* with the name of the feature they enable. Below that table are the  *
+* commands to install each module.                                    *
 EOT
     modules_message_required => <<EOT,
 ***********************************************************************
@@ -91,11 +93,7 @@ EOT
 ***********************************************************************
 * Bugzilla requires you to install some Perl modules which are either *
 * missing from your system, or the version on your system is too old. *
-*                                                                     *
-* The latest versions of each module can be installed by running the  *
-* commands below.                                                     *
-***********************************************************************
-COMMANDS:
+* See below for commands to install these modules.                    *
 EOT
 
     module_found => "found v##ver##",
@@ -120,7 +118,6 @@ EOT
 * Do that last command over and over until you see "theory58S" at the *
 * top of the displayed list.                                          *
 EOT
-    run_as_root => '* NOTE: You must run any commands listed below as ##root##',
     template_precompile   => "Precompiling templates...",
     template_removing_dir => "Removing existing compiled templates...",
 );