Page MenuHomePhabricator

Installer: confirm extensions have way to hook into install/update process
Open, MediumPublic

Description

The present 1.19 installer detects already whether or not extensions are present in $IP/extensions during the installation and offers the user a menu to select and enable these pre-installation extensions.

However, some extensions such as Extension:OpenID and LiquidThreads require database modifications by executing special installation/database-upgrade code in the extensions. One has to start this manually _after_ the MediaWiki installation has finished (run $IP/maintenance/php update.php ).

In my view this should automatically be done by the installer in a special post-installation phase; as explained, this post-phase is only needed if the installer detected any extension and the installing user has selected at least one for installation.


Version: 1.20.x
Severity: normal
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/85021#c17706

Details

Reference
bz28983

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:32 PM
bzimport set Reference to bz28983.
bzimport added a subscriber: Unknown Object (MLST).

Hmm, this really ought to be able to work in the main installer steps; I think there were some issues with how the extensions get initialized during the installer step that causes not all the hooks to make it through.

Chad, does that sound familiar? I think it was due to the way we load up the stub LocalSettings from local function scope, so not all global hooks get copied out.

(In reply to comment #1)

Hmm, this really ought to be able to work in the main installer steps; I think
there were some issues with how the extensions get initialized during the
installer step that causes not all the hooks to make it through.

Chad, does that sound familiar? I think it was due to the way we load up the
stub LocalSettings from local function scope, so not all global hooks get
copied out.

Yes, we only allowed LoadExtensionSchemaUpdates hooks during installation. Other hooks were likely to be dangerous (people doing db queries, etc).

The bug summary kind of sucks (we're not going to run update.php during install), but we should provide a method for extensions to add their own installation steps for post-table setup (which *is* already handled).

(In reply to comment #2)

The bug summary kind of sucks

No, the bug sucks, not the summary.

Let me explain, why.

It's a typical problem like in parsing.

The installer first detects some extensions present during the installation process and adds some "require" directives for those extensions the user wants to be installed:

example:

  1. Enabled Extensions. Most extensions are enabled by including the base extension file here
  2. but check specific extension documentation for more details
  3. The following extensions were automatically enabled:

require( "extensions/OpenID/OpenID.php" );
require( "extensions/LiquidThreads/LiquidThreads.php" );

Then - and not earlier - the hook(-targets) in the extensions are activated; a subsequent run of php update.php _would_ trigger the active "require"s and therefore the hooked database update processes.

I admit, that an automated "php update.php" is strange and might be dangerous.

But please consider the following scenario:

User downloads Mediawiki. User added "LiquidThreads" and "OpenID" extension. User ran the installation procedure, which correctly detected presence of the two extensions. User actived the two checkboxes of the installation process. Installer added the the two "require" lines in LocalSettings.php. User downloaded and installed the LocalSettings.php successfully and started the Wiki. The Wiki STALLS. Because the extensions require an database update, which is triggered by their hooks, if the hooks can be "seen" through the extensions' calls in LocalSettings.php.

Thus in my view, a typical problem and requiring either a "two-pass" installation: i) standard as now ii) if any extensions are added, then run "update.php"

or show a message to the user that they must run "php update.php" if they added at least one extension during the installation process which only added the "require" statements, but did not run the update.

ad-hoc fix to avoid post-installation stalls of MediaWikis for those cases where users opted-in for adding extensions during installation which requires update.php after installion:

i) disable the auto-detection of extensions in the installer; and
ii) disable the auto-adding of "require" statements in LocalSettings.php

Extensions that have new tables to create *will* be created as intended, the installer *already* does this.

What needs doing is a way for extensions to add other installation steps, for things other than table creation they may need to do.

Saying it can only be done by running update.php is wrong.

(In reply to comment #5)

Extensions that have new tables to create *will* be created as intended, the
installer *already* does this.

Try it please with a fresh installation, having some extensions requiring "update.php" present in $IP/extensions/xxx BEFORE you run the installer and opt-in during installation.

You will notice, that the wiki then stalls.

Correct me, if I am wrong. (I noticed the behaviour I am describing here three times)

To get the hot air out of it: in my view, it is only a http://en.wikipedia.org/wiki/Chicken_or_the_egg problem, the installer installs, but the extensions are not (yet) ready to know that they are called

Actually, I'm pretty sure this can be done already. Example:

$wgHooks['LoadExtensionSchemaUpdates'][] = 'myExtUpdate';

function myExtUpdate( $updater ) {
global $IP;
$updater->addExtensionUpdate( array( 'addTable', 'foobar', "$IP/extensions/FooBar/foobar.sql", true
) );
$updater->addExtensionUpdate( array( 'mySpecialUpdate' ) );
}

function mySpecialUpdate() {
// do something
}

This behavior is well documented and used in dozens of extensions. See DatabaseUpdater::runUpdates()/doUpdates() and DatabaseInstaller::createExtensionTables() if you don't believe me.

Retitling because I'm pedantic and we're not going to be "running update.php from the install"

(In reply to comment #8)

Actually, I'm pretty sure this can be done already. Example:

$wgHooks['LoadExtensionSchemaUpdates'][] = 'myExtUpdate';
function myExtUpdate( $updater ) {
...
This behavior is well documented and used in dozens of extensions. See
DatabaseUpdater::runUpdates()/doUpdates() and
DatabaseInstaller::createExtensionTables() if you don't believe me.

Thanks for pointing out. As OpenID already uses this hook since a while, there appears to be another problem, perhaps a timing problem in MediaWiki installer and OpenID.

Will report here soon, but close the bug now - because the mentioned hook apparently solves the bug, or at least hints to a clean solution using an existing mechanism.

(In reply to comment #8)

Actually, I'm pretty sure this can be done already. Example:
$wgHooks['LoadExtensionSchemaUpdates'][] = 'myExtUpdate';

I have the impression, that
public function doUpdates( $what = array( 'core', 'extensions', 'purge' ) ) {

		global $wgVersion;
		$what = array_flip( $what );
		if ( isset( $what['core'] ) ) {
			$this->runUpdates( $this->getCoreUpdateList(), false );
		}
		if ( isset( $what['extensions'] ) ) {
			$this->runUpdates( $this->getOldGlobalUpdates(), false );
			$this->runUpdates( $this->getExtensionUpdates(), true );
		}

nobody runs doUpdates( 'extensions' ) during the installation process of extensions. The hooked code in OpenID, LiquidThreads etc. is triggered, but nothing is actually changed in the table. After adding echoes and checking everything I came to the conclusion that an important call is missing during the installation process.

Pls. can you check whether my observation is correct ?

(In reply to comment #11)

I have the impression, that nobody actually runs doUpdates( 'extensions' ) during the installation process of extensions. The hooked code in OpenID, LiquidThreads etc. is triggered, but nothing is actually changed in the table. After adding echoes and checking everything I came to the conclusion that an important call is missing during the installation process.

Further analysis shows now, that public function DatabaseInstaller::createExtensionTables() is called, but stalls before
the not unimportant end
// Now run updates to create tables for old extensions
$updater->doUpdates( array( 'extensions' ) );

Can someone please have look?

The recent changes do work and allow a single extension to be installed.
I successfully tested mw installation plus i) OpenID or ii) LiquidThreads

However, when installing the both together, the installer hangs after the second step with message "Including extensions..."

I am going to check in the next hour whether this has to do with incorrect $path assignments in the extensions themselves, because I found a strange error in the Apache2 error log file (two extensions paths were mangled, and OpenID indeed sets path, which is to be avoided).

PHP Warning: require_once(/srv/www/htdocs/phase3/extensions/OpenID/LiquidThreads/LiquidThreads.php): failed to open stream: No such file or directory in /srv/www/htdocs/phase3/includes/installer/Installer.php on line 1243, referer: http://www.server.org/phase3/mw-config/index.php?page=Install

PHP Fatal error: require_once(): Failed opening required '/srv/www/htdocs/phase3/extensions/OpenID/LiquidThreads/LiquidThreads.php' (include_path='/srv/www/htdocs/phase3/extensions/OpenID:.:/usr/share/php') in /srv/www/htdocs/phase3/includes/installer/Installer.php on line 1243, referer: http://www.server.org/phase3/mw-config/index.php?page=Install

To do:
I do not reopen the bug for the moment; I need to check the path statements in the two extensions OpenID and LiquidThreads; inform users by means of thi sbug that perhaps a (remaining) problem exists with Installer.php & Co.

However, and this is good news, installing (a single) extension does work.

Einschließlich Erweiterungen…
Installer::includeExtensions: require_once /srv/www/htdocs/phase3/extensions/OpenID/OpenID.php (ok)
Installer::includeExtensions: require_once /srv/www/htdocs/phase3/extensions/OpenID/LiquidThreads/LiquidThreads.php <<< wrong (mangled paths)

Second extension path includes remains from first extension path
Should never happen

There's somewhere an unwanted side effect. Where?

reopened

in Installer.php function includeExtensions() I added the echo()

foreach( $exts as $e ) {

echo( "Installer::includeExtensions: require_once $path/$e/$e.php" );
require_once( "$path/$e/$e.php" );

}

(In reply to comment #17)

in Installer.php function includeExtensions() I added the echo()

foreach( $exts as $e ) {

echo( "Installer::includeExtensions: require_once $path/$e/$e.php" );
require_once( "$path/$e/$e.php" );

}

maybe because OpenID overwrites $path (bad extension...).
Will check. But even when an extension is unsane, it should not effect the foreach loop !

(In reply to comment #18)

(In reply to comment #17)

in Installer.php function includeExtensions() I added the echo()

foreach( $exts as $e ) {

echo( "Installer::includeExtensions: require_once $path/$e/$e.php" );
require_once( "$path/$e/$e.php" );

}

maybe because OpenID overwrites $path (bad extension...).
Will check. But even when an extension is unsane, it should not effect the
foreach loop !

fixed in r89707 . but not finally

checked: now the installer detected and installed correctly (in my case) four extensions incl. schema updates.

arrrrgh... during an upgrade installation from a former version to trunk and while having five current trunk extensions in $IP/extensions , at the end of the upgrade installation the extensions were _not_ installed.

Reopening (I am very sorry about this)