(stable) Promote 2020 Week 29

src/applications/config/check/PhabricatorDaemonsSetupCheck.php

@@ -8,14 +8,21 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck {
 
   protected function executeChecks() {
 
-    $task_daemon = id(new PhabricatorDaemonLogQuery())
+    try {
+      $task_daemons = id(new PhabricatorDaemonLogQuery())
         ->setViewer(PhabricatorUser::getOmnipotentUser())
         ->withStatus(PhabricatorDaemonLogQuery::STATUS_ALIVE)
         ->withDaemonClasses(array('PhabricatorTaskmasterDaemon'))
         ->setLimit(1)
         ->execute();
 
-    if (!$task_daemon) {
+      $no_daemons = !$task_daemons;
+    } catch (Exception $ex) {
+      // Just skip this warning if the query fails for some reason.
+      $no_daemons = false;
+    }
+
+    if ($no_daemons) {
       $doc_href = PhabricatorEnv::getDoclink('Managing Daemons with phd');
 
       $summary = pht(

src/applications/config/check/PhabricatorExtraConfigSetupCheck.php

@@ -322,6 +322,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
       'directly supported. Prefixes and other strings may be customized with '.
       '"translation.override".');
 
+    $phd_reason = pht(
+      'Use "bin/phd debug ..." to get a detailed daemon execution log.');
+
     $ancient_config += array(
       'phid.external-loaders' =>
         pht(
@@ -539,6 +542,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
 
       'phd.pid-directory' => pht(
         'Phabricator daemons no longer use PID files.'),
+
+      'phd.trace' => $phd_reason,
+      'phd.verbose' => $phd_reason,
     );
 
     return $ancient_config;

src/applications/config/option/PhabricatorPHDConfigOptions.php

@@ -43,22 +43,6 @@ final class PhabricatorPHDConfigOptions
             "configuration changes are picked up by the daemons ".
             "automatically, but pool sizes can not be changed without a ".
             "restart.")),
-      $this->newOption('phd.verbose', 'bool', false)
-        ->setLocked(true)
-        ->setBoolOptions(
-          array(
-            pht('Verbose mode'),
-            pht('Normal mode'),
-          ))
-        ->setSummary(pht("Launch daemons in 'verbose' mode by default."))
-        ->setDescription(
-          pht(
-            "Launch daemons in 'verbose' mode by default. This creates a lot ".
-            "of output, but can help debug issues. Daemons launched in debug ".
-            "mode with '%s' are always launched in verbose mode. ".
-            "See also '%s'.",
-            'phd debug',
-            'phd.trace')),
       $this->newOption('phd.user', 'string', null)
         ->setLocked(true)
         ->setSummary(pht('System user to run daemons as.'))
@@ -68,22 +52,6 @@ final class PhabricatorPHDConfigOptions
             'user will own the working copies of any repositories that '.
             'Phabricator imports or manages. This option is new and '.
             'experimental.')),
-      $this->newOption('phd.trace', 'bool', false)
-        ->setLocked(true)
-        ->setBoolOptions(
-          array(
-            pht('Trace mode'),
-            pht('Normal mode'),
-          ))
-        ->setSummary(pht("Launch daemons in 'trace' mode by default."))
-        ->setDescription(
-          pht(
-            "Launch daemons in 'trace' mode by default. This creates an ".
-            "ENORMOUS amount of output, but can help debug issues. Daemons ".
-            "launched in debug mode with '%s' are always launched in ".
-            "trace mode. See also '%s'.",
-            'phd debug',
-            'phd.verbose')),
       $this->newOption('phd.garbage-collection', 'wild', array())
         ->setLocked(true)
         ->setLockedMessage(

src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php

@@ -116,11 +116,11 @@ abstract class PhabricatorDaemonManagementWorkflow
     $trace = PhutilArgumentParser::isTraceModeEnabled();
 
     $flags = array();
-    if ($trace || PhabricatorEnv::getEnvConfig('phd.trace')) {
+    if ($trace) {
       $flags[] = '--trace';
     }
 
-    if ($debug || PhabricatorEnv::getEnvConfig('phd.verbose')) {
+    if ($debug) {
       $flags[] = '--verbose';
     }
 

src/infrastructure/daemon/PhutilDaemonHandle.php

@@ -16,7 +16,6 @@ final class PhutilDaemonHandle extends Phobject {
   private $restartAt;
   private $busyEpoch;
 
-  private $pid;
   private $daemonID;
   private $deadline;
   private $heartbeat;
@@ -104,7 +103,7 @@ final class PhutilDaemonHandle extends Phobject {
   }
 
   public function isRunning() {
-    return (bool)$this->future;
+    return (bool)$this->getFuture();
   }
 
   public function isHibernating() {
@@ -134,10 +133,6 @@ final class PhutilDaemonHandle extends Phobject {
     return (!$this->shouldRestart && !$this->isRunning());
   }
 
-  public function getFuture() {
-    return $this->future;
-  }
-
   public function update() {
     if (!$this->isRunning()) {
       if (!$this->shouldRestart) {
@@ -152,11 +147,19 @@ final class PhutilDaemonHandle extends Phobject {
       $this->startDaemonProcess();
     }
 
-    $future = $this->future;
+    $future = $this->getFuture();
 
     $result = null;
-    if ($future->isReady()) {
+    $caught = null;
+    if ($future->canResolve()) {
+      $this->future = null;
+      try {
         $result = $future->resolve();
+      } catch (Exception $ex) {
+        $caught = $ex;
+      } catch (Throwable $ex) {
+        $caught = $ex;
+      }
     }
 
     list($stdout, $stderr) = $future->read();
@@ -173,7 +176,14 @@ final class PhutilDaemonHandle extends Phobject {
       }
     }
 
-    if ($result !== null) {
+    if ($result !== null || $caught !== null) {
+
+      if ($caught) {
+        $message = pht(
+          'Process failed with exception: %s',
+          $caught->getMessage());
+        $this->logMessage('FAIL', $message);
+      } else {
         list($err) = $result;
 
         if ($err) {
@@ -181,8 +191,7 @@ final class PhutilDaemonHandle extends Phobject {
         } else {
           $this->logMessage('DONE', pht('Process exited normally.'));
         }
-
-      $this->future = null;
+      }
 
       if ($this->shouldShutdown) {
         $this->restartAt = null;
@@ -244,8 +253,22 @@ final class PhutilDaemonHandle extends Phobject {
     return $this->daemonID;
   }
 
-  public function getPID() {
-    return $this->pid;
+  private function getFuture() {
+    return $this->future;
+  }
+
+  private function getPID() {
+    $future = $this->getFuture();
+
+    if (!$future) {
+      return null;
+    }
+
+    if (!$future->hasPID()) {
+      return null;
+    }
+
+    return $future->getPID();
   }
 
   private function getCaptureBufferSize() {
@@ -327,15 +350,16 @@ final class PhutilDaemonHandle extends Phobject {
 
   private function annihilateProcessGroup() {
     $pid = $this->getPID();
-
+    if ($pid) {
       $pgid = posix_getpgid($pid);
-    if ($pid && $pgid) {
+      if ($pgid) {
         posix_kill(-$pgid, SIGTERM);
         sleep($this->getKillDelay());
         posix_kill(-$pgid, SIGKILL);
         $this->pid = null;
       }
     }
+  }
 
   private function startDaemonProcess() {
     $this->logMessage('INIT', pht('Starting process.'));
@@ -345,10 +369,12 @@ final class PhutilDaemonHandle extends Phobject {
     $this->stdoutBuffer = '';
     $this->hibernating = false;
 
-    $this->future = $this->newExecFuture();
-    $this->future->start();
+    $future = $this->newExecFuture();
+    $this->future = $future;
 
-    $this->pid = $this->future->getPID();
+    $pool = $this->getDaemonPool();
+    $overseer = $pool->getOverseer();
+    $overseer->addFutureToPool($future);
   }
 
   private function didReadStdout($data) {
@@ -440,7 +466,10 @@ final class PhutilDaemonHandle extends Phobject {
     // naturally be restarted after it exits, as though it had exited after an
     // unhandled exception.
 
-    posix_kill($this->getPID(), SIGINT);
+    $pid = $this->getPID();
+    if ($pid) {
+      posix_kill($pid, SIGINT);
+    }
   }
 
   public function didReceiveGracefulSignal($signo) {
@@ -461,7 +490,10 @@ final class PhutilDaemonHandle extends Phobject {
 
     $this->logMessage('DONE', $sigmsg, $signo);
 
-    posix_kill($this->getPID(), SIGINT);
+    $pid = $this->getPID();
+    if ($pid) {
+      posix_kill($pid, SIGINT);
+    }
   }
 
   public function didReceiveTerminateSignal($signo) {

src/infrastructure/daemon/PhutilDaemonOverseer.php

@@ -181,10 +181,6 @@ EOHELP
           }
         }
 
-        foreach ($pool->getFutures() as $future) {
-          $future_pool->addFuture($future);
-        }
-
         if ($pool->getDaemons()) {
           $running_pools = true;
         }
@@ -210,6 +206,11 @@ EOHELP
     exit($this->err);
   }
 
+  public function addFutureToPool(Future $future) {
+    $this->getFuturePool()->addFuture($future);
+    return $this;
+  }
+
   private function getFuturePool() {
     if (!$this->futurePool) {
       $pool = new FuturePool();

src/infrastructure/daemon/PhutilDaemonPool.php

@@ -111,18 +111,6 @@ final class PhutilDaemonPool extends Phobject {
     return $this->daemons;
   }
 
-  public function getFutures() {
-    $futures = array();
-    foreach ($this->getDaemons() as $daemon) {
-      $future = $daemon->getFuture();
-      if ($future) {
-        $futures[] = $future;
-      }
-    }
-
-    return $futures;
-  }
-
   public function didReceiveSignal($signal, $signo) {
     switch ($signal) {
       case PhutilDaemonOverseer::SIGNAL_GRACEFUL:

src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php

@@ -24,7 +24,7 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon {
           if ($ex) {
             if ($ex instanceof PhabricatorWorkerPermanentFailureException) {
               // NOTE: Make sure these reach the daemon log, even when not
-              // running in "phd.verbose" mode. See T12803 for discussion.
+              // running in verbose mode. See T12803 for discussion.
               $log_exception = new PhutilProxyException(
                 pht(
                   'Task "%s" encountered a permanent failure and was '.

src/infrastructure/diff/prose/PhutilProseDifferenceEngine.php

@@ -142,22 +142,9 @@ final class PhutilProseDifferenceEngine extends Phobject {
       }
 
       if ($level < 2) {
-        // Split pieces into separate text and whitespace sections: make one
-        // piece out of all the whitespace at the beginning, one piece out of
-        // all the actual text in the middle, and one piece out of all the
-        // whitespace at the end.
-
-        $matches = null;
-        preg_match('/^(\s*)(.*?)(\s*)\z/s', $result, $matches);
-
-        if (strlen($matches[1])) {
-          $results[] = $matches[1];
-        }
-        if (strlen($matches[2])) {
-          $results[] = $matches[2];
-        }
-        if (strlen($matches[3])) {
-          $results[] = $matches[3];
+        $trimmed_pieces = $this->trimApart($result);
+        foreach ($trimmed_pieces as $trimmed_piece) {
+          $results[] = $trimmed_piece;
         }
       } else {
         $results[] = $result;
@@ -272,4 +259,36 @@ final class PhutilProseDifferenceEngine extends Phobject {
     return $blocks;
   }
 
+  public static function trimApart($input) {
+    // Split pieces into separate text and whitespace sections: make one
+    // piece out of all the whitespace at the beginning, one piece out of
+    // all the actual text in the middle, and one piece out of all the
+    // whitespace at the end.
+
+    $parts = array();
+
+    $length = strlen($input);
+
+    $corpus = ltrim($input);
+    $l_length = strlen($corpus);
+    if ($l_length !== $length) {
+      $parts[] = substr($input, 0, $length - $l_length);
+    }
+
+    $corpus = rtrim($corpus);
+    $lr_length = strlen($corpus);
+
+    if ($lr_length) {
+      $parts[] = $corpus;
+    }
+
+    if ($lr_length !== $l_length) {
+      // NOTE: This will be a negative value; we're slicing from the end of
+      // the input string.
+      $parts[] = substr($input, $lr_length - $l_length);
+    }
+
+    return $parts;
+  }
+
 }

src/infrastructure/diff/prose/__tests__/PhutilProseDiffTestCase.php

@@ -3,6 +3,39 @@
 final class PhutilProseDiffTestCase
   extends PhabricatorTestCase {
 
+  public function testTrimApart() {
+    $map = array(
+      '' => array(),
+      'a' => array('a'),
+      ' a ' => array(
+        ' ',
+        'a',
+        ' ',
+      ),
+      ' a' => array(
+        ' ',
+        'a',
+      ),
+      'a ' => array(
+        'a',
+        ' ',
+      ),
+      ' a b ' => array(
+        ' ',
+        'a b',
+        ' ',
+      ),
+    );
+
+    foreach ($map as $input => $expect) {
+      $actual = PhutilProseDifferenceEngine::trimApart($input);
+      $this->assertEqual(
+        $expect,
+        $actual,
+        pht('Trim Apart: %s', $input));
+    }
+  }
+
   public function testProseDiffsDistance() {
     $this->assertProseParts(
       '',