* (bug 14497) Throw visible errors in installer scripts when SQL files fail due to...

Database::sourceFile() was suppressing the standard error reporting, and returning any errors as a string value.
While nice behavior in theory, several years ago, this doesn't work well in practice; most callers assume either that any error will be fatal and halt the script (no error checking) or that an error will return false and any boolean true is success. This caused false positive success, as error conditions returned as strings evaluated to true.
Changed to letting the database object's existing erorr reporting behavior control whether it throws an exception or suppresses the error and returns it as a string -- default behavior will be to throw an exception, making 'can't run CREATE TABLE' errors actually visible to the user trying to install an extension.
For good measure, failure to open the input file also throws an exception to ensure that it will produce a nice visible error instead of a hidden WTF.

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index cb1800b..398dad1 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -362,6 +362,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN
 * We no longer just give up on a missing upload base directory; it's now
   created automatically if we have sufficient permissions!
 * (bug 14479) MediaWiki:upload-maxfilesize should have a div id wrapper
+* (bug 14497) Throw visible errors in installer scripts when SQL files
+  fail due to database permission or other error
 
 
 === API changes in 1.13 ===

diff --git a/includes/Database.php b/includes/Database.php
index f3f98e5..b6a8c8b 100644 (file)
--- a/includes/Database.php
+++ b/includes/Database.php
@@ -2116,7 +2116,7 @@ class Database {
 
        /**
         * Read and execute SQL commands from a file.
-        * Returns true on success, error string on failure
+        * Returns true on success, error string or exception on failure (depending on object's error ignore settings)
         * @param string $filename File name to open
         * @param callback $lineCallback Optional function called before reading each line
         * @param callback $resultCallback Optional function called for each MySQL result
@@ -2124,7 +2124,7 @@ class Database {
        function sourceFile( $filename, $lineCallback = false, $resultCallback = false ) {
                $fp = fopen( $filename, 'r' );
                if ( false === $fp ) {
-                       return "Could not open \"{$filename}\".\n";
+                       throw new MWException( "Could not open \"{$filename}\".\n" );
                }
                $error = $this->sourceStream( $fp, $lineCallback, $resultCallback );
                fclose( $fp );
@@ -2133,7 +2133,7 @@ class Database {
 
        /**
         * Read and execute commands from an open file handle
-        * Returns true on success, error string on failure
+        * Returns true on success, error string or exception on failure (depending on object's error ignore settings)
         * @param string $fp File handle
         * @param callback $lineCallback Optional function called before reading each line
         * @param callback $resultCallback Optional function called for each MySQL result
@@ -2176,7 +2176,7 @@ class Database {
                        if ( $done ) {
                                $cmd = str_replace(';;', ";", $cmd);
                                $cmd = $this->replaceVars( $cmd );
-                               $res = $this->query( $cmd, __METHOD__, true );
+                               $res = $this->query( $cmd, __METHOD__ );
                                if ( $resultCallback ) {
                                        call_user_func( $resultCallback, $res );
                                }