From c8833d8e8ecc1b60f289f5c1ffdb15b2f6d44f8b Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Sun, 8 Apr 2018 16:26:01 -0700 Subject: [PATCH] Handle extension dependencies in the installer As there will likely be extensions bundled with the 1.31 release that depend upon other extensions, we should have the installer prevent users from enabling extensions that depend on other, not-enabled extensions. We can build a dependency map from extension.json's "requires" component. On the client-side, we'll first disable all checkboxes that require other extensions, and evaluate each checkbox click, updating the disabled checkboxes as possible. This required some refactoring of how ExtensionRegistry reports issues with dependency resolution so we could get a list of what was missing. While we're at it, sort the extensions under headings by type. This does not support skins that have dependencies yet (T186092). Bug: T31134 Bug: T55985 Change-Id: I5f0e3b1b540b5ef6f9b8e3fc2bbaad1c65b4b680 --- autoload.php | 1 + includes/installer/Installer.php | 93 ++++++++++++++++++- includes/installer/WebInstaller.php | 28 +++--- includes/installer/WebInstallerOptions.php | 80 +++++++++++++++- includes/installer/i18n/en.json | 1 + includes/installer/i18n/qqq.json | 1 + .../registration/ExtensionDependencyError.php | 81 ++++++++++++++++ includes/registration/ExtensionRegistry.php | 7 +- includes/registration/VersionChecker.php | 41 ++++++-- mw-config/config.js | 48 ++++++++++ .../registration/VersionCheckerTest.php | 26 +++++- 11 files changed, 373 insertions(+), 34 deletions(-) create mode 100644 includes/registration/ExtensionDependencyError.php diff --git a/autoload.php b/autoload.php index fc610cfd18..0e1b30fe0f 100644 --- a/autoload.php +++ b/autoload.php @@ -456,6 +456,7 @@ $wgAutoloadLocalClasses = [ 'ExplodeIterator' => __DIR__ . '/includes/libs/ExplodeIterator.php', 'ExportProgressFilter' => __DIR__ . '/includes/export/ExportProgressFilter.php', 'ExportSites' => __DIR__ . '/maintenance/exportSites.php', + 'ExtensionDependencyError' => __DIR__ . '/includes/registration/ExtensionDependencyError.php', 'ExtensionJsonValidationError' => __DIR__ . '/includes/registration/ExtensionJsonValidationError.php', 'ExtensionJsonValidator' => __DIR__ . '/includes/registration/ExtensionJsonValidator.php', 'ExtensionLanguages' => __DIR__ . '/maintenance/language/languages.inc', diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index 94a5a5a474..1efe5d6373 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -1301,7 +1301,15 @@ abstract class Installer { if ( !is_dir( "$extDir/$file" ) ) { continue; } - if ( file_exists( "$extDir/$file/$jsonFile" ) || file_exists( "$extDir/$file/$file.php" ) ) { + $fullJsonFile = "$extDir/$file/$jsonFile"; + $isJson = file_exists( $fullJsonFile ); + $isPhp = false; + if ( !$isJson ) { + // Only fallback to PHP file if JSON doesn't exist + $fullPhpFile = "$extDir/$file/$file.php"; + $isPhp = file_exists( $fullPhpFile ); + } + if ( $isJson || $isPhp ) { // Extension exists. Now see if there are screenshots $exts[$file] = []; if ( is_dir( "$extDir/$file/screenshots" ) ) { @@ -1312,6 +1320,13 @@ abstract class Installer { } } + if ( $isJson ) { + $info = $this->readExtension( $fullJsonFile ); + if ( $info === false ) { + continue; + } + $exts[$file] += $info; + } } closedir( $dh ); uksort( $exts, 'strnatcasecmp' ); @@ -1319,6 +1334,82 @@ abstract class Installer { return $exts; } + /** + * @param string $fullJsonFile + * @param array $extDeps + * @param array $skinDeps + * + * @return array|bool False if this extension can't be loaded + */ + private function readExtension( $fullJsonFile, $extDeps = [], $skinDeps = [] ) { + $load = [ + $fullJsonFile => 1 + ]; + if ( $extDeps ) { + $extDir = $this->getVar( 'IP' ) . '/extensions'; + foreach ( $extDeps as $dep ) { + $fname = "$extDir/$dep/extension.json"; + if ( !file_exists( $fname ) ) { + return false; + } + $load[$fname] = 1; + } + } + if ( $skinDeps ) { + $skinDir = $this->getVar( 'IP' ) . '/skins'; + foreach ( $skinDeps as $dep ) { + $fname = "$skinDir/$dep/skin.json"; + if ( !file_exists( $fname ) ) { + return false; + } + $load[$fname] = 1; + } + } + $registry = new ExtensionRegistry(); + try { + $info = $registry->readFromQueue( $load ); + } catch ( ExtensionDependencyError $e ) { + if ( $e->incompatibleCore || $e->incompatibleSkins + || $e->incompatibleExtensions + ) { + // If something is incompatible with a dependency, we have no real + // option besides skipping it + return false; + } elseif ( $e->missingExtensions || $e->missingSkins ) { + // There's an extension missing in the dependency tree, + // so add those to the dependency list and try again + return $this->readExtension( + $fullJsonFile, + array_merge( $extDeps, $e->missingExtensions ), + array_merge( $skinDeps, $e->missingSkins ) + ); + } + // Some other kind of dependency error? + return false; + } + $ret = []; + // The order of credits will be the order of $load, + // so the first extension is the one we want to load, + // everything else is a dependency + $i = 0; + foreach ( $info['credits'] as $name => $credit ) { + $i++; + if ( $i == 1 ) { + // Extension we want to load + continue; + } + $type = basename( $credit['path'] ) === 'skin.json' ? 'skins' : 'extensions'; + $ret['requires'][$type][] = $credit['name']; + } + $credits = array_values( $info['credits'] )[0]; + if ( isset( $credits['url'] ) ) { + $ret['url'] = $credits['url']; + } + $ret['type'] = $credits['type']; + + return $ret; + } + /** * Returns a default value to be used for $wgDefaultSkin: normally the one set in DefaultSettings, * but will fall back to another if the default skin is missing and some other one is present diff --git a/includes/installer/WebInstaller.php b/includes/installer/WebInstaller.php index 9d7e0514bb..8fb980791e 100644 --- a/includes/installer/WebInstaller.php +++ b/includes/installer/WebInstaller.php @@ -915,6 +915,7 @@ class WebInstaller extends Installer { * Parameters are: * var: The variable to be configured (required) * label: The message name for the label (required) + * labelAttribs:Additional attributes for the label element (optional) * attribs: Additional attributes for the input element (optional) * controlName: The name for the input element (optional) * value: The current value of the variable (optional) @@ -937,6 +938,9 @@ class WebInstaller extends Installer { if ( !isset( $params['help'] ) ) { $params['help'] = ""; } + if ( !isset( $params['labelAttribs'] ) ) { + $params['labelAttribs'] = []; + } if ( isset( $params['rawtext'] ) ) { $labelText = $params['rawtext']; } else { @@ -945,17 +949,19 @@ class WebInstaller extends Installer { return "
\n" . $params['help'] . - "\n" . + Html::rawElement( + 'label', + $params['labelAttribs'], + Xml::check( + $params['controlName'], + $params['value'], + $params['attribs'] + [ + 'id' => $params['controlName'], + 'tabindex' => $this->nextTabIndex(), + ] + ) . + $labelText . "\n" + ) . "
\n"; } diff --git a/includes/installer/WebInstallerOptions.php b/includes/installer/WebInstallerOptions.php index 07378ab32e..c62eb6572f 100644 --- a/includes/installer/WebInstallerOptions.php +++ b/includes/installer/WebInstallerOptions.php @@ -25,6 +25,8 @@ class WebInstallerOptions extends WebInstallerPage { * @return string|null */ public function execute() { + global $wgLang; + if ( $this->getVar( '_SkipOptional' ) == 'skip' ) { $this->submitSkins(); return 'skip'; @@ -145,20 +147,90 @@ class WebInstallerOptions extends WebInstallerPage { $this->addHTML( $skinHtml ); $extensions = $this->parent->findExtensions(); + $dependencyMap = []; if ( $extensions ) { $extHtml = $this->getFieldsetStart( 'config-extensions' ); + $extByType = []; + $types = SpecialVersion::getExtensionTypes(); + // Sort by type first foreach ( $extensions as $ext => $info ) { - $extHtml .= $this->parent->getCheckBox( [ - 'var' => "ext-$ext", - 'rawtext' => $ext, - ] ); + if ( !isset( $info['type'] ) || !isset( $types[$info['type']] ) ) { + // We let extensions normally define custom types, but + // since we aren't loading extensions, we'll have to + // categorize them under other + $info['type'] = 'other'; + } + $extByType[$info['type']][$ext] = $info; + } + + foreach ( $types as $type => $message ) { + if ( !isset( $extByType[$type] ) ) { + continue; + } + $extHtml .= Html::element( 'h2', [], $message ); + foreach ( $extByType[$type] as $ext => $info ) { + $urlText = ''; + if ( isset( $info['url'] ) ) { + $urlText = ' ' . Html::element( 'a', [ 'href' => $info['url'] ], '(more information)' ); + } + $attribs = [ 'data-name' => $ext ]; + $labelAttribs = []; + $fullDepList = []; + if ( isset( $info['requires']['extensions'] ) ) { + $dependencyMap[$ext]['extensions'] = $info['requires']['extensions']; + $labelAttribs['class'] = 'mw-ext-with-dependencies'; + } + if ( isset( $info['requires']['skins'] ) ) { + $dependencyMap[$ext]['skins'] = $info['requires']['skins']; + $labelAttribs['class'] = 'mw-ext-with-dependencies'; + } + if ( isset( $dependencyMap[$ext] ) ) { + $links = []; + // For each dependency, link to the checkbox for each + // extension/skin that is required + if ( isset( $dependencyMap[$ext]['extensions'] ) ) { + foreach ( $dependencyMap[$ext]['extensions'] as $name ) { + $links[] = Html::element( + 'a', + [ 'href' => "#config_ext-$name" ], + $name + ); + } + } + if ( isset( $dependencyMap[$ext]['skins'] ) ) { + foreach ( $dependencyMap[$ext]['skins'] as $name ) { + $links[] = Html::element( + 'a', + [ 'href' => "#config_skin-$name" ], + $name + ); + } + } + + $text = wfMessage( 'config-extensions-requires' ) + ->rawParams( $ext, $wgLang->commaList( $links ) ) + ->escaped(); + } else { + $text = $ext; + } + $extHtml .= $this->parent->getCheckBox( [ + 'var' => "ext-$ext", + 'rawtext' => $text, + 'attribs' => $attribs, + 'labelAttribs' => $labelAttribs, + ] ); + } } $extHtml .= $this->parent->getHelpBox( 'config-extensions-help' ) . $this->getFieldsetEnd(); $this->addHTML( $extHtml ); + // Push the dependency map to the client side + $this->addHTML( Html::inlineScript( + 'var extDependencyMap = ' . Xml::encodeJsVar( $dependencyMap ) + ) ); } // Having / in paths in Windows looks funny :) diff --git a/includes/installer/i18n/en.json b/includes/installer/i18n/en.json index f1b70806b4..d1a3b83423 100644 --- a/includes/installer/i18n/en.json +++ b/includes/installer/i18n/en.json @@ -311,6 +311,7 @@ "config-extension-link": "Did you know that your wiki supports [https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:Extensions extensions]?\n\nYou can browse [https://www.mediawiki.org/wiki/Special:MyLanguage/Category:Extensions_by_category extensions by category] or the [https://www.mediawiki.org/wiki/Extension_Matrix Extension Matrix] to see the full list of extensions.", "config-skins-screenshots": "$1 (screenshots: $2)", "config-skins-screenshot": "$1 ($2)", + "config-extensions-requires": "$1 (requires $2)", "config-screenshot": "screenshot", "mainpagetext": "MediaWiki has been installed.", "mainpagedocfooter": "Consult the [https://www.mediawiki.org/wiki/Special:MyLanguage/Help:Contents User's Guide] for information on using the wiki software.\n\n== Getting started ==\n* [https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:Configuration_settings Configuration settings list]\n* [https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:FAQ MediaWiki FAQ]\n* [https://lists.wikimedia.org/mailman/listinfo/mediawiki-announce MediaWiki release mailing list]\n* [https://www.mediawiki.org/wiki/Special:MyLanguage/Localisation#Translation_resources Localise MediaWiki for your language]\n* [https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:Combating_spam Learn how to combat spam on your wiki]" diff --git a/includes/installer/i18n/qqq.json b/includes/installer/i18n/qqq.json index d82c74b162..751b42a585 100644 --- a/includes/installer/i18n/qqq.json +++ b/includes/installer/i18n/qqq.json @@ -332,6 +332,7 @@ "config-extension-link": "Shown on last page of installation to inform about possible extensions.\n{{Identical|Did you know}}", "config-skins-screenshots": "Radio button text, $1 is the skin name, and $2 is a list of links to screenshots of that skin", "config-skins-screenshot": "Radio button text, $1 is the skin name, and $2 is a link to a screenshot of that skin, where the link text is {{msg-mw|config-screenshot}}.", + "config-extensions-requires": "Radio button text, $1 is the extension name, and $2 are links to other extensions that this one requires.", "config-screenshot": "Link text for the link in {{msg-mw|config-skins-screenshot}}\n{{Identical|Screenshot}}", "mainpagetext": "Along with {{msg-mw|mainpagedocfooter}}, the text you will see on the Main Page when your wiki is installed.", "mainpagedocfooter": "Along with {{msg-mw|mainpagetext}}, the text you will see on the Main Page when your wiki is installed.\nThis might be a good place to put information about {{GRAMMAR:}}. See [[{{NAMESPACE}}:{{BASEPAGENAME}}/fi]] for an example. For languages having grammatical distinctions and not having an appropriate {{GRAMMAR:}} software available, a suggestion to check and possibly amend the messages having {{SITENAME}} may be valuable. See [[{{NAMESPACE}}:{{BASEPAGENAME}}/ksh]] for an example." diff --git a/includes/registration/ExtensionDependencyError.php b/includes/registration/ExtensionDependencyError.php new file mode 100644 index 0000000000..d380d07761 --- /dev/null +++ b/includes/registration/ExtensionDependencyError.php @@ -0,0 +1,81 @@ + + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +/** + * @since 1.31 + */ +class ExtensionDependencyError extends Exception { + + /** + * @var string[] + */ + public $missingExtensions = []; + + /** + * @var string[] + */ + public $missingSkins = []; + + /** + * @var string[] + */ + public $incompatibleExtensions = []; + + /** + * @var string[] + */ + public $incompatibleSkins = []; + + /** + * @var bool + */ + public $incompatibleCore = false; + + /** + * @param array $errors Each error has a 'msg' and 'type' key at minimum + */ + public function __construct( array $errors ) { + $msg = ''; + foreach ( $errors as $info ) { + $msg .= $info['msg'] . "\n"; + switch ( $info['type'] ) { + case 'incompatible-core': + $this->incompatibleCore = true; + break; + case 'missing-skins': + $this->missingSkins[] = $info['missing']; + break; + case 'missing-extensions': + $this->missingExtensions[] = $info['missing']; + break; + case 'incompatible-skins': + $this->incompatibleSkins[] = $info['incompatible']; + break; + case 'incompatible-extensions': + $this->incompatibleExtensions[] = $info['incompatible']; + break; + // default: continue + } + } + + parent::__construct( $msg ); + } + +} diff --git a/includes/registration/ExtensionRegistry.php b/includes/registration/ExtensionRegistry.php index 1876645399..b000dc11b1 100644 --- a/includes/registration/ExtensionRegistry.php +++ b/includes/registration/ExtensionRegistry.php @@ -202,6 +202,7 @@ class ExtensionRegistry { * @param array $queue keys are filenames, values are ignored * @return array extracted info * @throws Exception + * @throws ExtensionDependencyError */ public function readFromQueue( array $queue ) { global $wgVersion; @@ -273,11 +274,7 @@ class ExtensionRegistry { ); if ( $incompatible ) { - if ( count( $incompatible ) === 1 ) { - throw new Exception( $incompatible[0] ); - } else { - throw new Exception( implode( "\n", $incompatible ) ); - } + throw new ExtensionDependencyError( $incompatible ); } // Need to set this so we can += to it later diff --git a/includes/registration/VersionChecker.php b/includes/registration/VersionChecker.php index 02e3a7c8ab..9c673bc5db 100644 --- a/includes/registration/VersionChecker.php +++ b/includes/registration/VersionChecker.php @@ -110,13 +110,18 @@ class VersionChecker { case ExtensionRegistry::MEDIAWIKI_CORE: $mwError = $this->handleMediaWikiDependency( $values, $extension ); if ( $mwError !== false ) { - $errors[] = $mwError; + $errors[] = [ + 'msg' => $mwError, + 'type' => 'incompatible-core', + ]; } break; case 'extensions': case 'skin': foreach ( $values as $dependency => $constraint ) { - $extError = $this->handleExtensionDependency( $dependency, $constraint, $extension ); + $extError = $this->handleExtensionDependency( + $dependency, $constraint, $extension, $dependencyType + ); if ( $extError !== false ) { $errors[] = $extError; } @@ -164,12 +169,19 @@ class VersionChecker { * @param string $dependencyName The name of the dependency * @param string $constraint The required version constraint for this dependency * @param string $checkedExt The Extension, which depends on this dependency - * @return bool|string false for no errors, or a string message + * @param string $type Either 'extension' or 'skin' + * @return bool|array false for no errors, or an array of info */ - private function handleExtensionDependency( $dependencyName, $constraint, $checkedExt ) { + private function handleExtensionDependency( $dependencyName, $constraint, $checkedExt, + $type + ) { // Check if the dependency is even installed if ( !isset( $this->loaded[$dependencyName] ) ) { - return "{$checkedExt} requires {$dependencyName} to be installed."; + return [ + 'msg' => "{$checkedExt} requires {$dependencyName} to be installed.", + 'type' => "missing-$type", + 'missing' => $dependencyName, + ]; } // Check if the dependency has specified a version if ( !isset( $this->loaded[$dependencyName]['version'] ) ) { @@ -180,8 +192,13 @@ class VersionChecker { return false; } else { // Otherwise, mark it as incompatible. - return "{$dependencyName} does not expose its version, but {$checkedExt}" + $msg = "{$dependencyName} does not expose its version, but {$checkedExt}" . " requires: {$constraint}."; + return [ + 'msg' => $msg, + 'type' => "incompatible-$type", + 'incompatible' => $checkedExt, + ]; } } else { // Try to get a constraint for the dependency version @@ -193,16 +210,24 @@ class VersionChecker { } catch ( UnexpectedValueException $e ) { // Non-parsable version, output an error message that the version // string is invalid - return "$dependencyName does not have a valid version string."; + return [ + 'msg' => "$dependencyName does not have a valid version string.", + 'type' => 'invalid-version', + ]; } // Check if the constraint actually matches... if ( !$this->versionParser->parseConstraints( $constraint )->matches( $installedVersion ) ) { - return "{$checkedExt} is not compatible with the current " + $msg = "{$checkedExt} is not compatible with the current " . "installed version of {$dependencyName} " . "({$this->loaded[$dependencyName]['version']}), " . "it requires: " . $constraint . '.'; + return [ + 'msg' => $msg, + 'type' => "incompatible-$type", + 'incompatible' => $checkedExt, + ]; } } diff --git a/mw-config/config.js b/mw-config/config.js index acb9664aee..ab57b7b859 100644 --- a/mw-config/config.js +++ b/mw-config/config.js @@ -1,3 +1,4 @@ +/* global extDependencyMap */ ( function ( $ ) { $( function () { var $label, labelText; @@ -100,5 +101,52 @@ $memc.hide( 'slow' ); } } ); + + function areReqsSatisfied( name ) { + var i, ext, skin, node; + if ( !extDependencyMap[ name ] ) { + return true; + } + + if ( extDependencyMap[ name ].extensions ) { + for ( i in extDependencyMap[ name ].extensions ) { + ext = extDependencyMap[ name ].extensions[ i ]; + node = document.getElementById( 'config_ext-' + ext ); + if ( !node || !node.checked ) { + return false; + } + } + } + if ( extDependencyMap[ name ].skins ) { + for ( i in extDependencyMap[ name ].skins ) { + skin = extDependencyMap[ name ].skins[ i ]; + node = document.getElementById( 'config_skin-' + skin ); + if ( !node || !node.checked ) { + return false; + } + } + } + + return true; + } + + // Disable checkboxes if the extension has dependencies + $( '.mw-ext-with-dependencies input' ).prop( 'disabled', true ); + $( 'input[data-name]' ).change( function () { + $( '.mw-ext-with-dependencies input' ).each( function () { + var $this = $( this ), + name = $this.data( 'name' ); + if ( areReqsSatisfied( name ) ) { + // Un-disable it! + $this.prop( 'disabled', false ); + } else { + // Disable the checkbox, and uncheck it if it is checked + $this.prop( 'disabled', true ); + if ( $this.prop( 'checked' ) ) { + $this.prop( 'checked', false ); + } + } + } ); + } ); } ); }( jQuery ) ); diff --git a/tests/phpunit/includes/registration/VersionCheckerTest.php b/tests/phpunit/includes/registration/VersionCheckerTest.php index 929ff0fa62..f65fcc77d3 100644 --- a/tests/phpunit/includes/registration/VersionCheckerTest.php +++ b/tests/phpunit/includes/registration/VersionCheckerTest.php @@ -93,7 +93,11 @@ class VersionCheckerTest extends PHPUnit\Framework\TestCase { 'NoVersionGiven' => '1.0', ] ], - [ 'NoVersionGiven does not expose its version, but FakeExtension requires: 1.0.' ], + [ [ + 'incompatible' => 'FakeExtension', + 'type' => 'incompatible-extensions', + 'msg' => 'NoVersionGiven does not expose its version, but FakeExtension requires: 1.0.' + ] ], ], [ [ @@ -101,7 +105,11 @@ class VersionCheckerTest extends PHPUnit\Framework\TestCase { 'Missing' => '*', ] ], - [ 'FakeExtension requires Missing to be installed.' ], + [ [ + 'missing' => 'Missing', + 'type' => 'missing-extensions', + 'msg' => 'FakeExtension requires Missing to be installed.', + ] ], ], [ [ @@ -109,8 +117,12 @@ class VersionCheckerTest extends PHPUnit\Framework\TestCase { 'FakeDependency' => '2.0.0', ] ], - // phpcs:ignore Generic.Files.LineLength.TooLong - [ 'FakeExtension is not compatible with the current installed version of FakeDependency (1.0.0), it requires: 2.0.0.' ], + [ [ + 'incompatible' => 'FakeExtension', + 'type' => 'incompatible-extensions', + // phpcs:ignore Generic.Files.LineLength.TooLong + 'msg' => 'FakeExtension is not compatible with the current installed version of FakeDependency (1.0.0), it requires: 2.0.0.' + ] ], ] ]; } @@ -127,7 +139,11 @@ class VersionCheckerTest extends PHPUnit\Framework\TestCase { 'version' => 'not really valid', ], ] ); - $this->assertEquals( [ "FakeDependency does not have a valid version string." ], + $this->assertEquals( + [ [ + 'type' => 'invalid-version', + 'msg' => "FakeDependency does not have a valid version string." + ] ], $checker->checkArray( [ 'FakeExtension' => [ 'extensions' => [ -- 2.20.1