Raise a setup warning for an unparseable VCS binary version

Summary:
Hit this locally, with an error like:

> Version <empty string> is older than 1.9, the minimum supported version.

(Where `<empty string>` was just the empty string, not literally the text `<empty string>`.)

Be more careful about parsing versions, and parse the newer string.

Test Plan: Got "unknown version" with intentionally-broken test data, then clean readout.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11558

src/applications/config/check/PhabricatorBinariesSetupCheck.php

@@ -93,6 +93,7 @@ final class PhabricatorBinariesSetupCheck extends PhabricatorSetupCheck {
         $this->raiseWarning($binary, $message);
       }
 
+      $version = null;
       switch ($binary) {
         case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
           $minimum_version = null;
@@ -118,25 +119,37 @@ final class PhabricatorBinariesSetupCheck extends PhabricatorSetupCheck {
                          'leak, fixed in 2.2.1. Pushing fails with this '.
                          'version as well; see T3046#54922.'),);
           list($err, $stdout, $stderr) = exec_manual('hg --version --quiet');
-          $version = rtrim(
-            substr($stdout, strlen('Mercurial Distributed SCM (version ')),
-            ")\n");
+
+          // NOTE: At least on OSX, recent versions of Mercurial report this
+          // string in this format:
+          //
+          //   Mercurial Distributed SCM (version 3.1.1+20140916)
+
+          $matches = null;
+          $pattern = '/^Mercurial Distributed SCM \(version ([\d.]+)/m';
+          if (preg_match($pattern, $stdout, $matches)) {
+            $version = $matches[1];
+          }
           break;
       }
 
-      if ($minimum_version &&
-        version_compare($version, $minimum_version, '<')) {
-        $this->raiseMinimumVersionWarning(
-          $binary,
-          $minimum_version,
-          $version);
-      }
-
-      foreach ($bad_versions as $bad_version => $details) {
-        if ($bad_version === $version) {
-          $this->raiseBadVersionWarning(
+      if ($version === null) {
+        $this->raiseUnknownVersionWarning($binary);
+      } else {
+        if ($minimum_version &&
+          version_compare($version, $minimum_version, '<')) {
+          $this->raiseMinimumVersionWarning(
             $binary,
-            $bad_version);
+            $minimum_version,
+            $version);
+        }
+
+        foreach ($bad_versions as $bad_version => $details) {
+          if ($bad_version === $version) {
+            $this->raiseBadVersionWarning(
+              $binary,
+              $bad_version);
+          }
         }
       }
     }
@@ -173,6 +186,43 @@ final class PhabricatorBinariesSetupCheck extends PhabricatorSetupCheck {
       ->addPhabricatorConfig('environment.append-paths');
   }
 
+  private function raiseUnknownVersionWarning($binary) {
+    $summary = pht(
+      'Unable to determine the version number of "%s".',
+      $binary);
+
+    $message = pht(
+      'Unable to determine the version number of "%s". Usually, this means '.
+      'the program changed its version format string recently and Phabricator '.
+      'does not know how to parse the new one yet, but might indicate that '.
+      'you have a very old (or broken) binary.'.
+      "\n\n".
+      'Because we can not determine the version number, checks against '.
+      'minimum and known-bad versions will be skipped, so we might fail '.
+      'to detect an incompatible binary.'.
+      "\n\n".
+      'You may be able to resolve this issue by updating Phabricator, since '.
+      'a newer version of Phabricator is likely to be able to parse the '.
+      'newer version string.'.
+      "\n\n".
+      'If updating Phabricator does not fix this, you can report the issue '.
+      'to the upstream so we can adjust the parser.'.
+      "\n\n".
+      'If you are confident you have a recent version of "%s" installed and '.
+      'working correctly, it is usually safe to ignore this warning.',
+      $binary,
+      $binary);
+
+    $this->newIssue('bin.'.$binary.'.unknown-version')
+      ->setShortName(pht("Unknown '%s' Version", $binary))
+      ->setName(pht("Unknown '%s' Version", $binary))
+      ->setSummary($summary)
+      ->setMessage($message)
+      ->addLink(
+        PhabricatorEnv::getDoclink('Contributing Bug Reports'),
+        pht('Report this Issue to the Upstream'));
+  }
+
   private function raiseMinimumVersionWarning(
     $binary,
     $minimum_version,
@@ -200,8 +250,6 @@ final class PhabricatorBinariesSetupCheck extends PhabricatorSetupCheck {
           ->setMessage($summary.' '.$message);
         break;
       }
-
-
   }
 
   private function raiseBadVersionWarning($binary, $bad_version) {

src/applications/config/issue/PhabricatorSetupIssue.php

@@ -17,6 +17,7 @@ final class PhabricatorSetupIssue {
   private $commands = array();
   private $mysqlConfig = array();
   private $originalPHPConfigValues = array();
+  private $links;
 
   public function addCommand($command) {
     $this->commands[] = $command;
@@ -162,4 +163,17 @@ final class PhabricatorSetupIssue {
   public function getIsIgnored() {
     return $this->isIgnored;
   }
+
+  public function addLink($href, $name) {
+    $this->links[] = array(
+      'href' => $href,
+      'name' => $name,
+    );
+    return $this;
+  }
+
+  public function getLinks() {
+    return $this->links;
+  }
+
 }

src/applications/config/view/PhabricatorSetupIssueView.php

@@ -107,6 +107,11 @@ final class PhabricatorSetupIssueView extends AphrontView {
 
     }
 
+    $related_links = $issue->getLinks();
+    if ($related_links) {
+      $description[] = $this->renderRelatedLinks($related_links);
+    }
+
     $actions = array();
     if (!$issue->getIsFatal()) {
       if ($issue->getIsIgnored()) {
@@ -508,4 +513,37 @@ final class PhabricatorSetupIssueView extends AphrontView {
     }
   }
 
+  private function renderRelatedLinks(array $links) {
+    $link_info = phutil_tag(
+      'p',
+      array(),
+      pht(
+        '%d related link(s):',
+        count($links)));
+
+    $link_list = array();
+    foreach ($links as $link) {
+      $link_tag = phutil_tag(
+        'a',
+        array(
+          'target' => '_blank',
+          'href' => $link['href'],
+        ),
+        $link['name']);
+      $link_item = phutil_tag('li', array(), $link_tag);
+      $link_list[] = $link_item;
+    }
+    $link_list = phutil_tag('ul', array(), $link_list);
+
+    return phutil_tag(
+      'div',
+      array(
+        'class' => 'setup-issue-config',
+      ),
+      array(
+        $link_info,
+        $link_list,
+      ));
+  }
+
 }

src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php

@@ -885,6 +885,11 @@ abstract class PhabricatorBaseEnglishTranslation
 
       '%s edited %s edge(s) for %s, added %s: %s; removed %s: %s.' =>
         '%s edited edges for %3$s, added: %5$s; removed %7$s.',
+
+      '%d related link(s):' => array(
+        'Related link:',
+        'Related links:',
+      ),
     );
   }