r85021 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r85020‎ | r85021 | r85022 >
Date:17:32, 30 March 2011
Author:maxsem
Status:resolved (Comments)
Tags:
Comment:
(bug 28237) Installer doesn't create extension tables
Modified paths:
  • /trunk/phase3/includes/installer/DatabaseInstaller.php (modified) (history)
  • /trunk/phase3/includes/installer/DatabaseUpdater.php (modified) (history)
  • /trunk/phase3/includes/installer/OracleUpdater.php (modified) (history)
  • /trunk/phase3/includes/installer/WebInstallerPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/installer/DatabaseUpdater.php
@@ -189,21 +189,29 @@
190190 /**
191191 * Do all the updates
192192 *
 193+ * @param $what Array: what updates to perform
193194 * @param $purge Boolean: whether to clear the objectcache table after updates
194195 */
195 - public function doUpdates( $purge = true ) {
 196+ public function doUpdates( $what = array( 'core', 'extensions', 'purge' ) ) {
196197 global $wgVersion;
197198
198 - $this->runUpdates( $this->getCoreUpdateList(), false );
199 - $this->runUpdates( $this->getOldGlobalUpdates(), false );
200 - $this->runUpdates( $this->getExtensionUpdates(), true );
 199+ $what = array_flip( $what );
 200+ if ( isset( $what['core'] ) ) {
 201+ $this->runUpdates( $this->getCoreUpdateList(), false );
 202+ }
 203+ if ( isset( $what['extensions'] ) ) {
 204+ $this->runUpdates( $this->getOldGlobalUpdates(), false );
 205+ $this->runUpdates( $this->getExtensionUpdates(), true );
 206+ }
201207
202208 $this->setAppliedUpdates( $wgVersion, $this->updates );
203209
204 - if( $purge ) {
 210+ if( isset( $what['purge'] ) ) {
205211 $this->purgeCache();
206212 }
207 - $this->checkStats();
 213+ if ( isset( $what['core'] ) ) {
 214+ $this->checkStats();
 215+ }
208216 }
209217
210218 /**
Index: trunk/phase3/includes/installer/OracleUpdater.php
@@ -83,8 +83,8 @@
8484 /**
8585 * Overload: after this action field info table has to be rebuilt
8686 */
87 - public function doUpdates( $purge = true ) {
88 - parent::doUpdates();
 87+ public function doUpdates( $what = array( 'core', 'extensions', 'purge' ) ) {
 88+ parent::doUpdates( $what );
8989
9090 $this->db->query( 'BEGIN fill_wiki_info; END;' );
9191 }
Index: trunk/phase3/includes/installer/DatabaseInstaller.php
@@ -202,6 +202,10 @@
203203 }
204204 }
205205 }
 206+
 207+ // Now run updates to create tables for old extensions
 208+ $updater->doUpdates( array( 'extensions' ) );
 209+
206210 return $status;
207211 }
208212
Index: trunk/phase3/includes/installer/WebInstallerPage.php
@@ -114,6 +114,27 @@
115115 protected function getFieldsetEnd() {
116116 return "</fieldset>\n";
117117 }
 118+
 119+ /**
 120+ * Opens a textarea used to display the progress of a long operation
 121+ */
 122+ protected function startLiveBox() {
 123+ $this->addHTML(
 124+ '<div id="config-spinner" style="display:none;"><img src="../skins/common/images/ajax-loader.gif" /></div>' .
 125+ '<script>jQuery( "#config-spinner" ).show();</script>' .
 126+ '<textarea id="config-live-log" name="LiveLog" rows="10" cols="30" readonly="readonly">'
 127+ );
 128+ $this->parent->output->flush();
 129+ }
 130+
 131+ /**
 132+ * Opposite to startLiveBox()
 133+ */
 134+ protected function endLiveBox() {
 135+ $this->addHTML( '</textarea>
 136+<script>jQuery( "#config-spinner" ).hide()</script>' );
 137+ $this->parent->output->flush();
 138+ }
118139 }
119140
120141 class WebInstaller_Language extends WebInstallerPage {
@@ -464,16 +485,11 @@
465486
466487 if ( $this->parent->request->wasPosted() ) {
467488 $installer->preUpgrade();
468 - $this->addHTML(
469 - '<div id="config-spinner" style="display:none;"><img src="../skins/common/images/ajax-loader.gif" /></div>' .
470 - '<script>jQuery( "#config-spinner" ).show();</script>' .
471 - '<textarea id="config-update-log" name="UpdateLog" rows="10" readonly="readonly">'
472 - );
473 - $this->parent->output->flush();
 489+
 490+ $this->startLiveBox();
474491 $result = $installer->doUpgrade();
475 - $this->addHTML( '</textarea>
476 -<script>jQuery( "#config-spinner" ).hide()</script>' );
477 - $this->parent->output->flush();
 492+ $this->endLiveBox();
 493+
478494 if ( $result ) {
479495 // If they're going to possibly regenerate LocalSettings, we
480496 // need to create the upgrade/secret keys. Bug 26481
@@ -1081,9 +1097,15 @@
10821098
10831099 public function startStage( $step ) {
10841100 $this->addHTML( "<li>" . wfMsgHtml( "config-install-$step" ) . wfMsg( 'ellipsis') );
 1101+ if ( $step == 'extension-tables' ) {
 1102+ $this->startLiveBox();
 1103+ }
10851104 }
10861105
10871106 public function endStage( $step, $status ) {
 1107+ if ( $step == 'extension-tables' ) {
 1108+ $this->endLiveBox();
 1109+ }
10881110 $msg = $status->isOk() ? 'config-install-step-done' : 'config-install-step-failed';
10891111 $html = wfMsgHtml( 'word-separator' ) . wfMsgHtml( $msg );
10901112 if ( !$status->isOk() ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r85047Followup to r85021, fix caller in updater maintenance scriptoverlordq20:56, 30 March 2011
r85066Follow-up r85021: fix commentmaxsem14:11, 31 March 2011
r85081MFT last round of db and installer changes: r81755, r82596, r85021, r85047, r...demon19:33, 31 March 2011
r86296Followup to r85021: Fix changed ID namediebuche09:35, 18 April 2011
r89653Fix r85021: extension tables weren't created in some casesmaxsem15:52, 7 June 2011
r89660Get rid of addNewExtension()/getNewExtensions() method of adding new extensio...demon17:33, 7 June 2011

Comments

#Comment by OverlordQ (talk | contribs)   20:28, 30 March 2011
# php maintenance/update.php --quick
MediaWiki 1.18alpha Updater

Going to run database updates for wikidb
Depending on the size of your database this may take a while!
PHP Warning:  array_flip() expects parameter 1 to be array, boolean given in /var/www/site/w/includes/installer/DatabaseUpdater.php on line 198

Warning: array_flip() expects parameter 1 to be array, boolean given in /var/www/site/w/includes/installer/DatabaseUpdater.php on line 198
Checking existence of old default messages...done.

Done.
#Comment by Krinkle (talk | contribs)   00:01, 31 March 2011
 	 * @param $purge Boolean: whether to clear the objectcache table after updates

That line should probably be removed. or, to preserve backwards compatiblity:

  • right at the to of the function add a check to see if the second argument isset() and if so, do $what[] = 'purge';
#Comment by MaxSem (talk | contribs)   14:12, 31 March 2011

This is not going to help since it had one parameter previously. All callers updated, comment fixed, resetting to new.

#Comment by Wikinaut (talk | contribs)   04:06, 7 June 2011

I double-checked a fresh installation with extension (OpenID or LiquidThreads) and found, that

206 // Now run updates to create tables for old extensions 207 $updater->doUpdates( array( 'extensions' ) );

in DatabaseInstaller::createExtensionTables() (which I tested, and yes, it was called) is *not* executed.

Please forgive if I am wrong, but currently my analysis shows that the extension schema updates during *installation* appears to be not working currently (upgrades do work).

See also bug28983 .

#Comment by Wikinaut (talk | contribs)   04:08, 7 June 2011

in short and better formatted:

  • in DatabaseInstaller::createExtensionTables()
  • $updater->doUpdates( array( 'extensions' ) ); // code is not reached
#Comment by 😂 (talk | contribs)   16:14, 7 June 2011

MaxSem: The more I think about it, DatabaseUpdater::addNewExtension() (r81266) is really an unnecessary interface, given this rev. Given that only 3 extensions (CR, ConfirmAccount, Math) have started using it...maybe we should just kill it :)

#Comment by MaxSem (talk | contribs)   16:36, 7 June 2011

Yeah, makes sense.

Status & tagging log