Refactor some Installer code into ExecutableFinder
authorKunal Mehta <legoktm@member.fsf.org>
Fri, 20 Oct 2017 07:36:03 +0000 (00:36 -0700)
committerKunal Mehta <legoktm@member.fsf.org>
Thu, 26 Oct 2017 18:42:05 +0000 (11:42 -0700)
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
autoload.php
includes/installer/Installer.php
includes/utils/ExecutableFinder.php [new file with mode: 0644]
maintenance/Maintenance.php
tests/parser/TidySupport.php
tests/phpunit/maintenance/DumpTestCase.php

index 60dd495..a4ce481 100644 (file)
@@ -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
index cf4a115..3bee411 100644 (file)
@@ -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',
index 012b477..e99ea7c 100644 (file)
@@ -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 (file)
index 0000000..93f635d
--- /dev/null
@@ -0,0 +1,110 @@
+<?php
+/**
+ * Copyright (C) 2017 Kunal Mehta <legoktm@member.fsf.org>
+ *
+ * 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;
+       }
+
+}
index 7de0ae4..174b973 100644 (file)
@@ -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 );
index 6aed02f..c0a9312 100644 (file)
@@ -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;
index 99bd427..58cb6f3 100644 (file)
@@ -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 ) {