From 0e1fc6575e00a9bb6d1f671f5df54b2faade518f Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Fri, 20 Oct 2017 00:36:03 -0700 Subject: [PATCH] Refactor some Installer code into ExecutableFinder Refactor Installer::locateExecutableInDefaultPaths() into a separate utility class, ExecutableFinder. This class is already used in plenty of places outside of the installer, so it's ripe for being extracted. This class is located in utils/ due to the dependency upon Shell::command(). Once that no longer has a dependence upon MediaWiki, this class can be moved to libs/ too. Change-Id: I175465acc0d64f990445ce05fabcee8b88a0b259 --- RELEASE-NOTES-1.31 | 2 + autoload.php | 1 + includes/installer/Installer.php | 103 +++---------------- includes/utils/ExecutableFinder.php | 110 +++++++++++++++++++++ maintenance/Maintenance.php | 2 +- tests/parser/TidySupport.php | 2 +- tests/phpunit/maintenance/DumpTestCase.php | 2 +- 7 files changed, 128 insertions(+), 94 deletions(-) create mode 100644 includes/utils/ExecutableFinder.php diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31 index 60dd49558a..a4ce4810ac 100644 --- a/RELEASE-NOTES-1.31 +++ b/RELEASE-NOTES-1.31 @@ -60,6 +60,8 @@ changes to languages because of Phabricator reports. * The Article::selectFields(), Article::onArticleCreate(), Article::onArticleDelete(), and Article::onArticleEdit() methods, deprecated in 1.24, were removed. +* Installer::locateExecutable() and Installer::locateExecutableInDefaultPaths() + were removed, use ExecutableFinder::findInDefaultPaths() instead. == Compatibility == MediaWiki 1.31 requires PHP 5.5.9 or later. There is experimental support for diff --git a/autoload.php b/autoload.php index cf4a115a9b..3bee4116d5 100644 --- a/autoload.php +++ b/autoload.php @@ -441,6 +441,7 @@ $wgAutoloadLocalClasses = [ 'EventRelayerGroup' => __DIR__ . '/includes/EventRelayerGroup.php', 'EventRelayerKafka' => __DIR__ . '/includes/libs/eventrelayer/EventRelayerKafka.php', 'EventRelayerNull' => __DIR__ . '/includes/libs/eventrelayer/EventRelayerNull.php', + 'ExecutableFinder' => __DIR__ . '/includes/utils/ExecutableFinder.php', 'Exif' => __DIR__ . '/includes/media/Exif.php', 'ExifBitmapHandler' => __DIR__ . '/includes/media/ExifBitmap.php', 'ExplodeIterator' => __DIR__ . '/includes/libs/ExplodeIterator.php', diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index 012b477578..e99ea7c049 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -893,10 +893,13 @@ abstract class Installer { * @return bool */ protected function envCheckDiff3() { - $names = [ "gdiff3", "diff3", "diff3.exe" ]; - $versionInfo = [ '$1 --version 2>&1', 'GNU diffutils' ]; + $names = [ "gdiff3", "diff3" ]; + if ( wfIsWindows() ) { + $names[] = 'diff3.exe'; + } + $versionInfo = [ '--version', 'GNU diffutils' ]; - $diff3 = self::locateExecutableInDefaultPaths( $names, $versionInfo ); + $diff3 = ExecutableFinder::findInDefaultPaths( $names, $versionInfo ); if ( $diff3 ) { $this->setVar( 'wgDiff3', $diff3 ); @@ -913,9 +916,9 @@ abstract class Installer { * @return bool */ protected function envCheckGraphics() { - $names = [ wfIsWindows() ? 'convert.exe' : 'convert' ]; - $versionInfo = [ '$1 -version', 'ImageMagick' ]; - $convert = self::locateExecutableInDefaultPaths( $names, $versionInfo ); + $names = wfIsWindows() ? 'convert.exe' : 'convert'; + $versionInfo = [ '-version', 'ImageMagick' ]; + $convert = ExecutableFinder::findInDefaultPaths( $names, $versionInfo ); $this->setVar( 'wgImageMagickConvertCommand', '' ); if ( $convert ) { @@ -939,10 +942,10 @@ abstract class Installer { * @return bool */ protected function envCheckGit() { - $names = [ wfIsWindows() ? 'git.exe' : 'git' ]; - $versionInfo = [ '$1 --version', 'git version' ]; + $names = wfIsWindows() ? 'git.exe' : 'git'; + $versionInfo = [ '--version', 'git version' ]; - $git = self::locateExecutableInDefaultPaths( $names, $versionInfo ); + $git = ExecutableFinder::findInDefaultPaths( $names, $versionInfo ); if ( $git ) { $this->setVar( 'wgGitBin', $git ); @@ -1191,88 +1194,6 @@ abstract class Installer { $this->setVar( 'IP', $IP ); } - /** - * Get an array of likely places we can find executables. Check a bunch - * of known Unix-like defaults, as well as the PATH environment variable - * (which should maybe make it work for Windows?) - * - * @return array - */ - protected static function getPossibleBinPaths() { - return array_merge( - [ '/usr/bin', '/usr/local/bin', '/opt/csw/bin', - '/usr/gnu/bin', '/usr/sfw/bin', '/sw/bin', '/opt/local/bin' ], - explode( PATH_SEPARATOR, getenv( 'PATH' ) ) - ); - } - - /** - * Search a path for any of the given executable names. Returns the - * executable name if found. Also checks the version string returned - * by each executable. - * - * Used only by environment checks. - * - * @param string $path Path to search - * @param array $names Array of executable names - * @param array|bool $versionInfo False or array with two members: - * 0 => Command to run for version check, with $1 for the full executable name - * 1 => String to compare the output with - * - * If $versionInfo is not false, only executables with a version - * matching $versionInfo[1] will be returned. - * @return bool|string - */ - public static function locateExecutable( $path, $names, $versionInfo = false ) { - if ( !is_array( $names ) ) { - $names = [ $names ]; - } - - foreach ( $names as $name ) { - $command = $path . DIRECTORY_SEPARATOR . $name; - - MediaWiki\suppressWarnings(); - $file_exists = is_executable( $command ); - MediaWiki\restoreWarnings(); - - if ( $file_exists ) { - if ( !$versionInfo ) { - return $command; - } - - $file = str_replace( '$1', wfEscapeShellArg( $command ), $versionInfo[0] ); - if ( strstr( wfShellExec( $file ), $versionInfo[1] ) !== false ) { - return $command; - } - } - } - - return false; - } - - /** - * Same as locateExecutable(), but checks in getPossibleBinPaths() by default - * @see locateExecutable() - * @param array $names Array of possible names. - * @param array|bool $versionInfo Default: false or array with two members: - * 0 => Command to run for version check, with $1 for the full executable name - * 1 => String to compare the output with - * - * If $versionInfo is not false, only executables with a version - * matching $versionInfo[1] will be returned. - * @return bool|string - */ - public static function locateExecutableInDefaultPaths( $names, $versionInfo = false ) { - foreach ( self::getPossibleBinPaths() as $path ) { - $exe = self::locateExecutable( $path, $names, $versionInfo ); - if ( $exe !== false ) { - return $exe; - } - } - - return false; - } - /** * Checks if scripts located in the given directory can be executed via the given URL. * diff --git a/includes/utils/ExecutableFinder.php b/includes/utils/ExecutableFinder.php new file mode 100644 index 0000000000..93f635d9a6 --- /dev/null +++ b/includes/utils/ExecutableFinder.php @@ -0,0 +1,110 @@ + + * + * 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. + * + */ + +use MediaWiki\Shell\Shell; + +/** + * Utility class to find executables in likely places + * + * @since 1.31 + */ +class ExecutableFinder { + + /** + * Get an array of likely places we can find executables. Check a bunch + * of known Unix-like defaults, as well as the PATH environment variable + * (which should maybe make it work for Windows?) + * + * @return array + */ + protected static function getPossibleBinPaths() { + return array_unique( array_merge( + [ '/usr/bin', '/bin', '/usr/local/bin', '/opt/csw/bin', + '/usr/gnu/bin', '/usr/sfw/bin', '/sw/bin', '/opt/local/bin' ], + explode( PATH_SEPARATOR, getenv( 'PATH' ) ) + ) ); + } + + /** + * Search a path for any of the given executable names. Returns the + * executable name if found. Also checks the version string returned + * by each executable. + * + * Used only by environment checks. + * + * @param string $path Path to search + * @param string $name Executable name to look for + * @param array|bool $versionInfo False or array with two members: + * 0 => Parameter to pass to binary for version check (e.g. --version) + * 1 => String to compare the output with + * + * If $versionInfo is not false, only executables with a version + * matching $versionInfo[1] will be returned. + * @return bool|string + */ + protected static function findExecutable( $path, $name, $versionInfo = false ) { + $command = $path . DIRECTORY_SEPARATOR . $name; + + MediaWiki\suppressWarnings(); + $file_exists = is_executable( $command ); + MediaWiki\restoreWarnings(); + + if ( $file_exists ) { + if ( !$versionInfo ) { + return $command; + } + + $output = Shell::command( $command, $versionInfo[0] ) + ->includeStderr()->execute()->getStdout(); + if ( strstr( $output, $versionInfo[1] ) !== false ) { + return $command; + } + } + + return false; + } + + /** + * Same as locateExecutable(), but checks in getPossibleBinPaths() by default + * @see locateExecutable() + * @param string|string[] $names Array of possible names. + * @param array|bool $versionInfo Default: false or array with two members: + * 0 => Parameter to run for version check, e.g. '--version' + * 1 => String to compare the output with + * + * If $versionInfo is not false, only executables with a version + * matching $versionInfo[1] will be returned. + * @return bool|string + */ + public static function findInDefaultPaths( $names, $versionInfo = false ) { + $paths = self::getPossibleBinPaths(); + foreach ( (array)$names as $name ) { + foreach ( $paths as $path ) { + $exe = self::findExecutable( $path, $name, $versionInfo ); + if ( $exe !== false ) { + return $exe; + } + } + } + + return false; + } + +} diff --git a/maintenance/Maintenance.php b/maintenance/Maintenance.php index 7de0ae4ccb..174b9732dd 100644 --- a/maintenance/Maintenance.php +++ b/maintenance/Maintenance.php @@ -1496,7 +1496,7 @@ abstract class Maintenance { * @return string */ private static function readlineEmulation( $prompt ) { - $bash = Installer::locateExecutableInDefaultPaths( [ 'bash' ] ); + $bash = ExecutableFinder::findInDefaultPaths( 'bash' ); if ( !wfIsWindows() && $bash ) { $retval = false; $encPrompt = wfEscapeShellArg( $prompt ); diff --git a/tests/parser/TidySupport.php b/tests/parser/TidySupport.php index 6aed02f304..c0a9312ba9 100644 --- a/tests/parser/TidySupport.php +++ b/tests/parser/TidySupport.php @@ -66,7 +66,7 @@ class TidySupport { $this->config['driver'] = 'RaggettExternal'; $this->config['tidyBin'] = $wgTidyBin; } else { - $path = Installer::locateExecutableInDefaultPaths( $wgTidyBin ); + $path = ExecutableFinder::findInDefaultPaths( $wgTidyBin ); if ( $path !== false ) { $this->config['driver'] = 'RaggettExternal'; $this->config['tidyBin'] = $wgTidyBin; diff --git a/tests/phpunit/maintenance/DumpTestCase.php b/tests/phpunit/maintenance/DumpTestCase.php index 99bd427737..58cb6f3902 100644 --- a/tests/phpunit/maintenance/DumpTestCase.php +++ b/tests/phpunit/maintenance/DumpTestCase.php @@ -35,7 +35,7 @@ abstract class DumpTestCase extends MediaWikiLangTestCase { */ protected function checkHasGzip() { if ( self::$hasGzip === null ) { - self::$hasGzip = ( Installer::locateExecutableInDefaultPaths( 'gzip' ) !== false ); + self::$hasGzip = ( ExecutableFinder::findInDefaultPaths( 'gzip' ) !== false ); } if ( !self::$hasGzip ) { -- 2.20.1