From 15b894bdbc2cf503ec785eb89c631bc8f34ae889 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Fri, 8 Dec 2017 12:57:08 -0800 Subject: [PATCH] Drop symfony/process dependency, use Shell\Command instead We originally started using symfony/process because kzykhys/pygments depended upon it. But that library was unmaintained and became broken, so we stopped using it, and just used symfony/process directly. At the time, the main reason in favor of symfony/process was that it could pass stdin to pygments, while Shell\Command couldn't - but it can now (T182463)! On top of that, there are downsides, like not respecting the default MediaWiki shell limits, being incompatible with core's firejail support, and requiring an external composer dependency. Note that because Shell::command() will enforce MediaWiki's normal limits, it's possible that some large pages may no longer render with syntax highlighting if they pass those limits. Bug: T182467 Bug: T181771 Change-Id: Ie1cb72b7eb17d943f79ecae4d94a2110546ef039 --- README | 5 +---- SyntaxHighlight.php | 32 +++++++++++--------------------- composer.json | 3 --- extension.json | 3 +-- maintenance/updateCSS.php | 24 ++++++++++-------------- maintenance/updateLexerList.php | 17 ++++++++--------- 6 files changed, 31 insertions(+), 53 deletions(-) diff --git a/README b/README index 61e774d1..1384e9f4 100644 --- a/README +++ b/README @@ -14,10 +14,7 @@ with earlier versions of MediaWiki, visit: == Installation == -First, you will need to ensure that this extension's Composer-managed -dependencies are available. In the extension directory, run 'composer update'. - -Next, Add this line to your LocalSettings.php: +Add this line to your LocalSettings.php: wfLoadExtension( 'SyntaxHighlight_GeSHi' ); diff --git a/SyntaxHighlight.php b/SyntaxHighlight.php index ee7eda5b..f17d5312 100644 --- a/SyntaxHighlight.php +++ b/SyntaxHighlight.php @@ -17,7 +17,6 @@ */ use MediaWiki\Shell\Shell; -use Symfony\Component\Process\ProcessBuilder; class SyntaxHighlight { @@ -288,32 +287,23 @@ class SyntaxHighlight { foreach ( $options as $k => $v ) { $optionPairs[] = "{$k}={$v}"; } - $builder = new ProcessBuilder(); - $builder->setPrefix( self::getPygmentizePath() ); - $process = $builder - ->add( '-l' )->add( $lexer ) - ->add( '-f' )->add( 'html' ) - ->add( '-O' )->add( implode( ',', $optionPairs ) ) - ->getProcess(); + $result = Shell::command( + self::getPygmentizePath(), + '-l', $lexer, + '-f', 'html', + '-O', implode( ',', $optionPairs ) + ) + ->input( $code ) + ->execute(); - $process->setInput( $code ); - - /* Workaround for T151523 (buggy $process->getOutput()). - If/when this issue is fixed in HHVM or Symfony, - replace this with "$process->run(); $output = $process->getOutput();" - */ - $output = ''; - $process->run( function ( $type, $capturedOutput ) use ( &$output ) { - $output .= $capturedOutput; - } ); - - if ( !$process->isSuccessful() ) { + if ( $result->getExitCode() != 0 ) { $status->warning( 'syntaxhighlight-error-pygments-invocation-failure' ); - wfWarn( 'Failed to invoke Pygments: ' . $process->getErrorOutput() ); + wfWarn( 'Failed to invoke Pygments: ' . $result->getStderr() ); $status->value = self::highlight( $code, null, $args )->getValue(); return $status; } + $output = $result->getStdout(); $cache->set( $cacheKey, $output ); } diff --git a/composer.json b/composer.json index e4c5736e..dc73239d 100644 --- a/composer.json +++ b/composer.json @@ -1,9 +1,6 @@ { "name": "mediawiki/syntax-highlight", "description": "Syntax highlighting extension for MediaWiki", - "require": { - "symfony/process": "~3.3" - }, "require-dev": { "jakub-onderka/php-parallel-lint": "0.9.2", "mediawiki/mediawiki-codesniffer": "15.0.0", diff --git a/extension.json b/extension.json index 6c755149..fc4316cd 100644 --- a/extension.json +++ b/extension.json @@ -14,7 +14,7 @@ "license-name": "GPL-2.0+", "type": "parserhook", "requires": { - "MediaWiki": ">= 1.27" + "MediaWiki": ">= 1.31" }, "MessagesDirs": { "SyntaxHighlight_GeSHi": [ @@ -77,6 +77,5 @@ "ParserTestFiles": [ "tests/parserTests.txt" ], - "load_composer_autoloader": true, "manifest_version": 1 } diff --git a/maintenance/updateCSS.php b/maintenance/updateCSS.php index 0bc86de0..f72a454b 100644 --- a/maintenance/updateCSS.php +++ b/maintenance/updateCSS.php @@ -22,7 +22,7 @@ * @ingroup Maintenance */ -use Symfony\Component\Process\ProcessBuilder; +use MediaWiki\Shell\Shell; $IP = getenv( 'MW_INSTALL_PATH' ) ?: __DIR__ . '/../../..'; @@ -39,22 +39,18 @@ class UpdateCSS extends Maintenance { $target = __DIR__ . '/../modules/pygments.generated.css'; $css = "/* Stylesheet generated by updateCSS.php */\n"; - $builder = new ProcessBuilder(); - $builder->setPrefix( SyntaxHighlight::getPygmentizePath() ); + $result = Shell::command( + SyntaxHighlight::getPygmentizePath(), + '-f', 'html', + '-S', 'default', + '-a', '.' . SyntaxHighlight::HIGHLIGHT_CSS_CLASS + )->execute(); - $process = $builder - ->add( '-f' )->add( 'html' ) - ->add( '-S' )->add( 'default' ) - ->add( '-a' )->add( '.' . SyntaxHighlight::HIGHLIGHT_CSS_CLASS ) - ->getProcess(); - - $process->run(); - - if ( !$process->isSuccessful() ) { - throw new \RuntimeException( $process->getErrorOutput() ); + if ( $result->getExitCode() != 0 ) { + throw new \RuntimeException( $result->getStderr() ); } - $css .= $process->getOutput(); + $css .= $result->getStdout(); if ( file_put_contents( $target, $css ) === false ) { $this->output( "Failed to write to {$target}\n" ); diff --git a/maintenance/updateLexerList.php b/maintenance/updateLexerList.php index 41a93473..4e1b2d19 100644 --- a/maintenance/updateLexerList.php +++ b/maintenance/updateLexerList.php @@ -22,7 +22,7 @@ * @ingroup Maintenance */ -use Symfony\Component\Process\ProcessBuilder; +use MediaWiki\Shell\Shell; $IP = getenv( 'MW_INSTALL_PATH' ) ?: __DIR__ . '/../../..'; @@ -43,17 +43,16 @@ class UpdateLexerList extends Maintenance { $lexers = []; - $builder = new ProcessBuilder(); - $builder->setPrefix( SyntaxHighlight::getPygmentizePath() ); + $result = Shell::command( + SyntaxHighlight::getPygmentizePath(), + '-L', 'lexer' + )->execute(); - $process = $builder->add( '-L' )->add( 'lexer' )->getProcess(); - $process->run(); - - if ( !$process->isSuccessful() ) { - throw new \RuntimeException( $process->getErrorOutput() ); + if ( $result->getExitCode() != 0 ) { + throw new \RuntimeException( $result->getStderr() ); } - $output = $process->getOutput(); + $output = $result->getStdout(); foreach ( explode( "\n", $output ) as $line ) { if ( substr( $line, 0, 1 ) === '*' ) { $newLexers = explode( ', ', trim( $line, "* :\n" ) );