Simplify "arc branch" and make it work in immutable history repositories

Summary:
  - Move "arc branch"-specific code to the branch workflow.
  - Instead of doing "git rev-parse", just do "git branch --verbose --abbrev=40".
  - Use revision owners to identify ownership, not working copy identity. Particularly with the advent of "Commandeer", you might not own commits you made.
  - Do a batch lookup for commits by hash (depends on D2859, but doesn't break without it).
  - Use PhutilConsole for console stuff.
  - Removed color from "arc list" for the moment.
  - The "--by-status" flag has a slightly different output format now.

Test Plan: Ran "arc branch" in various circumstances, verified it identifies branches in immutable history repositories.

Reviewers: btrahan, vrana, jungejason, nh, slawekbiel

Reviewed By: slawekbiel

CC: aran

Maniphest Tasks: T693

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

src/__phutil_library_map__.php

@@ -127,7 +127,6 @@ phutil_register_library_map(array(
     'ArcanistXHPASTLintNamingHookTestCase' => 'lint/linter/xhpast/__tests__/ArcanistXHPASTLintNamingHookTestCase.php',
     'ArcanistXHPASTLinter' => 'lint/linter/ArcanistXHPASTLinter.php',
     'ArcanistXHPASTLinterTestCase' => 'lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php',
-    'BranchInfo' => 'branch/BranchInfo.php',
     'ComprehensiveLintEngine' => 'lint/engine/ComprehensiveLintEngine.php',
     'ExampleLintEngine' => 'lint/engine/ExampleLintEngine.php',
     'NoseTestEngine' => 'unit/engine/NoseTestEngine.php',

src/branch/BranchInfo.php

@@ -1,180 +0,0 @@
-<?php
-
-/*
- * Copyright 2012 Facebook, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/**
- * Holds information about a single git branch, and provides methods
- * for loading and display.
- */
-final class BranchInfo {
-
-  private $branchName;
-  private $currentHead = false;
-  private $revisionID = null;
-  private $sha1;
-  private $status;
-  private $commitAuthor;
-  private $commitTime;
-  private $commitSubject;
-
-  /**
-   * Retrives all the branches from the current git repository,
-   * and parses their commit messages.
-   *
-   * @return array a list of BranchInfo objects, one per branch.
-   */
-  public static function loadAll(ArcanistGitAPI $api) {
-    $branches_raw = $api->getAllBranches();
-    $branches = array();
-    foreach ($branches_raw as $branch_raw) {
-      $branch_info = new BranchInfo($branch_raw['name']);
-      $branch_info->setSha1($branch_raw['sha1']);
-      if ($branch_raw['current']) {
-        $branch_info->setCurrent();
-      }
-      $branches[] = $branch_info;
-    }
-
-    $name_sha1_map = mpull($branches, 'getSha1', 'getName');
-    $commits_list = $api->multigetCommitMessages(
-      array_unique(array_values($name_sha1_map)),
-      "%ct%n%an%n%s%n%b");
-    foreach ($branches as $branch) {
-      $sha1 = $name_sha1_map[$branch->getName()];
-      $branch->setSha1($sha1);
-      $branch->parseCommitMessage($commits_list[$sha1]);
-    }
-    $branches = msort($branches, 'getCommitTime');
-    return $branches;
-  }
-
-  public function __construct($branch_name) {
-    $this->branchName = $branch_name;
-  }
-
-  public function setSha1($sha1) {
-    $this->sha1 = $sha1;
-    return $this;
-  }
-
-  public function getSha1() {
-    return $this->sha1;
-  }
-
-  public function setCurrent() {
-    $this->currentHead = true;
-    return $this;
-  }
-
-  public function isCurrentHead() {
-    return $this->currentHead;
-  }
-
-
-  public function setStatus($status) {
-    $this->status = $status;
-    return $this;
-  }
-
-  public function getStatus() {
-    return $this->status;
-  }
-
-  public function getRevisionID() {
-    return $this->revisionID;
-  }
-
-  public function getCommitTime() {
-    return $this->commitTime;
-  }
-
-  public function getCommitSubject() {
-    return $this->commitSubject;
-  }
-
-  public function getCommitDisplayName() {
-    if ($this->revisionID) {
-      return 'D'.$this->revisionID.': '.$this->commitSubject;
-    } else {
-      return $this->commitSubject;
-    }
-  }
-
-  public function getCommitAuthor() {
-    return $this->commitAuthor;
-  }
-
-  public function getName() {
-    return $this->branchName;
-  }
-
-  /**
-   * Based on the 'git show' output extracts the commit date, author,
-   * subject nad Differential revision .
-   * 'Differential Revision:'
-   *
-   * @param string message output of git show -s --format="format:%ct%n%cn%n%b"
-   */
-  public function parseCommitMessage($message) {
-    $message_lines = explode("\n", trim($message));
-    $this->commitTime = $message_lines[0];
-    $this->commitAuthor = $message_lines[1];
-    $this->commitSubject = trim($message_lines[2]);
-    $this->revisionID =
-      ArcanistDifferentialCommitMessage::newFromRawCorpus($message)
-      ->getRevisionID();
-  }
-
-  public function getFormattedName() {
-    $res = "";
-    if ($this->currentHead) {
-      $res = '* ';
-    }
-    $res .= $this->branchName;
-    return phutil_console_format('**%s**', $res);
-
-  }
-
-  /**
-   * Generates a colored status name
-   */
-  public function getFormattedStatus() {
-    return self::renderColorizedRevisionStatus($this->status);
-  }
-
-  /**
-   * Assigns a pretty color based on the status
-   */
-  private static function getColorForStatus($status) {
-    static $status_to_color = array(
-      'Closed' => 'cyan',
-      'Needs Review' => 'magenta',
-      'Needs Revision' => 'red',
-      'Accepted' => 'green',
-      'No Revision' => 'blue',
-      'Abandoned' => 'default',
-    );
-    return idx($status_to_color, $status, 'default');
-  }
-
-  public static function renderColorizedRevisionStatus($status) {
-    return phutil_console_format(
-      '<fg:'.self::getColorForStatus($status).'>%s</fg>',
-      $status);
-  }
-
-}

src/repository/api/ArcanistGitAPI.php

@@ -637,63 +637,34 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
   /**
    * Returns names of all the branches in the current repository.
    *
-   * @return array where each element is a triple ('name', 'sha1', 'current')
+   * @return list<dict<string, string>> Dictionary of branch information.
    */
   public function getAllBranches() {
-    list($branch_info) = $this->execxLocal('branch --no-color');
+    list($branch_info) = $this->execxLocal(
-    $lines = explode("\n", trim($branch_info));
+      'branch --verbose --abbrev=40 --no-color');
+    $lines = explode("\n", rtrim($branch_info));
+
     $result = array();
     foreach ($lines as $line) {
-      $match = array();
+
-      preg_match('/^(\*?)\s*(.*)$/', $line, $match);
+      if (preg_match('/^[* ]+\(no branch\)/', $line)) {
-      $name = $match[2];
+        // This is indicating that the working copy is in a detached state;
-      if ($name == '(no branch)') {
+        // just ignore it.
-        // Just ignore this, we could theoretically try to figure out the ref
-        // and treat it like a real branch but that's sort of ridiculous.
         continue;
       }
+
+      list($current, $name, $hash, $desc) = preg_split('/\s+/', $line, 4);
       $result[] = array(
-        'current' => !empty($match[1]),
+        'current' => !empty($current),
         'name'    => $name,
+        'hash'    => $hash,
+        'desc'    => $desc,
       );
     }
-    $all_names = ipull($result, 'name');
-    // Calling 'git branch' first and then 'git rev-parse' is way faster than
-    // 'git branch -v' for some reason.
-    list($sha1s_string) = $this->execxLocal('rev-parse %Ls', $all_names);
 
-    $sha1_map = array_combine($all_names, explode("\n", trim($sha1s_string)));
-    foreach ($result as &$branch) {
-      $branch['sha1'] = $sha1_map[$branch['name']];
-    }
     return $result;
   }
 
-  /**
-   * Returns git commit messages for the given revisions,
-   * in the specified format (see git show --help for options).
-   *
-   * @param array $revs a list of commit hashes
-   * @param string $format the format to show messages in
-   */
-  public function multigetCommitMessages($revs, $format) {
-
-    list($commits_string) = $this->execxLocal(
-      "show -s --pretty='format:'%s%s %Ls",
-      $format,
-      '%x00',
-      $revs);
-
-    $commits_list = array_slice(explode("\0", $commits_string), 0, -1);
-    $commits_list = array_combine($revs, $commits_list);
-    return $commits_list;
-  }
-
-  public function getRepositoryOwner() {
-    list($owner) = $this->execxLocal('config --get user.name');
-    return trim($owner);
-  }
-
   public function getWorkingCopyRevision() {
     list($stdout) = $this->execxLocal('rev-parse HEAD');
     return rtrim($stdout, "\n");

src/workflow/ArcanistBranchWorkflow.php

@@ -27,7 +27,7 @@ final class ArcanistBranchWorkflow extends ArcanistBaseWorkflow {
 
   public function getCommandSynopses() {
     return phutil_console_format(<<<EOTEXT
-      **branch**
+      **branch** [__options__]
 EOTEXT
       );
   }
@@ -37,9 +37,12 @@ EOTEXT
           Supports: git
           A wrapper on 'git branch'. It pulls data from Differential and
           displays the revision status next to the branch name.
-          Branches are sorted in ascending order by the last commit time.
+
-          By default branches with closed/abandoned revisions
+          By default, branches are sorted chronologically. You can sort them
-          are not displayed.
+          by status instead with __--by-status__.
+
+          By default, branches that are "Closed" or "Abandoned" are not
+          displayed. You can show them with __--view-all__.
 EOTEXT
       );
   }
@@ -60,11 +63,10 @@ EOTEXT
   public function getArguments() {
     return array(
       'view-all' => array(
-        'help' =>
+        'help' => 'Include closed and abandoned revisions',
-          "Include closed and abandoned revisions",
       ),
       'by-status' => array(
-        'help' => 'Group output by revision status.',
+        'help' => 'Sort branches by status instead of time.',
       ),
     );
   }
@@ -73,103 +75,179 @@ EOTEXT
     $repository_api = $this->getRepositoryAPI();
     if (!($repository_api instanceof ArcanistGitAPI)) {
       throw new ArcanistUsageException(
-        "arc branch is only supported under git."
+        'arc branch is only supported under git.');
+    }
+
+    $branches = $repository_api->getAllBranches();
+    if (!$branches) {
+      throw new ArcanistUsageException('No branches in this working copy.');
+    }
+
+    $commit_map = $this->loadCommitInfo($branches, $repository_api);
+    foreach ($branches as $key => $branch) {
+      $branches[$key] += $commit_map[$branch['hash']];
+    }
+
+    $revisions = $this->loadRevisions($branches);
+
+    $this->printBranches($branches, $revisions);
+
+    return 0;
+  }
+
+  private function loadCommitInfo(
+    array $branches,
+    ArcanistRepositoryAPI $repository_api) {
+
+    $commits = ipull($branches, 'hash');
+    list($info) = $repository_api->execxLocal(
+      'log --format=%C %Ls --',
+      '%H%x01%ct%x01%T%x01%s%n%b%x02',
+      $commits);
+
+    $commit_map = array();
+
+    $info = array_filter(explode("\2", trim($info)));
+    foreach ($info as $line) {
+      list($hash, $epoch, $tree, $text) = explode("\1", trim($line), 4);
+      $message = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
+      $id = $message->getRevisionID();
+
+      $commit_map[$hash] = array(
+        'epoch'       => (int)$epoch,
+        'tree'        => $tree,
+        'revisionID'  => $id,
       );
     }
 
-    $this->branches = BranchInfo::loadAll($repository_api);
+    return $commit_map;
-    $all_revisions = array_unique(
-      array_filter(mpull($this->branches, 'getRevisionId')));
-    $revision_status = $this->loadDifferentialStatuses($all_revisions);
-    $owner = $repository_api->getRepositoryOwner();
-    foreach ($this->branches as $branch) {
-      if ($branch->getCommitAuthor() != $owner) {
-        $branch->setStatus('Not Yours');
-        continue;
   }
 
-      $rev_id = $branch->getRevisionID();
+  private function loadRevisions(array $branches) {
-      if ($rev_id) {
+    $ids = array();
-        $status = idx($revision_status, $rev_id, 'Unknown Status');
+    $hashes = array();
-        $branch->setStatus($status);
+
-      } else {
+    foreach ($branches as $branch) {
-        $branch->setStatus('No Revision');
+      if ($branch['revisionID']) {
+        $ids[] = $branch['revisionID'];
       }
-    }
+      $hashes[] = array('gtcm', $branch['hash']);
-    if (!$this->getArgument('view-all')) {
+      $hashes[] = array('gttr', $branch['tree']);
-      $this->filterOutFinished();
-    }
-    $this->printInColumns();
     }
 
+    $calls = array();
 
-
+    if ($ids) {
-  /**
+      $calls[] = $this->getConduit()->callMethod(
-   * Makes a conduit call to differential to find out revision statuses
-   * based on their IDs
-   */
-  private function loadDifferentialStatuses($rev_ids) {
-    $conduit = $this->getConduit();
-    $revisions = $conduit->callMethodSynchronous(
         'differential.query',
         array(
-        'ids'   => $rev_ids,
+          'ids' => $ids,
         ));
-    $statuses = ipull($revisions, 'statusName', 'id');
-    return $statuses;
     }
 
-  /**
+    if ($hashes) {
-   * Removes the branches with status either closed or abandoned.
+      $calls[] = $this->getConduit()->callMethod(
-   */
+        'differential.query',
-  private function filterOutFinished() {
+        array(
-    foreach ($this->branches as $id => $branch) {
+          'commitHashes' => $hashes,
-      if ($branch->isCurrentHead() ) {
+        ));
-        continue; //never filter the current branch
     }
-      $status = $branch->getStatus();
+
+    $results = array();
+    foreach (Futures($calls) as $call) {
+      $results[] = $call->resolve();
+    }
+
+    return array_mergev($results);
+  }
+
+  private function printBranches(array $branches, array $revisions) {
+    $revisions = ipull($revisions, null, 'id');
+
+    static $color_map = array(
+      'Closed'          => 'cyan',
+      'Needs Review'    => 'magenta',
+      'Needs Revision'  => 'red',
+      'Accepted'        => 'green',
+      'No Revision'     => 'blue',
+      'Abandoned'       => 'default',
+    );
+
+    static $ssort_map = array(
+      'Closed'          => 1,
+      'No Revision'     => 2,
+      'Needs Review'    => 3,
+      'Needs Revision'  => 4,
+      'Accepted'        => 5,
+    );
+
+    $out = array();
+    foreach ($branches as $branch) {
+      $revision = idx($revisions, idx($branch, 'revisionID'));
+
+      // If we haven't identified a revision by ID, try to identify it by hash.
+      if (!$revision) {
+        foreach ($revisions as $rev) {
+          $hashes = idx($rev, 'hashes', array());
+          foreach ($hashes as $hash) {
+            if (($hash[0] == 'gtcm' && $hash[1] == $branch['hash']) ||
+                ($hash[0] == 'gttr' && $hash[1] == $branch['tree'])) {
+              $revision = $rev;
+              break;
+            }
+          }
+        }
+      }
+
+      if ($revision) {
+        $desc = 'D'.$revision['id'].': '.$revision['title'];
+        $status = $revision['statusName'];
+      } else {
+        $desc = $branch['desc'];
+        $status = 'No Revision';
+      }
+
+      if (!$this->getArgument('view-all')) {
         if ($status == 'Closed' || $status == 'Abandoned') {
-        unset($this->branches[$id]);
-      }
-    }
-  }
-
-  public function printInColumns() {
-    $longest_name = 0;
-    $longest_status = 0;
-    foreach ($this->branches as $branch) {
-      $longest_name = max(strlen($branch->getFormattedName()), $longest_name);
-      $longest_status = max(strlen($branch->getStatus()), $longest_status);
-    }
-
-    if ($this->getArgument('by-status')) {
-      $by_status = mgroup($this->branches, 'getStatus');
-      foreach (array('Accepted', 'Needs Revision',
-                     'Needs Review', 'No Revision') as $status) {
-        $branches = idx($by_status, $status);
-        if (!$branches) {
           continue;
         }
-        echo reset($branches)->getFormattedStatus()."\n";
-        foreach ($branches as $branch) {
-          $name_markdown = $branch->getFormattedName();
-          $subject = $branch->getCommitDisplayName();
-          $name_markdown = str_pad($name_markdown, $longest_name + 4, ' ');
-          echo "  $name_markdown $subject\n";
       }
+
+      $epoch = $branch['epoch'];
+
+      $color = idx($color_map, $status, 'default');
+      $ssort = sprintf('%d%012d', idx($ssort_map, $status, 0), $epoch);
+
+      $out[] = array(
+        'name'      => $branch['name'],
+        'current'   => $branch['current'],
+        'status'    => $status,
+        'desc'      => $desc,
+        'color'     => $color,
+        'esort'     => $epoch,
+        'ssort'     => $ssort,
+      );
     }
+
+    $len_name = max(array_map('strlen', ipull($out, 'name'))) + 2;
+    $len_status = max(array_map('strlen', ipull($out, 'status'))) + 2;
+
+    if ($this->getArgument('by-status')) {
+      $out = isort($out, 'ssort');
     } else {
-      foreach ($this->branches as $branch) {
+      $out = isort($out, 'esort');
-        $name_markdown = $branch->getFormattedName();
+    }
-        $status_markdown = $branch->getFormattedStatus();
+
-        $subject = $branch->getCommitDisplayName();
+    $console = PhutilConsole::getConsole();
-        $subject_pad = $longest_status - strlen($branch->getStatus()) + 4;
+    foreach ($out as $line) {
-        $name_markdown =
+      $color = $line['color'];
-          str_pad($name_markdown, $longest_name + 4, ' ');
+      $console->writeOut(
-        $subject =
+        "%s **%s** <fg:{$color}>%s</fg> %s\n",
-          str_pad($subject, strlen($subject) + $subject_pad, ' ', STR_PAD_LEFT);
+        $line['current'] ? '* ' : '  ',
-        echo "$name_markdown $status_markdown $subject\n";
+        str_pad($line['name'], $len_name),
-      }
+        str_pad($line['status'], $len_status),
+        $line['desc']);
     }
   }
+
 }

src/workflow/ArcanistListWorkflow.php

@@ -81,12 +81,10 @@ EOTEXT
         $info[$key]['here'],
         $revision['status'],
         $revision['id']);
-      $info[$key]['statusColorized'] =
+      $info[$key]['statusName'] = $revision['statusName'];
-        BranchInfo::renderColorizedRevisionStatus(
-          $revision['statusName']);
       $status_len = max(
         $status_len,
-        strlen($info[$key]['statusColorized']));
+        strlen($info[$key]['statusName']));
     }
 
     $info = isort($info, 'sort');
@@ -97,7 +95,7 @@ EOTEXT
         $spec['here']
           ? phutil_console_format('**%s**', '*')
           : ' ',
-        $spec['statusColorized'],
+        $spec['statusName'],
         $revision['id'],
         $revision['title']);
     }