Last modified: 2013-07-30 00:10:49 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T30983, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 28983 - Installer: confirm extensions have way to hook into install/update process
Installer: confirm extensions have way to hook into install/update process
Status: REOPENED
Product: MediaWiki
Classification: Unclassified
Installer (Other open bugs)
1.20.x
All All
: Normal normal with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
http://www.mediawiki.org/wiki/Special...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-14 21:54 UTC by T. Gries
Modified: 2013-07-30 00:10 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description T. Gries 2011-05-14 21:54:13 UTC
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.
Comment 1 Brion Vibber 2011-05-15 09:46:36 UTC
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.
Comment 2 Chad H. 2011-05-15 14:00:01 UTC
(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).
Comment 3 T. Gries 2011-05-15 23:04:50 UTC
(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:

# Enabled Extensions. Most extensions are enabled by including the base extension file here
# but check specific extension documentation for more details
# 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.
Comment 4 T. Gries 2011-05-15 23:09:07 UTC
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
Comment 5 Chad H. 2011-05-15 23:12:14 UTC
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.
Comment 6 T. Gries 2011-05-15 23:21:58 UTC
(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)
Comment 7 T. Gries 2011-05-15 23:25:06 UTC
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
Comment 8 Chad H. 2011-06-02 16:10:35 UTC
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.
Comment 9 Chad H. 2011-06-02 19:41:09 UTC
Retitling because I'm pedantic and we're not going to be "running update.php from the install"
Comment 10 T. Gries 2011-06-06 05:53:29 UTC
(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.
Comment 11 T. Gries 2011-06-06 23:54:36 UTC
(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 ?
Comment 12 T. Gries 2011-06-07 03:48:50 UTC
(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?
Comment 14 Max Semenik 2011-06-07 15:54:02 UTC
Fixed in r89653.
Comment 15 T. Gries 2011-06-07 21:53:56 UTC
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.
Comment 16 T. Gries 2011-06-07 22:21:05 UTC
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
Comment 17 T. Gries 2011-06-07 22:22:59 UTC
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" );
}
Comment 18 T. Gries 2011-06-07 22:24:46 UTC
(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 !
Comment 19 T. Gries 2011-06-07 22:30:58 UTC
(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
Comment 20 T. Gries 2011-06-07 22:40:23 UTC
checked: now the installer detected and installed correctly (in my case) four extensions incl. schema updates.
Comment 21 T. Gries 2011-06-08 23:26:01 UTC
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)
Comment 22 Max Semenik 2011-06-13 18:02:29 UTC
Exact repro steps, plz :P)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links