r65672 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r65671‎ \| r65672 \| r65673 >
Date:	21:49, 29 April 2010
Author:	platonides
Status:	ok
Tags:
Comment:	Actually check sourceFile for failure, showing the error message in the install. See report on http://permalink.gmane.org/gmane.org.wikimedia.mediawiki/33902 Fixed several tag nesting problems from setup_database(). Make sourceFile() always use text errors in the installer. It can't handle exceptions properly (introduced in r36211). But they are appropiate for update.php thus the ugly branches. The installer now always handles sourceFile() errors.
Modified paths:	/trunk/phase3/config/Installer.php (modified) (history) /trunk/phase3/includes/db/Database.php (modified) (history) /trunk/phase3/includes/db/DatabaseIbm_db2.php (modified) (history) /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history) /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history) /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabaseOracle.php
—	—	@@ -978,14 +978,19 @@
979	979	function setup_database() {
980	980	global $wgVersion, $wgDBmwschema, $wgDBts2schema, $wgDBport, $wgDBuser;
981	981
982		~~- echo "<li>Creating DB objects</li>\n";~~
983	982	$res = $this->sourceFile( "../maintenance/ora/tables.sql" );
	983	+ if ($res === true) {
	984	+ print " done.</li>\n";
	985	+ } else {
	986	+ print " <b>FAILED</b></li>\n";
	987	+ dieout( htmlspecialchars( $res ) );
	988	+ }
984	989
985	990	// Avoid the non-standard "REPLACE INTO" syntax
986		~~- echo "<li>Populating table interwiki</li>\n";~~
	991	+ echo "<li>Populating interwiki table</li>\n";
987	992	$f = fopen( "../maintenance/interwiki.sql", 'r' );
988	993	if ( $f == false ) {
989		~~- dieout( "<li>Could not find the interwiki.sql file</li>" );~~
	994	+ dieout( "Could not find the interwiki.sql file" );
990	995	}
991	996
992	997	// do it like the postgres :D
Index: trunk/phase3/includes/db/DatabasePostgres.php
—	—	@@ -1330,11 +1330,17 @@
1331	1331	if (!$res) {
1332	1332	print "<b>FAILED</b>. Make sure that the user \"" . htmlspecialchars( $wgDBuser ) .
1333	1333	"\" can write to the schema \"" . htmlspecialchars( $wgDBmwschema ) . "\"</li>\n";
1334		~~- dieout("</ul>");~~
	1334	+ dieout(""); # Will close the main list <ul> and finish the page.
1335	1335	}
1336	1336	$this->doQuery("DROP TABLE $safeschema.$ctest");
1337	1337
1338	1338	$res = $this->sourceFile( "../maintenance/postgres/tables.sql" );
	1339	+ if ($res === true) {
	1340	+ print " done.</li>\n";
	1341	+ } else {
	1342	+ print " <b>FAILED</b></li>\n";
	1343	+ dieout( htmlspecialchars( $res ) );
	1344	+ }
1339	1345
1340	1346	## Update version information
1341	1347	$mwv = $this->addQuotes($wgVersion);
—	—	@@ -1346,10 +1352,12 @@
1347	1353	$dbn = $this->addQuotes($this->mDBname);
1348	1354	$ctype = $this->addQuotes( pg_fetch_result($this->doQuery("SHOW lc_ctype"),0,0) );
1349	1355
	1356	+ echo "<li>Populating interwiki table... ";
1350	1357	## Avoid the non-standard "REPLACE INTO" syntax
1351	1358	$f = fopen( "../maintenance/interwiki.sql", 'r' );
1352	1359	if ($f == false ) {
1353		~~- dieout( "<li>Could not find the interwiki.sql file");~~
	1360	+ print "<b>FAILED</b></li>";
	1361	+ dieout( "Could not find the interwiki.sql file" );
1354	1362	}
1355	1363	## We simply assume it is already empty as we have just created it
1356	1364	$SQL = "INSERT INTO interwiki(iw_prefix,iw_url,iw_local) VALUES ";
—	—	@@ -1361,7 +1369,7 @@
1362	1370	}
1363	1371	$this->query("$SQL $matches[1],$matches[2])");
1364	1372	}
1365		~~- print " (table interwiki successfully populated)...\n";~~
	1373	+ print " successfully populated.</li>\n";
1366	1374
1367	1375	$this->doQuery("COMMIT");
1368	1376	}
Index: trunk/phase3/includes/db/DatabaseIbm_db2.php
—	—	@@ -746,6 +746,11 @@
747	747	$this->begin();
748	748
749	749	$res = $this->sourceFile( "../maintenance/ibm_db2/tables.sql" );
	750	+ if ($res !== true) {
	751	+ print " <b>FAILED</b>: " . htmlspecialchars( $res ) . "</li>";
	752	+ } else {
	753	+ print " done</li>";
	754	+ }
750	755	$res = null;
751	756
752	757	// TODO: populate interwiki links
Index: trunk/phase3/includes/db/Database.php
—	—	@@ -2193,9 +2193,23 @@
2194	2194	function sourceFile( $filename, $lineCallback = false, $resultCallback = false ) {
2195	2195	$fp = fopen( $filename, 'r' );
2196	2196	if ( false === $fp ) {
2197		~~- throw new MWException( "Could not open \"{$filename}\".\n" );~~
	2197	+ if (!defined("MEDIAWIKI_INSTALL"))
	2198	+ throw new MWException( "Could not open \"{$filename}\".\n" );
	2199	+ else
	2200	+ return "Could not open \"{$filename}\".\n";
2198	2201	}
2199		~~- $error = $this->sourceStream( $fp, $lineCallback, $resultCallback );~~
	2202	+ try {
	2203	+ $error = $this->sourceStream( $fp, $lineCallback, $resultCallback );
	2204	+ }
	2205	+ catch( MWException $e ) {
	2206	+ if ( defined("MEDIAWIKI_INSTALL") ) {
	2207	+ $error = $e->getMessage();
	2208	+ } else {
	2209	+ fclose( $fp );
	2210	+ throw $e;
	2211	+ }
	2212	+ }
	2213	+
2200	2214	fclose( $fp );
2201	2215	return $error;
2202	2216	}
Index: trunk/phase3/includes/db/DatabaseSqlite.php
—	—	@@ -487,13 +487,17 @@
488	488	# Process common MySQL/SQLite table definitions
489	489	$err = $this->sourceFile( "$IP/maintenance/tables.sql" );
490	490	if ( $err !== true ) {
491		~~- $this->reportQueryError( $err, 0, $sql, __FUNCTION__ );~~
492		~~- exit( 1 );~~
	491	+ echo " <b>FAILED</b></li>";
	492	+ dieout( htmlspecialchars( $err ) );
493	493	}
	494	+ echo " done.</li>";
494	495
495	496	# Use DatabasePostgres's code to populate interwiki from MySQL template
496	497	$f = fopen( "$IP/maintenance/interwiki.sql", 'r' );
497		~~- if ( $f == false ) dieout( "<li>Could not find the interwiki.sql file" );~~
	498	+ if ( $f == false ) {
	499	+ dieout( "Could not find the interwiki.sql file." );
	500	+ }
	501	+
498	502	$sql = "INSERT INTO interwiki(iw_prefix,iw_url,iw_local) VALUES ";
499	503	while ( !feof( $f ) ) {
500	504	$line = fgets( $f, 1024 );
Index: trunk/phase3/config/Installer.php
—	—	@@ -1004,7 +1004,8 @@
1005	1005	if ($wgDatabase->isOpen()) {
1006	1006	$wgDBOracleDefTS = $conf->DBdefTS_ora;
1007	1007	$wgDBOracleTempTS = $conf->DBtempTS_ora;
1008		~~- $wgDatabase->sourceFile( "../maintenance/ora/user.sql" );~~
	1008	+ $res = $wgDatabase->sourceFile( "../maintenance/ora/user.sql" );
	1009	+ if ($res !== true) dieout($res);
1009	1010	} else {
1010	1011	echo "<li>Invalid database superuser, please supply a valid superuser account.</li>";
1011	1012	echo "<li>ERR: ".print_r(oci_error(), true)."</li>";
—	—	@@ -1181,7 +1182,8 @@
1182	1183	print " <b class='error'>If the next step fails, see <a href='http://dev.mysql.com/doc/mysql/en/old-client.html'>http://dev.mysql.com/doc/mysql/en/old-client.html</a> for help.</b>";
1183	1184	}
1184	1185	print "</li>\n";
1185		~~- $wgDatabase->sourceFile( "../maintenance/users.sql" );~~
	1186	+ $res = $wgDatabase->sourceFile( "../maintenance/users.sql" );
	1187	+ if ($res !== true) dieout($res);
1186	1188	}
1187	1189	}
1188	1190	}
—	—	@@ -1216,8 +1218,17 @@
1217	1219	# FIXME: Check for errors
1218	1220	print "<li>Creating tables...";
1219	1221	if ($conf->DBtype == 'mysql') {
1220		~~- $wgDatabase->sourceFile( "../maintenance/tables.sql" );~~
1221		~~- $wgDatabase->sourceFile( "../maintenance/interwiki.sql" );~~
	1222	+ $res = $wgDatabase->sourceFile( "../maintenance/tables.sql" );
	1223	+ if ($res === true) {
	1224	+ print " done.</li>\n<li>Populating interwiki table... \n";
	1225	+ $res = $wgDatabase->sourceFile( "../maintenance/interwiki.sql" );
	1226	+ }
	1227	+ if ($res === true) {
	1228	+ print " done.</li>\n";
	1229	+ } else {
	1230	+ print " <b>FAILED</b></li>\n";
	1231	+ dieout( htmlspecialchars( $res ) );
	1232	+ }
1222	1233	} elseif (is_callable(array($wgDatabase, 'setup_database'))) {
1223	1234	$wgDatabase->setup_database();
1224	1235	}
—	—	@@ -1226,9 +1237,7 @@
1227	1238	continue;
1228	1239	}
1229	1240
1230		~~- print " done.</li>\n";~~
1231	1241
1232		-
1233	1242	if ( $conf->DBtype == 'ibm_db2' ) {
1234	1243	// Now that table creation is done, make sure everything is committed
1235	1244	// Do this before doing inserts through API
—	—	@@ -1268,8 +1277,13 @@
1269	1278	} else {
1270	1279	# Yes, so run the grants
1271	1280	echo( "<li>" . htmlspecialchars( "Granting user permissions to $wgDBuser on $wgDBname..." ) );
1272		~~- $wgDatabase->sourceFile( "../maintenance/users.sql" );~~
1273		~~- echo( "success.</li>\n" );~~
	1281	+ $res = $wgDatabase->sourceFile( "../maintenance/users.sql" );
	1282	+ if ( $res === true ) {
	1283	+ echo( " success.</li>\n" );
	1284	+ } else {
	1285	+ echo( " <b>FAILED</b>.</li>\n" );
	1286	+ dieout( $res );
	1287	+ }
1274	1288	}
1275	1289	}
1276	1290

Follow-up revisions

Revision	Commit summary	Author	Date
r65673	MFT r65672	platonides	22:00, 29 April 2010
r65674	Follow up r65672. FIXED.	platonides	22:02, 29 April 2010

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r36211	* (bug 14497) Throw visible errors in installer scripts when SQL files fail d...	brion	00:15, 12 June 2008

Status & tagging log

07:10, 29 June 2010 Tim Starling (talk | contribs) changed the status of r65672 [removed: new added: ok]