From 90698a878b5b0dfdaffd06a0cb5ac6b1465bcbdb Mon Sep 17 00:00:00 2001 From: Florian Schmidt Date: Sat, 3 Dec 2016 19:19:25 +0100 Subject: [PATCH] registration: Allow specifying extension dependencies There are some extensoins that depend upon another extension or skin, usually in different ways: * A constant that is added in the dependency extension, and the existence of is checked for. This is problematic because it requires a specific load order. * Checking whether a specific class exists. This is problematic because it is extremely fragile, and breaks whenever the class is renamed. * Checking ExtensionRegistry::isLoaded(). This is mostly there, but it only checks at runtime, and doesn't provide any machine readable data. Furthermore, developers implement each one differently, with very little standardization. With this, extensions may now specify what other extensions they depend on. This is for explicit *hard* dependencies that must be installed. For example: "requires": { "MediaWiki": ">= 1.25.0", "extensions": { "FakeExtension": "*" }, "skins": { "FakeSkin": "*" } } This would add a minimum requirement on MediaWiki 1.25.0+ (already implemented), as well as the requirement that the FakeExtension extension needs to be installed, as well as the FakeSkin skin. A wildcard (*) is used instead of an explicit version requirement as many extensions do not actually version themselves, and there is no consistent versioning scheme yet. Bug: T117277 Change-Id: If1cccee1a16a867a71bb0285691c400443d8a30a --- docs/extension.schema.json | 10 ++- includes/registration/ExtensionRegistry.php | 39 ++++---- includes/registration/VersionChecker.php | 88 ++++++++++++++++++- .../registration/VersionCheckerTest.php | 78 ++++++++++++++++ 4 files changed, 197 insertions(+), 18 deletions(-) diff --git a/docs/extension.schema.json b/docs/extension.schema.json index 30feaef245..a5543d12a6 100644 --- a/docs/extension.schema.json +++ b/docs/extension.schema.json @@ -55,11 +55,19 @@ }, "requires": { "type": "object", - "description": "Indicates what versions of MediaWiki core are required. This syntax may be extended in the future, for example to check dependencies between other extensions.", + "description": "Indicates what versions of MediaWiki core or extensions are required. This syntax may be extended in the future, for example to check dependencies between other services.", "properties": { "MediaWiki": { "type": "string", "description": "Version constraint string against MediaWiki core." + }, + "extensions": { + "type": "object", + "description": "Set of version constraint strings against specific extensions." + }, + "skins": { + "type": "object", + "description": "Set of version constraint strings against specific skins." } } }, diff --git a/includes/registration/ExtensionRegistry.php b/includes/registration/ExtensionRegistry.php index 0521f3b263..76d25b68e6 100644 --- a/includes/registration/ExtensionRegistry.php +++ b/includes/registration/ExtensionRegistry.php @@ -203,9 +203,10 @@ class ExtensionRegistry { $autoloadClasses = []; $autoloaderPaths = []; $processor = new ExtensionProcessor(); + $versionChecker = new VersionChecker(); + $versionChecker->setCoreVersion( $wgVersion ); + $extDependencies = []; $incompatible = []; - $versionParser = new VersionChecker(); - $versionParser->setCoreVersion( $wgVersion ); foreach ( $queue as $path => $mtime ) { $json = file_get_contents( $path ); if ( $json === false ) { @@ -216,25 +217,13 @@ class ExtensionRegistry { throw new Exception( "$path is not a valid JSON file." ); } - // Check any constraints against MediaWiki core - $requires = $processor->getRequirements( $info ); - if ( $requires ) { - $versionCheck = $versionParser->checkArray( - [ $info['name'] => $requires ] - ); - $incompatible = array_merge( $incompatible, $versionCheck ); - if ( $versionCheck ) { - continue; - } - } - if ( !isset( $info['manifest_version'] ) ) { // For backwards-compatability, assume a version of 1 $info['manifest_version'] = 1; } $version = $info['manifest_version']; if ( $version < self::OLDEST_MANIFEST_VERSION || $version > self::MANIFEST_VERSION ) { - throw new Exception( "$path: unsupported manifest_version: {$version}" ); + $incompatible[] = "$path: unsupported manifest_version: {$version}"; } $autoload = $this->processAutoLoader( dirname( $path ), $info ); @@ -242,12 +231,30 @@ class ExtensionRegistry { $GLOBALS['wgAutoloadClasses'] += $autoload; $autoloadClasses += $autoload; + // get all requirements/dependencies for this extension + $requires = $processor->getRequirements( $info ); + + // validate the information needed and add the requirements + if ( is_array( $requires ) && $requires && isset( $info['name'] ) ) { + $extDependencies[$info['name']] = $requires; + } + // Get extra paths for later inclusion $autoloaderPaths = array_merge( $autoloaderPaths, $processor->getExtraAutoloaderPaths( dirname( $path ), $info ) ); // Compatible, read and extract info $processor->extractInfo( $path, $info, $version ); } + $data = $processor->getExtractedInfo(); + + // check for incompatible extensions + $incompatible = array_merge( + $incompatible, + $versionChecker + ->setLoadedExtensionsAndSkins( $data['credits'] ) + ->checkArray( $extDependencies ) + ); + if ( $incompatible ) { if ( count( $incompatible ) === 1 ) { throw new Exception( $incompatible[0] ); @@ -255,7 +262,7 @@ class ExtensionRegistry { throw new Exception( implode( "\n", $incompatible ) ); } } - $data = $processor->getExtractedInfo(); + // Need to set this so we can += to it later $data['globals']['wgAutoloadClasses'] = []; $data['autoload'] = $autoloadClasses; diff --git a/includes/registration/VersionChecker.php b/includes/registration/VersionChecker.php index b61a10e4b2..2a9401eab6 100644 --- a/includes/registration/VersionChecker.php +++ b/includes/registration/VersionChecker.php @@ -35,6 +35,11 @@ class VersionChecker { */ private $coreVersion = false; + /** + * @var array Loaded extensions + */ + private $loaded = []; + /** * @var VersionParser */ @@ -44,6 +49,18 @@ class VersionChecker { $this->versionParser = new VersionParser(); } + /** + * Set an array with credits of all loaded extensions and skins. + * + * @param array $credits An array of installed extensions with credits of them + * @return VersionChecker $this + */ + public function setLoadedExtensionsAndSkins( array $credits ) { + $this->loaded = $credits; + + return $this; + } + /** * Set MediaWiki core version. * @@ -71,7 +88,13 @@ class VersionChecker { * Example $extDependencies: * { * 'GoogleAPIClient' => { - * 'MediaWiki' => '>= 1.25.0' + * 'MediaWiki' => '>= 1.25.0', + * 'extensions' => { + * 'FakeExtension' => '>= 1.25.0' + * }, + * 'skins' => { + * 'FakeSkin' => '>= 1.0.0' + * } * } * } * @@ -89,6 +112,15 @@ class VersionChecker { $this->handleMediaWikiDependency( $values, $extension ) ); break; + case 'extensions': + case 'skin': + foreach ( $values as $dependency => $constraint ) { + $errors = array_merge( + $errors, + $this->handleExtensionDependency( $dependency, $constraint, $extension ) + ); + } + break; default: throw new UnexpectedValueException( 'Dependency type ' . $dependencyType . ' unknown in ' . $extension ); @@ -125,4 +157,58 @@ class VersionChecker { . "MediaWiki core (version {$this->coreVersion->getPrettyString()}), it requires: " . $constraint . '.' ]; } + + /** + * Handle a dependency to another extension. + * + * @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 array An empty array, if installed version is compatible with $constraint, an array + * with an error message, otherwise. + */ + private function handleExtensionDependency( $dependencyName, $constraint, $checkedExt ) { + $incompatible = []; + // Check if the dependency is even installed + if ( !isset( $this->loaded[$dependencyName] ) ) { + $incompatible[] = "{$checkedExt} requires {$dependencyName} to be installed."; + return $incompatible; + } + // Check if the dependency has specified a version + if ( !isset( $this->loaded[$dependencyName]['version'] ) ) { + // If we depend upon any version, and none is set, that's fine. + if ( $constraint === '*' ) { + wfDebug( "{$dependencyName} does not expose it's version, but {$checkedExt} + mentions it with constraint '*'. Assume it's ok so." ); + } else { + // Otherwise, mark it as incompatible. + $incompatible[] = "{$dependencyName} does not expose it's version, but {$checkedExt} + requires: {$constraint}."; + } + } else { + // Try to get a constraint for the dependency version + try { + $installedVersion = new Constraint( + '==', + $this->versionParser->normalize( $this->loaded[$dependencyName]['version'] ) + ); + } catch ( UnexpectedValueException $e ) { + // Non-parsable version, don't fatal, output an error message that the version + // string is invalid + return [ "Dependency $dependencyName provides an invalid version string." ]; + } + // Check if the constraint actually matches... + if ( + isset( $installedVersion ) && + !$this->versionParser->parseConstraints( $constraint )->matches( $installedVersion ) + ) { + $incompatible[] = "{$checkedExt} is not compatible with the current " + . "installed version of {$dependencyName} " + . "({$this->loaded[$dependencyName]['version']}), " + . "it requires: " . $constraint . '.'; + } + } + + return $incompatible; + } } diff --git a/tests/phpunit/includes/registration/VersionCheckerTest.php b/tests/phpunit/includes/registration/VersionCheckerTest.php index daa407f3b2..2bb1fe4e59 100644 --- a/tests/phpunit/includes/registration/VersionCheckerTest.php +++ b/tests/phpunit/includes/registration/VersionCheckerTest.php @@ -41,4 +41,82 @@ class CoreVersionCheckerTest extends PHPUnit_Framework_TestCase { [ 'totallyinvalid', '== 1.0', true ], ]; } + + /** + * @dataProvider provideType + */ + public function testType( $given, $expected ) { + $checker = new VersionChecker(); + $checker + ->setCoreVersion( '1.0.0' ) + ->setLoadedExtensionsAndSkins( [ + 'FakeDependency' => [ + 'version' => '1.0.0', + ], + ] ); + $this->assertEquals( $expected, $checker->checkArray( [ + 'FakeExtension' => $given, + ] ) + ); + } + + public static function provideType() { + return [ + // valid type + [ + [ + 'extensions' => [ + 'FakeDependency' => '1.0.0' + ] + ], + [] + ], + [ + [ + 'MediaWiki' => '1.0.0' + ], + [] + ], + ]; + } + + /** + * Check, if a non-parsable version constraint does not throw an exception or + * returns any error message. + */ + public function testInvalidConstraint() { + $checker = new VersionChecker(); + $checker + ->setCoreVersion( '1.0.0' ) + ->setLoadedExtensionsAndSkins( [ + 'FakeDependency' => [ + 'version' => 'not really valid', + ], + ] ); + $this->assertEquals( [ "Dependency FakeDependency provides an invalid version string." ], + $checker->checkArray( [ + 'FakeExtension' => [ + 'extensions' => [ + 'FakeDependency' => '1.24.3', + ], + ], + ] ) + ); + + $checker = new VersionChecker(); + $checker + ->setCoreVersion( '1.0.0' ) + ->setLoadedExtensionsAndSkins( [ + 'FakeDependency' => [ + 'version' => '1.24.3', + ], + ] ); + + $this->setExpectedException( 'UnexpectedValueException' ); + $checker->checkArray( [ + 'FakeExtension' => [ + 'FakeDependency' => 'not really valid', + ] + ] ); + } } -- 2.20.1