r101449 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101448‎ | r101449 | r101450 >
Date:05:23, 1 November 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
Bug 30934 - Warning: Invalid argument supplied for foreach() in /languages/Language.php on line 423

Doing a run of doRebuildLocalisationCache on each run of update.php
Modified paths:
  • /trunk/phase3/includes/installer/DatabaseUpdater.php (modified) (history)
  • /trunk/phase3/includes/installer/MysqlUpdater.php (modified) (history)
  • /trunk/phase3/includes/installer/OracleUpdater.php (modified) (history)
  • /trunk/phase3/includes/installer/SqliteUpdater.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/installer/SqliteUpdater.php
——@@ -59,6 +59,7 @@
6060 array( 'addIndex', 'user', 'user_email', 'patch-user_email_index.sql' ),
6161 array( 'addTable', 'uploadstash', 'patch-uploadstash.sql' ),
6262 array( 'addTable', 'user_former_groups', 'patch-user_former_groups.sql'),
 63+ array( 'doRebuildLocalisationCache' ),
6364
6465 // 1.19
6566 array( 'addTable', 'config', 'patch-config.sql' ),
Index: trunk/phase3/includes/installer/DatabaseUpdater.php
——@@ -619,4 +619,13 @@
620620 $cl->execute();
621621 $this->output( "done.\n" );
622622 }
 623+
 624+ protected function doRebuildLocalisationCache() {
 625+ global $wgLocalisationCacheConf;
 626+ $wgLocalisationCacheConf['forceRecache'] = true;
 627+ $cl = $this->maintenance->runChild( 'RebuildLocalisationCache', 'rebuildLocalisationCache.php' );
 628+ $this->output( "Rebuilding Localisation Cache... " );
 629+ $cl->execute();
 630+ $this->output( "done.\n" );
 631+ }
623632 }
Index: trunk/phase3/includes/installer/MysqlUpdater.php
——@@ -181,6 +181,7 @@
182182 array( 'modifyField', 'user_properties', 'up_property', 'patch-up_property.sql' ),
183183 array( 'addTable', 'uploadstash', 'patch-uploadstash.sql' ),
184184 array( 'addTable', 'user_former_groups', 'patch-user_former_groups.sql'),
 185+ array( 'doRebuildLocalisationCache' ),
185186
186187 // 1.19
187188 array( 'addTable', 'config', 'patch-config.sql' ),
Index: trunk/phase3/includes/installer/OracleUpdater.php
——@@ -36,6 +36,7 @@
3737 array( 'addIndex', 'user', 'i02', 'patch-user_email_index.sql' ),
3838 array( 'modifyField', 'user_properties', 'up_property', 'patch-up_property.sql' ),
3939 array( 'addTable', 'uploadstash', 'patch-uploadstash.sql' ),
 40+ array( 'doRebuildLocalisationCache' ),
4041
4142 //1.19
4243 array( 'addTable', 'config', 'patch-config.sql' ),
——@@ -43,7 +44,7 @@
4445 array( 'addTable', 'globaltemplatelinks', 'patch-globaltemplatelinks.sql' ),
4546 array( 'addTable', 'globalnamespaces', 'patch-globalnamespaces.sql' ),
4647 array( 'addTable', 'globalinterwiki', 'patch-globalinterwiki.sql' ),
47 -
 48+
4849 // till 2.0 i guess
4950 array( 'doRebuildDuplicateFunction' ),
5051

Follow-up revisions

RevisionCommit summaryAuthorDate
r101450Followup r101449, bit better message outputreedy05:33, 1 November 2011
r101454Followup r101449, just changing forceRecache isn't actually enough...reedy07:34, 1 November 2011
r101897REL1_18 MFT r101449, r101450, r101454reedy22:29, 3 November 2011
r103328Follow-up r101449 and friends: no need to consider LC rebuild a database upda...maxsem15:21, 16 November 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:19, 1 November 2011 
+		$wgLocalisationCacheConf['forceRecache'] = true;

What's this? Shouldn't you be passing --force to the script instead?

#Comment by Reedy (talk | contribs)   07:25, 1 November 2011

It's only setting it for the time that update.php runs. 

		$force = $this->hasOption( 'force' );
//SNIP
		$conf = $wgLocalisationCacheConf;
		$conf['manualRecache'] = false; // Allow fallbacks to create CDB files
		if ( $force ) {
			$conf['forceRecache'] = true;
		}
		$lc = new LocalisationCache_BulkLoad( $conf );

There is currently no functional way to pass --force dynamically to a maintenance script, so this was the simplest workaround. Though, I notice that beneath it, $force is also used in other places

I don't really want to be getting to the extent of shelling out to do a call to run the maintenance script

#Comment by Nikerabbit (talk | contribs)   07:31, 1 November 2011

Is there a bug for this defect?

I didn't know that forceRecache is used internally.

#Comment by Aaron Schulz (talk | contribs)   21:36, 1 November 2011

runChild() should be refactored to allow passing in $opts onto loadParamsAndArgs() rather than global abuse.

#Comment by Reedy (talk | contribs)   21:43, 1 November 2011

Global abuse was already changed in r101454

#Comment by Reedy (talk | contribs)   21:44, 1 November 2011

But doing that would be a good idea also (though, IMHO, not as pressing. Maybe log as a TODO bug)

#Comment by Reedy (talk | contribs)   23:38, 1 November 2011

Logged as bug 32133

Status & tagging log