Merge "Deprecate wfShellWikiCmd()"

diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index 0ae6a83..312933b 100644 (file)
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -343,6 +343,8 @@ changes to languages because of Phabricator reports.
 * Wikimedia\Rdbms\SavepointPostgres is deprecated.
 * The DO_MAINTENANCE constant is deprecated. RUN_MAINTENANCE_IF_MAIN should be
   used instead.
+* The function wfShellWikiCmd() has been deprecated, use
+  MediaWiki\Shell::makeScriptCommand().
 
 === Other changes in 1.31 ===
 * Browser support for Internet Explorer 10 was lowered from Grade A to Grade C.

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 513f593..cd8ae46 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -2377,6 +2377,8 @@ function wfInitShellLocale() {
  * Note that $parameters should be a flat array and an option with an argument
  * should consist of two consecutive items in the array (do not use "--option value").
  *
+ * @deprecated since 1.31, use Shell::makeScriptCommand()
+ *
  * @param string $script MediaWiki cli script path
  * @param array $parameters Arguments and options to the script
  * @param array $options Associative array of options:

diff --git a/includes/SiteConfiguration.php b/includes/SiteConfiguration.php
index 2d1d961..6bd179a 100644 (file)
--- a/includes/SiteConfiguration.php
+++ b/includes/SiteConfiguration.php
@@ -20,6 +20,8 @@
  * @file
  */
 
+use MediaWiki\Shell\Shell;
+
 /**
  * This is a class for holding configuration settings, particularly for
  * multi-wiki sites.
@@ -546,19 +548,21 @@ class SiteConfiguration {
                        } else {
                                $this->cfgCache[$wiki] = [];
                        }
-                       $retVal = 1;
-                       $cmd = wfShellWikiCmd(
+                       $result = Shell::makeScriptCommand(
                                "$IP/maintenance/getConfiguration.php",
                                [
                                        '--wiki', $wiki,
                                        '--settings', implode( ' ', $settings ),
-                                       '--format', 'PHP'
+                                       '--format', 'PHP',
                                ]
-                       );
-                       // ulimit5.sh breaks this call
-                       $data = trim( wfShellExec( $cmd, $retVal, [], [ 'memory' => 0, 'filesize' => 0 ] ) );
-                       if ( $retVal != 0 || !strlen( $data ) ) {
-                               throw new MWException( "Failed to run getConfiguration.php." );
+                       )
+                               // limit.sh breaks this call
+                               ->limits( [ 'memory' => 0, 'filesize' => 0 ] )
+                               ->execute();
+
+                       $data = trim( $result->getStdout() );
+                       if ( $result->getExitCode() != 0 || !strlen( $data ) ) {
+                               throw new MWException( "Failed to run getConfiguration.php: {$result->getStdout()}" );
                        }
                        $res = unserialize( $data );
                        if ( !is_array( $res ) ) {

diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php
index d57bf4f..72107e9 100644 (file)
--- a/includes/shell/Shell.php
+++ b/includes/shell/Shell.php
@@ -22,6 +22,7 @@
 
 namespace MediaWiki\Shell;
 
+use Hooks;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -212,4 +213,32 @@ class Shell {
                }
                return $retVal;
        }
+
+       /**
+        * Generate a Command object to run a MediaWiki CLI script.
+        * Note that $parameters should be a flat array and an option with an argument
+        * should consist of two consecutive items in the array (do not use "--option value").
+        *
+        * @param string $script MediaWiki CLI script with full path
+        * @param string[] $parameters Arguments and options to the script
+        * @param array $options Associative array of options:
+        *     'php': The path to the php executable
+        *     'wrapper': Path to a PHP wrapper to handle the maintenance script
+        * @return Command
+        */
+       public static function makeScriptCommand( $script, $parameters, $options = [] ) {
+               global $wgPhpCli;
+               // Give site config file a chance to run the script in a wrapper.
+               // The caller may likely want to call wfBasename() on $script.
+               Hooks::run( 'wfShellWikiCmd', [ &$script, &$parameters, &$options ] );
+               $cmd = isset( $options['php'] ) ? [ $options['php'] ] : [ $wgPhpCli ];
+               if ( isset( $options['wrapper'] ) ) {
+                       $cmd[] = $options['wrapper'];
+               }
+               $cmd[] = $script;
+
+               return self::command( $cmd )
+                       ->params( $parameters )
+                       ->restrict( self::RESTRICT_DEFAULT & ~self::NO_LOCALSETTINGS );
+       }
 }

diff --git a/tests/phpunit/includes/shell/ShellTest.php b/tests/phpunit/includes/shell/ShellTest.php
index 8162bc0..bf46f44 100644 (file)
--- a/tests/phpunit/includes/shell/ShellTest.php
+++ b/tests/phpunit/includes/shell/ShellTest.php
@@ -1,12 +1,14 @@
 <?php
 
+use MediaWiki\Shell\Command;
 use MediaWiki\Shell\Shell;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * @covers \MediaWiki\Shell\Shell
  * @group Shell
  */
-class ShellTest extends PHPUnit\Framework\TestCase {
+class ShellTest extends MediaWikiTestCase {
 
        use MediaWikiCoversValidator;
 
@@ -32,4 +34,72 @@ class ShellTest extends PHPUnit\Framework\TestCase {
                        'skip nulls' => [ [ 'ls', null ], "'ls'" ],
                ];
        }
+
+       /**
+        * @covers \MediaWiki\Shell\Shell::makeScriptCommand
+        * @dataProvider provideMakeScriptCommand
+        *
+        * @param string $expected
+        * @param string $script
+        * @param string[] $parameters
+        * @param string[] $options
+        * @param callable|null $hook
+        */
+       public function testMakeScriptCommand( $expected,
+                                                                                  $script,
+                                                                                  $parameters,
+                                                                                  $options = [],
+                                                                                  $hook = null
+       ) {
+               // Running tests under Vagrant involves MWMultiVersion that uses the below hook
+               $this->setMwGlobals( 'wgHooks', [] );
+
+               if ( $hook ) {
+                       $this->setTemporaryHook( 'wfShellWikiCmd', $hook );
+               }
+
+               $command = Shell::makeScriptCommand( $script, $parameters, $options );
+               $command->params( 'safe' )
+                       ->unsafeParams( 'unsafe' );
+
+               $this->assertType( Command::class, $command );
+
+               $wrapper = TestingAccessWrapper::newFromObject( $command );
+               $this->assertEquals( $expected, $wrapper->command );
+               $this->assertEquals( 0, $wrapper->restrictions & Shell::NO_LOCALSETTINGS );
+       }
+
+       public function provideMakeScriptCommand() {
+               global $wgPhpCli;
+
+               return [
+                       [
+                               "'$wgPhpCli' 'maintenance/foobar.php' 'bar'\\''\"baz' 'safe' unsafe",
+                               'maintenance/foobar.php',
+                               [ 'bar\'"baz' ],
+                       ],
+                       [
+                               "'$wgPhpCli' 'changed.php' '--wiki=somewiki' 'bar'\\''\"baz' 'safe' unsafe",
+                               'maintenance/foobar.php',
+                               [ 'bar\'"baz' ],
+                               [],
+                               function ( &$script, array &$parameters ) {
+                                       $script = 'changed.php';
+                                       array_unshift( $parameters, '--wiki=somewiki' );
+                               }
+                       ],
+                       [
+                               "'/bin/perl' 'maintenance/foobar.php' 'bar'\\''\"baz' 'safe' unsafe",
+                               'maintenance/foobar.php',
+                               [ 'bar\'"baz' ],
+                               [ 'php' => '/bin/perl' ],
+                       ],
+                       [
+                               "'$wgPhpCli' 'foobinize' 'maintenance/foobar.php' 'bar'\\''\"baz' 'safe' unsafe",
+                               'maintenance/foobar.php',
+                               [ 'bar\'"baz' ],
+                               [ 'wrapper' => 'foobinize' ],
+                       ],
+               ];
+       }
 }