From 407d50c860fa5ff73cf342ca74b1a321e628eab5 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 5 Nov 2019 10:07:06 -0500 Subject: [PATCH] Add $wgDiffEngine The immediate use case is for testing, where some tests need to use the PHP implementation even when wikidiff2 is installed. Bug: T237049 Change-Id: I41dc4c0933429065d7638f518ec31f0a056afc41 (cherry picked from commit f3058c81b9bbdede6eeb70503ecc44a6ba423e0d) --- RELEASE-NOTES-1.34 | 4 ++ includes/DefaultSettings.php | 21 ++++-- includes/content/ContentHandler.php | 2 +- includes/diff/DifferenceEngine.php | 65 +++++++++++++------ .../includes/api/ApiComparePagesTest.php | 2 + .../includes/diff/DifferenceEngineTest.php | 2 + ...eEngineSlotDiffRendererIntegrationTest.php | 5 +- 7 files changed, 76 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 27d30284f4..1259628298 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -6,6 +6,10 @@ MediaWiki 1.34 is an pre-release testing branch, and is not recommended for use in production. === Changes since MediaWiki 1.34.0-rc.1 === +* $wgDiffEngine (T237049) – This configuration can be used to specify which + difference engine to use. MediaWiki continues to default to automatically + choosing the first of $wgExternalDiffEngine, wikidiff2, or php that is + usable. == MediaWiki 1.34.0-rc.1 == diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index a10c30b959..b514504b9a 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -8487,10 +8487,23 @@ $wgUpdateRowsPerQuery = 100; */ /** - * Name of the external diff engine to use. Supported values: - * * string: path to an external diff executable - * * false: wikidiff2 PHP/HHVM module if installed, otherwise the default PHP implementation - * * 'wikidiff', 'wikidiff2', and 'wikidiff3' are treated as false for backwards compatibility + * Specify the difference engine to use. + * + * Supported values: + * - 'external': Use an external diff engine, which must be specified via $wgExternalDiffEngine + * - 'wikidiff2': Use the wikidiff2 PHP extension + * - 'php': PHP implementations included in MediaWiki + * + * The default (null) is to use the first engine that's available. + * + * @since 1.34 + * @var string|null + */ +$wgDiffEngine = null; + +/** + * Name of the external diff engine to use. + * @var string|false Path to an external diff executable */ $wgExternalDiffEngine = false; diff --git a/includes/content/ContentHandler.php b/includes/content/ContentHandler.php index 533f6397c5..0e8b5bcf5d 100644 --- a/includes/content/ContentHandler.php +++ b/includes/content/ContentHandler.php @@ -651,7 +651,7 @@ abstract class ContentHandler { $slotDiffRenderer->setLanguage( $contentLanguage ); $engine = DifferenceEngine::getEngine(); - if ( $engine === false ) { + if ( $engine === 'php' ) { $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP ); } elseif ( $engine === 'wikidiff2' ) { $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2 ); diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index 42354889d2..af01ca1d01 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -1178,7 +1178,7 @@ class DifferenceEngine extends ContextSource { $engine = $this->getEngine(); $params = [ 'diff', - $engine, + $engine === 'php' ? false : $engine, // Back compat self::DIFF_VERSION, "old-{$this->mOldid}", "rev-{$this->mNewid}" @@ -1288,32 +1288,59 @@ class DifferenceEngine extends ContextSource { } /** - * Process ExternalDiffEngine config and get a sane, usable engine + * Process DiffEngine config and get a sane, usable engine * - * @return bool|string 'wikidiff2', path to an executable, or false + * @return string 'wikidiff2', 'php', or path to an executable * @internal For use by this class and TextSlotDiffRenderer only. */ public static function getEngine() { + $diffEngine = MediaWikiServices::getInstance()->getMainConfig() + ->get( 'DiffEngine' ); $externalDiffEngine = MediaWikiServices::getInstance()->getMainConfig() ->get( 'ExternalDiffEngine' ); - if ( $externalDiffEngine ) { - if ( is_string( $externalDiffEngine ) ) { - if ( is_executable( $externalDiffEngine ) ) { - return $externalDiffEngine; - } - wfDebug( 'ExternalDiffEngine config points to a non-executable, ignoring' ); - } else { - wfWarn( 'ExternalDiffEngine config is set to a non-string value, ignoring' ); - } - } + if ( $diffEngine === null ) { + $engines = [ 'external', 'wikidiff2', 'php' ]; + } else { + $engines = [ $diffEngine ]; + } + + $failureReason = null; + foreach ( $engines as $engine ) { + switch ( $engine ) { + case 'external': + if ( is_string( $externalDiffEngine ) ) { + if ( is_executable( $externalDiffEngine ) ) { + return $externalDiffEngine; + } + $failureReason = 'ExternalDiffEngine config points to a non-executable'; + if ( $diffEngine === null ) { + wfDebug( "$failureReason, ignoring" ); + } + } else { + $failureReason = 'ExternalDiffEngine config is set to a non-string value'; + if ( $diffEngine === null && $externalDiffEngine ) { + wfWarn( "$failureReason, ignoring" ); + } + } + break; - if ( function_exists( 'wikidiff2_do_diff' ) ) { - return 'wikidiff2'; - } + case 'wikidiff2': + if ( function_exists( 'wikidiff2_do_diff' ) ) { + return 'wikidiff2'; + } + $failureReason = 'wikidiff2 is not available'; + break; - // Native PHP - return false; + case 'php': + // Always available. + return 'php'; + + default: + throw new DomainException( 'Invalid value for $wgDiffEngine: ' . $engine ); + } + } + throw new UnexpectedValueException( "Cannot use diff engine '$engine': $failureReason" ); } /** @@ -1367,7 +1394,7 @@ class DifferenceEngine extends ContextSource { $engine = self::getEngine(); if ( $engine === 'wikidiff2' ) { return $this->debug( 'wikidiff2' ); - } elseif ( $engine === false ) { + } elseif ( $engine === 'php' ) { return $this->debug( 'native PHP' ); } else { return $this->debug( "external $engine" ); diff --git a/tests/phpunit/includes/api/ApiComparePagesTest.php b/tests/phpunit/includes/api/ApiComparePagesTest.php index 9b32e9e5d1..26c3f8fa18 100644 --- a/tests/phpunit/includes/api/ApiComparePagesTest.php +++ b/tests/phpunit/includes/api/ApiComparePagesTest.php @@ -121,6 +121,8 @@ class ApiComparePagesTest extends ApiTestCase { * @dataProvider provideDiff */ public function testDiff( $params, $expect, $exceptionCode = false, $sysop = false ) { + $this->setMwGlobals( [ 'wgDiffEngine' => 'php' ] ); + $this->doReplacements( $params ); $params += [ diff --git a/tests/phpunit/includes/diff/DifferenceEngineTest.php b/tests/phpunit/includes/diff/DifferenceEngineTest.php index 46eaaeaa9d..43fe53fb74 100644 --- a/tests/phpunit/includes/diff/DifferenceEngineTest.php +++ b/tests/phpunit/includes/diff/DifferenceEngineTest.php @@ -32,6 +32,8 @@ class DifferenceEngineTest extends MediaWikiTestCase { if ( !self::$revisions ) { self::$revisions = $this->doEdits(); } + + $this->setMwGlobals( [ 'wgDiffEngine' => 'php' ] ); } /** diff --git a/tests/phpunit/integration/includes/diff/DifferenceEngineSlotDiffRendererIntegrationTest.php b/tests/phpunit/integration/includes/diff/DifferenceEngineSlotDiffRendererIntegrationTest.php index a211001238..e0e321e191 100644 --- a/tests/phpunit/integration/includes/diff/DifferenceEngineSlotDiffRendererIntegrationTest.php +++ b/tests/phpunit/integration/includes/diff/DifferenceEngineSlotDiffRendererIntegrationTest.php @@ -9,7 +9,10 @@ class DifferenceEngineSlotDiffRendererIntegrationTest extends \MediaWikiIntegrat * @covers DifferenceEngineSlotDiffRenderer::getExtraCacheKeys */ public function testGetExtraCacheKeys_noExternalDiffEngineConfigured() { - $this->setMwGlobals( [ 'wgExternalDiffEngine' => null ] ); + $this->setMwGlobals( [ + 'wgDiffEngine' => null, + 'wgExternalDiffEngine' => null, + ] ); $differenceEngine = new CustomDifferenceEngine(); $slotDiffRenderer = new DifferenceEngineSlotDiffRenderer( $differenceEngine ); -- 2.20.1