r69108 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r69107‎ | r69108 | r69109 >
Date:20:40, 6 July 2010
Author:avar
Status:reverted (Comments)
Tags:
Comment:
new-installer: Delay database object construction until ->execute time

My method of putting code that alters $this->parent->installSteps in
the MySQL constructor didn't work because the installer will construct
objects for all the databases, even those it doesn't use.

This ostensibly happens because it needs to be able to provide
defaults for all of them on the DBConnect page.

But when I was going to fix that by exiting the MySQL constructior by
checking $wgDBtype I found that it didn't work, because WebInstaller
calls Installer's __construct *before* any sessions are read or set
up, so $wgDBtype will always be mysql, since that's the default.

Fix that by delaying the construction of the database objects. The
WebInstaller (or equivalent) now has to call ->setupDatabaseObjects()
in its ->execute method. This way the defaults on the DBConnect will
still be provided, but we'll have access to session data in the
database constructors.

Ughed-by: Chad <innocentkiller@gmail.com>
Modified paths:
  • /trunk/phase3/includes/installer/Installer.php (modified) (history)
  • /trunk/phase3/includes/installer/MysqlInstaller.php (modified) (history)
  • /trunk/phase3/includes/installer/WebInstaller.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/installer/WebInstaller.php
@@ -81,6 +81,10 @@
8282 if ( isset( $session['settings'] ) ) {
8383 $this->settings = $session['settings'] + $this->settings;
8484 }
 85+
 86+ /* Must be called after the session setup above */
 87+ $this->setupDatabaseObjects();
 88+
8589 $this->exportVars();
8690 $this->setupLanguage();
8791
Index: trunk/phase3/includes/installer/Installer.php
@@ -233,6 +233,18 @@
234234 foreach ( $this->defaultVarNames as $var ) {
235235 $this->settings[$var] = $GLOBALS[$var];
236236 }
 237+
 238+ $this->parserTitle = Title::newFromText( 'Installer' );
 239+ $this->parserOptions = new ParserOptions;
 240+ $this->parserOptions->setEditSection( false );
 241+ }
 242+
 243+ /*
 244+ * Set up our database objects. They need to inject some of their
 245+ * own configuration into our global context. Usually this'll just be
 246+ * things like the default $wgDBname.
 247+ */
 248+ function setupDatabaseObjects() {
237249 foreach ( $this->dbTypes as $type ) {
238250 $installer = $this->getDBInstaller( $type );
239251 if ( !$installer->isCompiled() ) {
@@ -247,10 +259,6 @@
248260 }
249261 }
250262 }
251 -
252 - $this->parserTitle = Title::newFromText( 'Installer' );
253 - $this->parserOptions = new ParserOptions;
254 - $this->parserOptions->setEditSection( false );
255263 }
256264
257265 /**
Index: trunk/phase3/includes/installer/MysqlInstaller.php
@@ -35,6 +35,10 @@
3636 function __construct( $parent ) {
3737 parent::__construct( $parent );
3838
 39+ if ( $this->parent->getVar( 'wgDBtype' ) !== $this->getName() ) {
 40+ return;
 41+ }
 42+
3943 # Add our user callback to installSteps, right before the tables are created.
4044 $where_tables = array_search( "tables", $this->parent->installSteps );
4145 $callback = array(

Follow-up revisions

RevisionCommit summaryAuthorDate
r69129finish revert of r69108mah02:53, 7 July 2010

Comments

#Comment by MarkAHershberger (talk | contribs)   02:54, 7 July 2010

This breaks sqlite setup in the cli installer. To do the same thing, I've made the extension check a static method.

Also, I really hate modifying the installsteps by mucking with the parent's data. And the check for wgDBtype shouldn't be needed now.

See r69128 + r69129

#Comment by Ævar Arnfjörð Bjarmason (talk | contribs)   11:05, 7 July 2010

Those broke the non-MySQL installer because the database object construction isn't being delayed anymore. See the comments in r69128.

Status & tagging log