Shell\Command: Move code that builds final shell command into separate method

[lhc/web/wiklou.git] / includes / shell / Command.php

diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 59f40bf..1816c5a 100644 (file)
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -63,7 +63,7 @@ class Command {
        private $everExecuted = false;
 
        /** @var string|false */
-       private $cGroup = false;
+       private $cgroup = false;
 
        /**
         * Constructor. Don't call directly, instead use Shell::command()
@@ -83,12 +83,14 @@ class Command {
         */
        public function __destruct() {
                if ( !$this->everExecuted ) {
+                       $context = [ 'command' => $this->command ];
                        $message = __CLASS__ . " was instantiated, but execute() was never called.";
                        if ( $this->method ) {
-                               $message .= " Calling method: {$this->method}.";
+                               $message .= ' Calling method: {method}.';
+                               $context['method'] = $this->method;
                        }
-                       $message .= " Command: {$this->command}";
-                       trigger_error( $message, E_USER_NOTICE );
+                       $message .= ' Command: {command}';
+                       $this->logger->warning( $message, $context );
                }
        }
 
@@ -131,7 +133,8 @@ class Command {
        /**
         * Sets execution limits
         *
-        * @param array $limits Optional array with limits(filesize, memory, time, walltime).
+        * @param array $limits Associative array of limits. Keys (all optional):
+        *   filesize (for ulimit -f), memory, time, walltime.
         * @return $this
         */
        public function limits( array $limits ) {
@@ -185,29 +188,22 @@ class Command {
        /**
         * Sets cgroup for this command
         *
-        * @param string|false $cgroup
+        * @param string|false $cgroup Absolute file path to the cgroup, or false to not use a cgroup
         * @return $this
         */
        public function cgroup( $cgroup ) {
-               $this->cGroup = $cgroup;
+               $this->cgroup = $cgroup;
 
                return $this;
        }
 
        /**
-        * Executes command. Afterwards, getExitCode() and getOutput() can be used to access execution
-        * results.
+        * String together all the options and build the final command
+        * to execute
         *
-        * @return Result
-        * @throws Exception
-        * @throws ProcOpenError
-        * @throws ShellDisabledError
+        * @return array [ command, whether to use log pipe ]
         */
-       public function execute() {
-               $this->everExecuted = true;
-
-               $profileMethod = $this->method ?: wfGetCaller();
-
+       protected function buildFinalCommand() {
                $envcmd = '';
                foreach ( $this->env as $k => $v ) {
                        if ( wfIsWindows() ) {
@@ -226,9 +222,9 @@ class Command {
                        }
                }
 
+               $useLogPipe = false;
                $cmd = $envcmd . trim( $this->command );
 
-               $useLogPipe = false;
                if ( is_executable( '/bin/bash' ) ) {
                        $time = intval( $this->limits['time'] );
                        $wallTime = intval( $this->limits['walltime'] );
@@ -237,22 +233,42 @@ class Command {
 
                        if ( $time > 0 || $mem > 0 || $filesize > 0 || $wallTime > 0 ) {
                                $cmd = '/bin/bash ' . escapeshellarg( __DIR__ . '/limit.sh' ) . ' ' .
-                                          escapeshellarg( $cmd ) . ' ' .
-                                          escapeshellarg(
-                                                  "MW_INCLUDE_STDERR=" . ( $this->useStderr ? '1' : '' ) . ';' .
-                                                  "MW_CPU_LIMIT=$time; " .
-                                                  'MW_CGROUP=' . escapeshellarg( $this->cGroup ) . '; ' .
-                                                  "MW_MEM_LIMIT=$mem; " .
-                                                  "MW_FILE_SIZE_LIMIT=$filesize; " .
-                                                  "MW_WALL_CLOCK_LIMIT=$wallTime; " .
-                                                  "MW_USE_LOG_PIPE=yes"
-                                          );
+                                       escapeshellarg( $cmd ) . ' ' .
+                                       escapeshellarg(
+                                               "MW_INCLUDE_STDERR=" . ( $this->useStderr ? '1' : '' ) . ';' .
+                                               "MW_CPU_LIMIT=$time; " .
+                                               'MW_CGROUP=' . escapeshellarg( $this->cgroup ) . '; ' .
+                                               "MW_MEM_LIMIT=$mem; " .
+                                               "MW_FILE_SIZE_LIMIT=$filesize; " .
+                                               "MW_WALL_CLOCK_LIMIT=$wallTime; " .
+                                               "MW_USE_LOG_PIPE=yes"
+                                       );
                                $useLogPipe = true;
                        }
                }
                if ( !$useLogPipe && $this->useStderr ) {
                        $cmd .= ' 2>&1';
                }
+
+               return [ $cmd, $useLogPipe ];
+       }
+
+       /**
+        * Executes command. Afterwards, getExitCode() and getOutput() can be used to access execution
+        * results.
+        *
+        * @return Result
+        * @throws Exception
+        * @throws ProcOpenError
+        * @throws ShellDisabledError
+        */
+       public function execute() {
+               $this->everExecuted = true;
+
+               $profileMethod = $this->method ?: wfGetCaller();
+
+               list( $cmd, $useLogPipe ) = $this->buildFinalCommand();
+
                $this->logger->debug( __METHOD__ . ": $cmd" );
 
                // Don't try to execute commands that exceed Linux's MAX_ARG_STRLEN.