From 1aa819513ba34521d9ef59978c0e23a4132bcaa1 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Sun, 19 May 2019 02:17:56 -0700 Subject: [PATCH] registration: Add development requirements to extension.json Extensions can specify development dependencies in extension.json under the "dev-requires" key. It's identical to the "requires" field. Any requirement that is needed to pass tests, including but not limited to, PHPUnit, QUnit, structure, phan, should be documented in this new field. The main intention is that CI will ensure that all of these dependencies are satisfied before running tests. At standard runtime, the development requirements will be ignored by MediaWiki, since it only checks for real requirements. Scripts can manually check development requirements by calling ExtensionRegistry::setCheckDevRequires( true ) before trying to load things. If both "requires" and "dev-requires" are present, MediaWiki will merge the two together, so the environment will need to satisfy both before proceeding. Bug: T193824 Change-Id: I9b2936666ee3c96f5c976c7a17f11c437c2c7f48 --- docs/extension.schema.v1.json | 42 +++++++++++ docs/extension.schema.v2.json | 42 +++++++++++ includes/registration/ExtensionProcessor.php | 72 ++++++++++++++++++- includes/registration/ExtensionRegistry.php | 18 ++++- includes/registration/Processor.php | 3 +- .../registration/ExtensionProcessorTest.php | 60 +++++++++++++++- 6 files changed, 231 insertions(+), 6 deletions(-) diff --git a/docs/extension.schema.v1.json b/docs/extension.schema.v1.json index 32946d38f6..cf02f2be2c 100644 --- a/docs/extension.schema.v1.json +++ b/docs/extension.schema.v1.json @@ -95,6 +95,48 @@ } } }, + "dev-requires": { + "type": "object", + "description": "Indicates what dependencies are required for development purposes such as running tests. This syntax may be extended in the future.", + "additionalProperties": false, + "properties": { + "MediaWiki": { + "type": "string", + "description": "Version constraint string against MediaWiki core." + }, + "platform": { + "type": "object", + "description": "Indicates version constraints against platform services.", + "additionalProperties": false, + "properties": { + "php": { + "type": "string", + "description": "Version constraint string against PHP." + }, + "ability-shell": { + "type": "boolean", + "default": false, + "description": "Whether this extension requires shell access." + } + }, + "patternProperties": { + "^ext-": { + "type": "string", + "description": "Required PHP extension.", + "enum": ["*"] + } + } + }, + "extensions": { + "type": "object", + "description": "Set of version constraint strings against specific extensions." + }, + "skins": { + "type": "object", + "description": "Set of version constraint strings against specific skins." + } + } + }, "ResourceFileModulePaths": { "type": "object", "description": "Default paths to use for all ResourceLoader file modules", diff --git a/docs/extension.schema.v2.json b/docs/extension.schema.v2.json index 42b34b7d2a..f29f8501b0 100644 --- a/docs/extension.schema.v2.json +++ b/docs/extension.schema.v2.json @@ -102,6 +102,48 @@ } } }, + "dev-requires": { + "type": "object", + "description": "Indicates what dependencies are required for development purposes such as running tests. This syntax may be extended in the future.", + "additionalProperties": false, + "properties": { + "MediaWiki": { + "type": "string", + "description": "Version constraint string against MediaWiki core." + }, + "platform": { + "type": "object", + "description": "Indicates version constraints against platform services.", + "additionalProperties": false, + "properties": { + "php": { + "type": "string", + "description": "Version constraint string against PHP." + }, + "ability-shell": { + "type": "boolean", + "default": false, + "description": "Whether this extension requires shell access." + } + }, + "patternProperties": { + "^ext-": { + "type": "string", + "description": "Required PHP extension.", + "enum": ["*"] + } + } + }, + "extensions": { + "type": "object", + "description": "Set of version constraint strings against specific extensions." + }, + "skins": { + "type": "object", + "description": "Set of version constraint strings against specific skins." + } + } + }, "ResourceFileModulePaths": { "type": "object", "description": "Default paths to use for all ResourceLoader file modules", diff --git a/includes/registration/ExtensionProcessor.php b/includes/registration/ExtensionProcessor.php index b474ddc766..faaaece456 100644 --- a/includes/registration/ExtensionProcessor.php +++ b/includes/registration/ExtensionProcessor.php @@ -304,8 +304,76 @@ class ExtensionProcessor implements Processor { ]; } - public function getRequirements( array $info ) { - return $info['requires'] ?? []; + public function getRequirements( array $info, $includeDev ) { + // Quick shortcuts + if ( !$includeDev || !isset( $info['dev-requires'] ) ) { + return $info['requires'] ?? []; + } + + if ( !isset( $info['requires'] ) ) { + return $info['dev-requires'] ?? []; + } + + // OK, we actually have to merge everything + $merged = []; + + // Helper that combines version requirements by + // picking the non-null if one is, or combines + // the two. Note that it is not possible for + // both inputs to be null. + $pick = function ( $a, $b ) { + if ( $a === null ) { + return $b; + } elseif ( $b === null ) { + return $a; + } else { + return "$a $b"; + } + }; + + $req = $info['requires']; + $dev = $info['dev-requires']; + if ( isset( $req['MediaWiki'] ) || isset( $dev['MediaWiki'] ) ) { + $merged['MediaWiki'] = $pick( + $req['MediaWiki'] ?? null, + $dev['MediaWiki'] ?? null + ); + } + + $platform = array_merge( + array_keys( $req['platform'] ?? [] ), + array_keys( $dev['platform'] ?? [] ) + ); + if ( $platform ) { + foreach ( $platform as $pkey ) { + if ( $pkey === 'php' ) { + $value = $pick( + $req['platform']['php'] ?? null, + $dev['platform']['php'] ?? null + ); + } else { + // Prefer dev value, but these should be constant + // anyways (ext-* and ability-*) + $value = $dev['platform'][$pkey] ?? $req['platform'][$pkey]; + } + $merged['platform'][$pkey] = $value; + } + } + + foreach ( [ 'extensions', 'skins' ] as $thing ) { + $things = array_merge( + array_keys( $req[$thing] ?? [] ), + array_keys( $dev[$thing] ?? [] ) + ); + foreach ( $things as $name ) { + $merged[$thing][$name] = $pick( + $req[$thing][$name] ?? null, + $dev[$thing][$name] ?? null + ); + } + } + + return $merged; } protected function extractHooks( array $info ) { diff --git a/includes/registration/ExtensionRegistry.php b/includes/registration/ExtensionRegistry.php index fb897314f3..768b488323 100644 --- a/includes/registration/ExtensionRegistry.php +++ b/includes/registration/ExtensionRegistry.php @@ -86,6 +86,13 @@ class ExtensionRegistry { */ protected $testAttributes = []; + /** + * Whether to check dev-requires + * + * @var bool + */ + protected $checkDev = false; + /** * @var ExtensionRegistry */ @@ -103,6 +110,14 @@ class ExtensionRegistry { return self::$instance; } + /** + * @since 1.34 + * @param bool $check + */ + public function setCheckDevRequires( $check ) { + $this->checkDev = $check; + } + /** * @param string $path Absolute path to the JSON file */ @@ -148,6 +163,7 @@ class ExtensionRegistry { 'registration' => self::CACHE_VERSION, 'mediawiki' => $wgVersion, 'abilities' => $this->getAbilities(), + 'checkDev' => $this->checkDev, ]; // We use a try/catch because we don't want to fail here @@ -295,7 +311,7 @@ class ExtensionRegistry { } // get all requirements/dependencies for this extension - $requires = $processor->getRequirements( $info ); + $requires = $processor->getRequirements( $info, $this->checkDev ); // validate the information needed and add the requirements if ( is_array( $requires ) && $requires && isset( $info['name'] ) ) { diff --git a/includes/registration/Processor.php b/includes/registration/Processor.php index 68ba4130a0..51cca365c8 100644 --- a/includes/registration/Processor.php +++ b/includes/registration/Processor.php @@ -36,10 +36,11 @@ interface Processor { * * @since 1.26 * @param array $info + * @param bool $includeDev * @return array Where keys are the name to have a constraint on, * like 'MediaWiki'. Values are a constraint string like "1.26.1". */ - public function getRequirements( array $info ); + public function getRequirements( array $info, $includeDev ); /** * Get the path for additional autoloaders, e.g. the one of Composer. diff --git a/tests/phpunit/includes/registration/ExtensionProcessorTest.php b/tests/phpunit/includes/registration/ExtensionProcessorTest.php index d5a2b3a5a7..cdd5c63eff 100644 --- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php +++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php @@ -717,11 +717,67 @@ class ExtensionProcessorTest extends MediaWikiTestCase { $processor = new ExtensionProcessor(); $this->assertSame( $info['requires'], - $processor->getRequirements( $info ) + $processor->getRequirements( $info, false ) ); $this->assertSame( [], - $processor->getRequirements( [] ) + $processor->getRequirements( [], false ) + ); + } + + public function testGetDevRequirements() { + $info = self::$default + [ + 'dev-requires' => [ + 'MediaWiki' => '>= 1.31.0', + 'platform' => [ + 'ext-foo' => '*', + ], + 'skins' => [ + 'Baz' => '*', + ], + 'extensions' => [ + 'Biz' => '*', + ], + ], + ]; + $processor = new ExtensionProcessor(); + $this->assertSame( + $info['dev-requires'], + $processor->getRequirements( $info, true ) + ); + // Set some standard requirements, so we can test merging + $info['requires'] = [ + 'MediaWiki' => '>= 1.25.0', + 'platform' => [ + 'php' => '>= 5.5.9' + ], + 'extensions' => [ + 'Bar' => '*' + ] + ]; + $this->assertSame( + [ + 'MediaWiki' => '>= 1.25.0 >= 1.31.0', + 'platform' => [ + 'php' => '>= 5.5.9', + 'ext-foo' => '*', + ], + 'extensions' => [ + 'Bar' => '*', + 'Biz' => '*', + ], + 'skins' => [ + 'Baz' => '*', + ], + ], + $processor->getRequirements( $info, true ) + ); + + // If there's no dev-requires, it just returns requires + unset( $info['dev-requires'] ); + $this->assertSame( + $info['requires'], + $processor->getRequirements( $info, true ) ); } -- 2.20.1