Bug 560284: Make all errors that checksetup.pl throws be red

r=mkanat, a=mkanat (module owner)

6 files changed, 47 insertions, 28 deletions

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 87210ffd4..47aeb8a9d 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -143,6 +143,8 @@ use File::Basename;
     ERROR_MODE_DIE_SOAP_FAULT
     ERROR_MODE_JSON_RPC
 
+    COLOR_ERROR
+
     INSTALLATION_MODE_INTERACTIVE
     INSTALLATION_MODE_NON_INTERACTIVE
 
@@ -445,6 +447,9 @@ use constant ERROR_MODE_DIE            => 1;
 use constant ERROR_MODE_DIE_SOAP_FAULT => 2;
 use constant ERROR_MODE_JSON_RPC       => 3;
 
+# The ANSI colors of messages that command-line scripts use
+use constant COLOR_ERROR => 'red';
+
 # The various modes that checksetup.pl can run in.
 use constant INSTALLATION_MODE_INTERACTIVE => 0;
 use constant INSTALLATION_MODE_NON_INTERACTIVE => 1;
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 85ad77e17..fc2e05899 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -147,7 +147,7 @@ sub bz_check_requirements {
         my $dbd_mod = $dbd->{module};
         my $dbd_ver = $dbd->{version};
         my $version = $dbd_ver ? " $dbd_ver or higher" : '';
-        print <<EOT;
+        die <<EOT;
 
 For $sql_server, Bugzilla requires that perl's $dbd_mod $dbd_ver or later be
 installed. To install this module, run the following command (as $root):
@@ -155,7 +155,6 @@ installed. To install this module, run the following command (as $root):
     $command
 
 EOT
-        exit;
     }
 
     # We don't try to connect to the actual database if $db_check is
@@ -178,14 +177,13 @@ EOT
     if ( vers_cmp($sql_vers,$sql_want) > -1 ) {
         print "ok: found v$sql_vers\n" if $output;
     } else {
-        print <<EOT;
+        die <<EOT;
 
 Your $sql_server v$sql_vers is too old. Bugzilla requires version
 $sql_want or later of $sql_server. Please download and install a
 newer version.
 
 EOT
-        exit;
     }
 
     print "\n" if $output;
@@ -213,10 +211,9 @@ sub bz_create_database {
         if (!$success) {
             my $error = $dbh->errstr || $@;
             chomp($error);
-            print STDERR  "The '$db_name' database could not be created.",
-                          " The error returned was:\n\n    $error\n\n",
-                          _bz_connect_error_reasons();
-            exit;
+            die "The '$db_name' database could not be created.",
+                " The error returned was:\n\n    $error\n\n",
+                _bz_connect_error_reasons();
         }
     }
 
@@ -237,9 +234,8 @@ sub _get_no_db_connection {
         # Can't use $dbh->errstr because $dbh is undef.
         my $error = $DBI::errstr || $@;
         chomp($error);
-        print STDERR "There was an error connecting to $sql_server:\n\n",
-                     "    $error\n\n", _bz_connect_error_reasons();
-        exit;
+        die "There was an error connecting to $sql_server:\n\n",
+            "    $error\n\n", _bz_connect_error_reasons(), "\n";
     }
     return $dbh;    
 }
@@ -1309,13 +1305,11 @@ sub _check_references {
             }
         }
         else {
-            print "\n", get_text('install_fk_invalid',
+            die "\n", get_text('install_fk_invalid',
                 { table => $table, column => $column,
                   foreign_table => $foreign_table,
                   foreign_column => $foreign_column,
                  'values' => $bad_values }), "\n";
-            # I just picked a number above 2, to be considered "abnormal exit"
-            exit 3
         }
     }
 }
diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index daf34d04e..297cf5758 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -311,13 +311,12 @@ sub bz_setup_database {
     my ($innodb_on) = @{$self->selectcol_arrayref(
         q{SHOW VARIABLES LIKE '%have_innodb%'}, {Columns=>[2]})};
     if ($innodb_on ne 'YES') {
-        print <<EOT;
+        die <<EOT;
 InnoDB is disabled in your MySQL installation. 
 Bugzilla requires InnoDB to be enabled. 
 Please enable it and then re-run checksetup.pl.
 
 EOT
-        exit 3;
     }
 
 
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 81372da71..aa71c7d0a 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -3080,11 +3080,11 @@ sub _check_content_length {
           WHERE CHAR_LENGTH($field_name) > ?", {Columns=>[1,2]}, $max_length) };
 
     if (scalar keys %contents) {
-        print install_string('install_data_too_long',
-                             { column     => $field_name,
-                               id_column  => $id_field,
-                               table      => $table_name,
-                               max_length => $max_length });
+        my $error = install_string('install_data_too_long',
+                                   { column     => $field_name,
+                                     id_column  => $id_field,
+                                     table      => $table_name,
+                                     max_length => $max_length });
         foreach my $id (keys %contents) {
             my $string = $contents{$id};
             # Don't dump the whole string--it could be 16MB.
@@ -3092,9 +3092,9 @@ sub _check_content_length {
                 $string = substr($string, 0, 30) . "..." 
                          . substr($string, -30) . "\n";
             }
-            print "$id: $string\n";
+            $error .= "$id: $string\n";
         }
-        exit 3;
+        die $error;
     }
 }
 
diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index 34e93b675..b80d7fa8b 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -425,8 +425,11 @@ sub print_module_instructions {
             if (vers_cmp($perl_ver, '5.10') > -1) {
                 $url_to_theory58S = 'http://cpan.uwinnipeg.ca/PPMPackages/10xx/';
             }
-            print colored(install_string('ppm_repo_add', 
-                                 { theory_url => $url_to_theory58S }), 'red');
+            print colored(
+                install_string('ppm_repo_add', 
+                               { theory_url => $url_to_theory58S }),
+                COLOR_ERROR);
+
             # ActivePerls older than revision 819 require an additional command.
             if (ON_ACTIVESTATE < 819) {
                 print install_string('ppm_repo_up');
@@ -459,7 +462,7 @@ sub print_module_instructions {
     }
 
     if (my @missing = @{$check_results->{missing}}) {
-        print colored(install_string('commands_required'), 'red') . "\n";
+        print colored(install_string('commands_required'), COLOR_ERROR), "\n";
         foreach my $package (@missing) {
             my $command = install_command($package);
             print "    $command\n";
@@ -472,7 +475,8 @@ sub print_module_instructions {
         print install_string('install_all', { perl => $^X });
     }
     if (!$check_results->{pass}) {
-        print colored(install_string('installation_failed'), 'red') . "\n\n";
+        print colored(install_string('installation_failed'), COLOR_ERROR),
+              "\n\n";
     }
 }
 
@@ -565,7 +569,7 @@ sub have_vers {
         $ok = "$ok:" if $ok;
         my $str = sprintf "%s %19s %-9s $ok $vstr $black_string\n",
                     install_string('checking_for'), $package, "($want_string)";
-        print $vok ? $str : colored($str, 'red');
+        print $vok ? $str : colored($str, COLOR_ERROR);
     }
     
     return $vok ? 1 : 0;
diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm
index 6621a7a41..7bafa9330 100644
--- a/Bugzilla/Install/Util.pm
+++ b/Bugzilla/Install/Util.pm
@@ -32,6 +32,7 @@ use File::Basename;
 use POSIX qw(setlocale LC_CTYPE);
 use Safe;
 use Scalar::Util qw(tainted);
+use Term::ANSIColor qw(colored);
 
 use base qw(Exporter);
 our @EXPORT_OK = qw(
@@ -559,9 +560,25 @@ sub get_console_locale {
 sub init_console {
     eval { ON_WINDOWS && require Win32::Console::ANSI; };
     $ENV{'ANSI_COLORS_DISABLED'} = 1 if ($@ || !-t *STDOUT);
+    $SIG{__DIE__} = \&_console_die;
     prevent_windows_dialog_boxes();
 }
 
+sub _console_die {
+    my ($message) = @_;
+    # $^S means "we are in an eval"
+    if ($^S) {
+        die $message;
+    }
+    # Remove newlines from the message before we color it, and then
+    # add them back in on display. Otherwise the ANSI escape code
+    # for resetting the color comes after the newline, and Perl thinks
+    # that it should put "at Bugzilla/Install.pm line 1234" after the
+    # message.
+    $message =~ s/\n+$//;
+    die colored($message, COLOR_ERROR) . "\n";
+}
+
 sub prevent_windows_dialog_boxes {
     # This code comes from http://bugs.activestate.com/show_bug.cgi?id=82183
     # and prevents Perl modules from popping up dialog boxes, particularly