(stable) Promote 2017 Week 51

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/repository/api/ArcanistGitAPI.php

@@ -52,8 +52,12 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
   }
 
   public function getGitVersion() {
+    static $version = null;
+    if ($version === null) {
       list($stdout) = $this->execxLocal('--version');
-    return rtrim(str_replace('git version ', '', $stdout));
+      $version = rtrim(str_replace('git version ', '', $stdout));
+    }
+    return $version;
   }
 
   public function getMetadataPath() {
@@ -645,8 +649,70 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     return $this->executeSVNFindRev($hash, 'SVN');
   }
 
+  private function buildUncommittedStatusViaStatus() {
+    $status = $this->buildLocalFuture(
+      array(
+        'status --porcelain=2 -z',
+      ));
+    list($stdout) = $status->resolvex();
+
+    $result = new PhutilArrayWithDefaultValue();
+    $parts = explode("\0", $stdout);
+    while (count($parts) > 1) {
+      $entry = array_shift($parts);
+      $entry_parts = explode(' ', $entry);
+      if ($entry_parts[0] == '1') {
+        $path = $entry_parts[8];
+      } else if ($entry_parts[0] == '2') {
+        $path = $entry_parts[9];
+      } else if ($entry_parts[0] == 'u') {
+        $path = $entry_parts[10];
+      } else if ($entry_parts[0] == '?') {
+        $result[$entry_parts[1]] = self::FLAG_UNTRACKED;
+        continue;
+      }
+
+      $result[$path] |= self::FLAG_UNCOMMITTED;
+      $index_state = substr($entry_parts[1], 0, 1);
+      $working_state = substr($entry_parts[1], 1, 1);
+      if ($index_state == 'A') {
+        $result[$path] |= self::FLAG_ADDED;
+      } else if ($index_state == 'M') {
+        $result[$path] |= self::FLAG_MODIFIED;
+      } else if ($index_state == 'D') {
+        $result[$path] |= self::FLAG_DELETED;
+      }
+      if ($working_state != '.') {
+        $result[$path] |= self::FLAG_UNSTAGED;
+        if ($index_state == '.') {
+          if ($working_state == 'A') {
+            $result[$path] |= self::FLAG_ADDED;
+          } else if ($working_state == 'M') {
+            $result[$path] |= self::FLAG_MODIFIED;
+          } else if ($working_state == 'D') {
+            $result[$path] |= self::FLAG_DELETED;
+          }
+        }
+      }
+      $submodule_tracked = substr($entry_parts[2], 2, 1);
+      $submodule_untracked = substr($entry_parts[2], 3, 1);
+      if ($submodule_tracked == 'M' || $submodule_untracked == 'U') {
+        $result[$path] |= self::FLAG_EXTERNALS;
+      }
+
+      if ($entry_parts[0] == '2') {
+        $result[array_shift($parts)] = $result[$path] | self::FLAG_DELETED;
+        $result[$path] |= self::FLAG_ADDED;
+      }
+    }
+    return $result->toArray();
+  }
 
   protected function buildUncommittedStatus() {
+    if (version_compare($this->getGitVersion(), '2.11.0', '>=')) {
+      return $this->buildUncommittedStatusViaStatus();
+    }
+
     $diff_options = $this->getDiffBaseOptions();
 
     if ($this->repositoryHasNoCommits) {
@@ -719,7 +785,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
   protected function buildCommitRangeStatus() {
     list($stdout, $stderr) = $this->execxLocal(
-      'diff %C --raw %s --',
+      'diff %C --raw %s HEAD --',
       $this->getDiffBaseOptions(),
       $this->getBaseCommit());
 

src/repository/api/ArcanistMercurialAPI.php

@@ -372,41 +372,18 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   protected function buildCommitRangeStatus() {
-    // TODO: Possibly we should use "hg status --rev X --rev ." for this
-    // instead, but we must run "hg diff" later anyway in most cases, so
-    // building and caching it shouldn't hurt us.
+    list($stdout) = $this->execxLocal(
+      'status --rev %s --rev tip',
+      $this->getBaseCommit());
 
-    $diff = $this->getFullMercurialDiff();
-    if (!$diff) {
-      return array();
+    $results = new PhutilArrayWithDefaultValue();
+
+    $working_status = ArcanistMercurialParser::parseMercurialStatus($stdout);
+    foreach ($working_status as $path => $mask) {
+      $results[$path] |= $mask;
     }
 
-    $parser = new ArcanistDiffParser();
-    $changes = $parser->parseDiff($diff);
-
-    $status_map = array();
-    foreach ($changes as $change) {
-      $flags = 0;
-      switch ($change->getType()) {
-        case ArcanistDiffChangeType::TYPE_ADD:
-        case ArcanistDiffChangeType::TYPE_MOVE_HERE:
-        case ArcanistDiffChangeType::TYPE_COPY_HERE:
-          $flags |= self::FLAG_ADDED;
-          break;
-        case ArcanistDiffChangeType::TYPE_CHANGE:
-        case ArcanistDiffChangeType::TYPE_COPY_AWAY: // Check for changes?
-          $flags |= self::FLAG_MODIFIED;
-          break;
-        case ArcanistDiffChangeType::TYPE_DELETE:
-        case ArcanistDiffChangeType::TYPE_MOVE_AWAY:
-        case ArcanistDiffChangeType::TYPE_MULTICOPY:
-          $flags |= self::FLAG_DELETED;
-          break;
-      }
-      $status_map[$change->getCurrentPath()] = $flags;
-    }
-
-    return $status_map;
+    return $results->toArray();
   }
 
   protected function didReloadWorkingCopy() {

src/repository/api/ArcanistSubversionAPI.php

@@ -45,9 +45,9 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
   }
 
   protected function buildCommitRangeStatus() {
-    // In SVN, the commit range is always "uncommitted changes", so these
-    // statuses are equivalent.
-    return $this->getUncommittedStatus();
+    // In SVN, there are never any previous commits in the range -- it is all in
+    // the uncommitted status.
+    return array();
   }
 
   protected function buildUncommittedStatus() {

src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php

@@ -5,6 +5,8 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
   public function testGitStateParsing() {
     if (Filesystem::binaryExists('git')) {
       $this->parseState('git_basic.git.tgz');
+      $this->parseState('git_submodules_dirty.git.tgz');
+      $this->parseState('git_submodules_staged.git.tgz');
     } else {
       $this->assertSkipped(pht('Git is not installed'));
     }
@@ -54,6 +56,16 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
   }
 
   private function assertCorrectState($test, ArcanistRepositoryAPI $api) {
+    if ($api instanceof ArcanistGitAPI) {
+      $version = $api->getGitVersion();
+      if (version_compare($version, '2.11.0', '<')) {
+        // Behavior differs slightly on older versions of git; rather than code
+        // both variants, skip the tests in the presence of such a git.
+        $this->assertSkipped(pht('Behavior differs slightly on git < 2.11.0'));
+        return;
+      }
+    }
+
     $f_mod = ArcanistRepositoryAPI::FLAG_MODIFIED;
     $f_add = ArcanistRepositoryAPI::FLAG_ADDED;
     $f_del = ArcanistRepositoryAPI::FLAG_DELETED;
@@ -68,7 +80,7 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
 
     switch ($test) {
       case 'svn_basic.svn.tgz':
-        $expect = array(
+        $expect_uncommitted = array(
           'ADDED'       => $f_add,
           'COPIED_TO'   => $f_add,
           'DELETED'     => $f_del,
@@ -78,8 +90,22 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
           'PROPCHANGE'  => $f_mod,
           'UNTRACKED'   => $f_unt,
         );
-        $this->assertEqual($expect, $api->getUncommittedStatus());
-        $this->assertEqual($expect, $api->getCommitRangeStatus());
+        $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus());
+
+        $expect_range = array();
+        $this->assertEqual($expect_range, $api->getCommitRangeStatus());
+
+        $expect_working = array(
+          'ADDED'       => $f_add,
+          'COPIED_TO'   => $f_add,
+          'DELETED'     => $f_del,
+          'MODIFIED'    => $f_mod,
+          'MOVED'       => $f_del,
+          'MOVED_TO'    => $f_add,
+          'PROPCHANGE'  => $f_mod,
+          'UNTRACKED'   => $f_unt,
+        );
+        $this->assertEqual($expect_working, $api->getWorkingCopyStatus());
         break;
       case 'git_basic.git.tgz':
         $expect_uncommitted = array(
@@ -94,10 +120,41 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
           'ADDED'       => $f_add,
           'DELETED'     => $f_del,
           'MODIFIED'    => $f_mod,
-          'UNCOMMITTED' => $f_add,
           'UNSTAGED'    => $f_add,
         );
         $this->assertEqual($expect_range, $api->getCommitRangeStatus());
+
+        $expect_working = array(
+          'ADDED'       => $f_add,
+          'DELETED'     => $f_del,
+          'MODIFIED'    => $f_mod,
+          'UNCOMMITTED' => $f_add | $f_unc,
+          'UNSTAGED'    => $f_add | $f_mod | $f_uns | $f_unc,
+          'UNTRACKED'   => $f_unt,
+        );
+        $this->assertEqual($expect_working, $api->getWorkingCopyStatus());
+        break;
+      case 'git_submodules_dirty.git.tgz':
+        $expect_uncommitted = array(
+          '.gitmodules'           => $f_mod | $f_uns | $f_unc,
+          'added/'                => $f_unt,
+          'deleted'               => $f_del | $f_uns | $f_unc,
+          'modified-commit'       => $f_mod | $f_uns | $f_unc,
+          'modified-commit-dirty' => $f_ext | $f_mod | $f_uns | $f_unc,
+          'modified-dirty'        => $f_ext | $f_mod | $f_uns | $f_unc,
+        );
+        $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus());
+        break;
+      case 'git_submodules_staged.git.tgz':
+        $expect_uncommitted = array(
+          '.gitmodules'           => $f_mod | $f_unc,
+          'added'                 => $f_add | $f_unc,
+          'deleted'               => $f_del | $f_unc,
+          'modified-commit'       => $f_mod | $f_unc,
+          'modified-commit-dirty' => $f_ext | $f_mod | $f_uns | $f_unc,
+          'modified-dirty'        => $f_ext | $f_mod | $f_uns | $f_unc,
+        );
+        $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus());
         break;
       case 'hg_basic.hg.tgz':
         $expect_uncommitted = array(
@@ -113,6 +170,15 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
           'UNCOMMITTED' => $f_add,
         );
         $this->assertEqual($expect_range, $api->getCommitRangeStatus());
+
+        $expect_working = array(
+          'ADDED'       => $f_add,
+          'DELETED'     => $f_del,
+          'MODIFIED'    => $f_mod,
+          'UNCOMMITTED' => $f_add | $f_mod | $f_unc,
+          'UNTRACKED'   => $f_unt,
+        );
+        $this->assertEqual($expect_working, $api->getWorkingCopyStatus());
         break;
       default:
         throw new Exception(

src/workflow/ArcanistLandWorkflow.php

@@ -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;
+  }
+
+
 }