Make "arc land" accommodate a minor API change in Harbormaster build statuses

Summary:
See PHI261. Currently "arc land" shows every build staus (passed, failed, building, etc) as yellow. Intended behavior is that passed builds are green, failed builds are red, and so on.

This is because of an unintended API change a while ago in D16356. Since the only impact was a cosmetic color issue, this escaped notice until now.

Additionally, try to use the modern `harbormaster.build.search` if it is available.

Test Plan:
  - Ran `arc land` with running builds, got reasonable coloration.
  - Faked the new method not being available, still got sensible behavior from the old method.

Reviewers: amckinley

Reviewed By: amckinley

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

src/__phutil_library_map__.php

@@ -42,6 +42,7 @@ phutil_register_library_map(array(
     'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php',
     'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php',
     'ArcanistBrowseWorkflow' => 'workflow/ArcanistBrowseWorkflow.php',
+    'ArcanistBuildRef' => 'ref/ArcanistBuildRef.php',
     'ArcanistBundle' => 'parser/ArcanistBundle.php',
     'ArcanistBundleTestCase' => 'parser/__tests__/ArcanistBundleTestCase.php',
     'ArcanistCSSLintLinter' => 'lint/linter/ArcanistCSSLintLinter.php',
@@ -459,6 +460,7 @@ phutil_register_library_map(array(
     'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow',
     'ArcanistBrowseWorkflow' => 'ArcanistWorkflow',
+    'ArcanistBuildRef' => 'Phobject',
     'ArcanistBundle' => 'Phobject',
     'ArcanistBundleTestCase' => 'PhutilTestCase',
     'ArcanistCSSLintLinter' => 'ArcanistExternalLinter',

src/ref/ArcanistBuildRef.php

@@ -0,0 +1,115 @@
+<?php
+
+final class ArcanistBuildRef
+  extends Phobject {
+
+  private $parameters;
+
+  public static function newFromConduit(array $data) {
+    $ref = new self();
+    $ref->parameters = $data;
+    return $ref;
+  }
+
+  private function getStatusMap() {
+    // The modern "harbormaster.build.search" API method returns this in the
+    // "fields" list; the older API method returns it at the root level.
+    if (isset($this->parameters['fields']['buildStatus'])) {
+      $status = $this->parameters['fields']['buildStatus'];
+    } else if (isset($this->parameters['buildStatus'])) {
+      $status = $this->parameters['buildStatus'];
+    } else {
+      $status = 'unknown';
+    }
+
+    // We may either have an array or a scalar here. The array comes from
+    // "harbormaster.build.search", or from "harbormaster.querybuilds" if
+    // the server is newer than August 2016. The scalar comes from older
+    // versions of that method. See PHI261.
+    if (is_array($status)) {
+      $map = $status;
+    } else {
+      $map = array(
+        'value' => $status,
+      );
+    }
+
+    // If we don't have a name, try to fill one in.
+    if (!isset($map['name'])) {
+      $name_map = array(
+        'inactive' => pht('Inactive'),
+        'pending' => pht('Pending'),
+        'building' => pht('Building'),
+        'passed' => pht('Passed'),
+        'failed' => pht('Failed'),
+        'aborted' => pht('Aborted'),
+        'error' => pht('Error'),
+        'paused' => pht('Paused'),
+        'deadlocked' => pht('Deadlocked'),
+        'unknown' => pht('Unknown'),
+      );
+
+      $map['name'] = idx($name_map, $map['value'], $map['value']);
+    }
+
+    // If we don't have an ANSI color code, try to fill one in.
+    if (!isset($map['color.ansi'])) {
+      $color_map = array(
+        'failed' => 'red',
+        'passed' => 'green',
+      );
+
+      $map['color.ansi'] = idx($color_map, $map['value'], 'yellow');
+    }
+
+    return $map;
+  }
+
+  public function getID() {
+    return $this->parameters['id'];
+  }
+
+  public function getName() {
+    if (isset($this->parameters['fields']['name'])) {
+      return $this->parameters['fields']['name'];
+    }
+
+    return $this->parameters['name'];
+  }
+
+  public function getStatus() {
+    $map = $this->getStatusMap();
+    return $map['value'];
+  }
+
+  public function getStatusName() {
+    $map = $this->getStatusMap();
+    return $map['name'];
+  }
+
+  public function getStatusANSIColor() {
+    $map = $this->getStatusMap();
+    return $map['color.ansi'];
+  }
+
+  public function getObjectName() {
+    return pht('Build %d', $this->getID());
+  }
+
+  public function getStatusSortVector() {
+    $status = $this->getStatus();
+
+    // For now, just sort passed builds first.
+    if ($this->getStatus() == 'passed') {
+      $status_class = 1;
+    } else {
+      $status_class = 2;
+    }
+
+    return id(new PhutilSortVector())
+      ->addInt($status_class)
+      ->addString($status);
+  }
+
+
+}

src/workflow/ArcanistLandWorkflow.php

@@ -1391,7 +1391,7 @@ EOTEXT
 
     $console = PhutilConsole::getConsole();
 
-    $buildable = head($buildables['data']);
+    $buildable = head($buildables);
 
     if ($buildable['buildableStatus'] == 'passed') {
       $console->writeOut(
@@ -1404,13 +1404,12 @@ EOTEXT
     switch ($buildable['buildableStatus']) {
       case 'building':
         $message = pht(
-          'Harbormaster is still building the active diff for this revision:');
+          'Harbormaster is still building the active diff for this revision.');
         $prompt = pht('Land revision anyway, despite ongoing build?');
         break;
       case 'failed':
         $message = pht(
-          'Harbormaster failed to build the active diff for this revision. '.
-          'Build failures:');
+          'Harbormaster failed to build the active diff for this revision.');
         $prompt = pht('Land revision anyway, despite build failures?');
         break;
       default:
@@ -1418,28 +1417,25 @@ EOTEXT
         return;
     }
 
-    $builds = $this->getConduit()->callMethodSynchronous(
-      'harbormaster.querybuilds',
+    $builds = $this->queryBuilds(
       array(
         'buildablePHIDs' => array($buildable['phid']),
       ));
 
     $console->writeOut($message."\n\n");
-    foreach ($builds['data'] as $build) {
-      switch ($build['buildStatus']) {
-        case 'failed':
-          $color = 'red';
-          break;
-        default:
-          $color = 'yellow';
-          break;
-      }
 
-      $console->writeOut(
-        "    **<bg:".$color."> %s </bg>** %s: %s\n",
-        phutil_utf8_strtoupper($build['buildStatusName']),
-        pht('Build %d', $build['id']),
-        $build['name']);
+    $builds = msort($builds, 'getStatusSortVector');
+    foreach ($builds as $build) {
+      $ansi_color = $build->getStatusANSIColor();
+      $status_name = $build->getStatusName();
+      $object_name = $build->getObjectName();
+      $build_name = $build->getName();
+
+      echo tsprintf(
+        "    **<bg:".$ansi_color."> %s </bg>** %s: %s\n",
+        $status_name,
+        $object_name,
+        $build_name);
     }
 
     $console->writeOut(
@@ -1478,4 +1474,34 @@ EOTEXT
     $mark_workflow->run();
   }
 
+  private function queryBuilds(array $constraints) {
+    $conduit = $this->getConduit();
+
+    // NOTE: This method only loads the 100 most recent builds. It's rare for
+    // a revision to have more builds than that and there's currently no paging
+    // wrapper for "*.search" Conduit API calls available in Arcanist.
+
+    try {
+      $raw_result = $conduit->callMethodSynchronous(
+        'harbormaster.build.search',
+        array(
+          'constraints' => $constraints,
+        ));
+    } catch (Exception $ex) {
+      // If the server doesn't have "harbormaster.build.search" yet (Aug 2016),
+      // try the older "harbormaster.querybuilds" instead.
+      $raw_result = $conduit->callMethodSynchronous(
+        'harbormaster.querybuilds',
+        $constraints);
+    }
+
+    $refs = array();
+    foreach ($raw_result['data'] as $raw_data) {
+      $refs[] = ArcanistBuildRef::newFromConduit($raw_data);
+    }
+
+    return $refs;
+  }
+
+
 }