Consolidate querying of things which we can use git for-each-ref for

Summary: Ref T2230. This cleans up D7442, by using `git for-each-ref` everywhere we can, in a basically reasonable way.

Test Plan:
In bare and non-bare repositories:

  - Ran discovery with `bin/repository discover`;
  - listed branches on `/diffusion/X/`;
  - listed tags on `/diffusion/X/`;
  - listed tags, branches and refs on `/diffusion/rXnnnn`.

Reviewers: btrahan, avivey

Reviewed By: avivey

CC: aran

Maniphest Tasks: T2230

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

src/__phutil_library_map__.php

@@ -491,6 +491,8 @@ phutil_register_library_map(array(
     'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
     'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php',
     'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
+    'DiffusionLowLevelGitRefQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php',
+    'DiffusionLowLevelQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php',
     'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php',
     'DiffusionMercurialExpandShortNameQuery' => 'applications/diffusion/query/expandshortname/DiffusionMercurialExpandShortNameQuery.php',
     'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php',
@@ -525,6 +527,7 @@ phutil_register_library_map(array(
     'DiffusionRepositoryEditSubversionController' => 'applications/diffusion/controller/DiffusionRepositoryEditSubversionController.php',
     'DiffusionRepositoryListController' => 'applications/diffusion/controller/DiffusionRepositoryListController.php',
     'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php',
+    'DiffusionRepositoryRef' => 'applications/diffusion/data/DiffusionRepositoryRef.php',
     'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php',
     'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php',
     'DiffusionSSHGitReceivePackWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitReceivePackWorkflow.php',
@@ -2684,6 +2687,8 @@ phutil_register_library_map(array(
     'DiffusionLastModifiedController' => 'DiffusionController',
     'DiffusionLintController' => 'DiffusionController',
     'DiffusionLintDetailsController' => 'DiffusionController',
+    'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery',
+    'DiffusionLowLevelQuery' => 'Phobject',
     'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery',
     'DiffusionMercurialExpandShortNameQuery' => 'DiffusionExpandShortNameQuery',
     'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',

src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php

@@ -27,44 +27,31 @@ final class ConduitAPI_diffusion_branchquery_Method
     $limit = $request->getValue('limit');
     $offset = $request->getValue('offset');
 
-    // We need to add 1 in case we pick up HEAD.
-    $count = $offset + $limit + 1;
-
-    if ($repository->isWorkingCopyBare()) {
-      list($stdout) = $repository->execxLocalCommand(
-        'for-each-ref %C --sort=-creatordate --format=%s refs/heads',
-        $count ? '--count='.(int)$count : null,
-        '%(refname:short) %(objectname)');
-      $branch_list = DiffusionGitBranch::parseLocalBranchOutput(
-        $stdout);
-    } else {
-      list($stdout) = $repository->execxLocalCommand(
-        'for-each-ref %C --sort=-creatordate --format=%s refs/remotes',
-        $count ? '--count='.(int)$count : null,
-        '%(refname:short) %(objectname)');
-      $branch_list = DiffusionGitBranch::parseRemoteBranchOutput(
-        $stdout,
-        $only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
-    }
+    $refs = id(new DiffusionLowLevelGitRefQuery())
+      ->setRepository($repository)
+      ->withIsOriginBranch(true)
+      ->execute();
 
     $branches = array();
-    foreach ($branch_list as $name => $head) {
-      if (!$repository->shouldTrackBranch($name)) {
+    foreach ($refs as $ref) {
+      $branch = id(new DiffusionBranchInformation())
+        ->setName($ref->getShortName())
+        ->setHeadCommitIdentifier($ref->getCommitIdentifier());
+
+      if (!$repository->shouldTrackBranch($branch->getName())) {
         continue;
       }
 
-      $branch = new DiffusionBranchInformation();
-      $branch->setName($name);
-      $branch->setHeadCommitIdentifier($head);
       $branches[] = $branch->toDictionary();
     }
 
+    // NOTE: We can't apply the offset or limit until here, because we may have
+    // filtered untrackable branches out of the result set.
+
     if ($offset) {
       $branches = array_slice($branches, $offset);
     }
 
-    // We might have too many even after offset slicing, if there was no HEAD
-    // for some reason.
     if ($limit) {
       $branches = array_slice($branches, 0, $limit);
     }

src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php

@@ -25,6 +25,8 @@ final class ConduitAPI_diffusion_commitbranchesquery_Method
     $repository = $drequest->getRepository();
     $commit = $request->getValue('commit');
 
+    // NOTE: We can't use DiffusionLowLevelGitRefQuery here because
+    // `git for-each-ref` does not support `--contains`.
     if ($repository->isWorkingCopyBare()) {
       list($contains) = $repository->execxLocalCommand(
         'branch --verbose --no-abbrev --contains %s',

src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php

@@ -75,47 +75,21 @@ final class ConduitAPI_diffusion_tagsquery_Method
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
-    $count = $offset + $limit;
-
-    list($stdout) = $repository->execxLocalCommand(
-      'for-each-ref %C --sort=-creatordate --format=%s refs/tags',
-      $count ? '--count='.(int)$count : null,
-      '%(objectname) %(objecttype) %(refname) %(*objectname) %(*objecttype) '.
-        '%(subject)%01%(creator)');
-
-    $stdout = trim($stdout);
-    if (!strlen($stdout)) {
-      return array();
-    }
+    $refs = id(new DiffusionLowLevelGitRefQuery())
+      ->setRepository($repository)
+      ->withIsTag(true)
+      ->execute();
 
     $tags = array();
-    foreach (explode("\n", $stdout) as $line) {
-      list($info, $creator) = explode("\1", $line);
-      list(
-        $objectname,
-        $objecttype,
-        $refname,
-        $refobjectname,
-        $refobjecttype,
-        $description) = explode(' ', $info, 6);
-
-      $matches = null;
-      if (!preg_match('/^(.*) ([0-9]+) ([0-9+-]+)$/', $creator, $matches)) {
-        // It's possible a tag doesn't have a creator (tagger)
-        $author = null;
-        $epoch = null;
-      } else {
-        $author = $matches[1];
-        $epoch  = $matches[2];
-      }
-
-      $tag = new DiffusionRepositoryTag();
-      $tag->setAuthor($author);
-      $tag->setEpoch($epoch);
-      $tag->setCommitIdentifier(nonempty($refobjectname, $objectname));
-      $tag->setName(preg_replace('@^refs/tags/@', '', $refname));
-      $tag->setDescription($description);
-      $tag->setType('git/'.$objecttype);
+    foreach ($refs as $ref) {
+      $fields = $ref->getRawFields();
+      $tag = id(new DiffusionRepositoryTag())
+        ->setAuthor($fields['author'])
+        ->setEpoch($fields['epoch'])
+        ->setCommitIdentifier($ref->getCommitIdentifier())
+        ->setName($ref->getShortName())
+        ->setDescription($fields['subject'])
+        ->setType('git/'.$fields['objecttype']);
 
       $tags[] = $tag;
     }
@@ -124,6 +98,10 @@ final class ConduitAPI_diffusion_tagsquery_Method
       $tags = array_slice($tags, $offset);
     }
 
+    if ($limit) {
+      $tags = array_slice($tags, 0, $limit);
+    }
+
     if ($serialize) {
       $tags = mpull($tags, 'toDictionary');
     }

src/applications/diffusion/data/DiffusionRepositoryRef.php

@@ -0,0 +1,36 @@
+<?php
+
+final class DiffusionRepositoryRef {
+
+  private $shortName;
+  private $commitIdentifier;
+  private $rawFields;
+
+  public function setRawFields(array $raw_fields) {
+    $this->rawFields = $raw_fields;
+    return $this;
+  }
+
+  public function getRawFields() {
+    return $this->rawFields;
+  }
+
+  public function setCommitIdentifier($commit_identifier) {
+    $this->commitIdentifier = $commit_identifier;
+    return $this;
+  }
+
+  public function getCommitIdentifier() {
+    return $this->commitIdentifier;
+  }
+
+  public function setShortName($short_name) {
+    $this->shortName = $short_name;
+    return $this;
+  }
+
+  public function getShortName() {
+    return $this->shortName;
+  }
+
+}

src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php

@@ -0,0 +1,135 @@
+<?php
+
+/**
+ * Execute and parse a low-level Git ref query using `git for-each-ref`. This
+ * is useful for returning a list of tags or branches.
+ *
+ *
+ */
+final class DiffusionLowLevelGitRefQuery extends DiffusionLowLevelQuery {
+
+  private $isTag;
+  private $isOriginBranch;
+
+  public function withIsTag($is_tag) {
+    $this->isTag = $is_tag;
+    return $this;
+  }
+
+  public function withIsOriginBranch($is_origin_branch) {
+    $this->isOriginBranch = $is_origin_branch;
+    return $this;
+  }
+
+  protected function executeQuery() {
+    $repository = $this->getRepository();
+
+    if ($this->isTag && $this->isOriginBranch) {
+      throw new Exception("Specify tags or origin branches, not both!");
+    } else if ($this->isTag) {
+      $prefix = 'refs/tags/';
+    } else if ($this->isOriginBranch) {
+      if ($repository->isWorkingCopyBare()) {
+        $prefix = 'refs/heads/';
+      } else {
+        $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE;
+        $prefix = 'refs/remotes/'.$remote.'/';
+      }
+    } else {
+      throw new Exception("Specify tags or origin branches!");
+    }
+
+    $order = '-creatordate';
+
+    list($stdout) = $repository->execxLocalCommand(
+      'for-each-ref --sort=%s --format=%s %s',
+      $order,
+      $this->getFormatString(),
+      $prefix);
+
+    $stdout = rtrim($stdout);
+    if (!strlen($stdout)) {
+      return array();
+    }
+
+    // NOTE: Although git supports --count, we can't apply any offset or limit
+    // logic until the very end because we may encounter a HEAD which we want
+    // to discard.
+
+    $lines = explode("\n", $stdout);
+    $results = array();
+    foreach ($lines as $line) {
+      $fields = $this->extractFields($line);
+
+      $creator = $fields['creator'];
+      $matches = null;
+      if (preg_match('/^(.*) ([0-9]+) ([0-9+-]+)$/', $creator, $matches)) {
+        $fields['author'] = $matches[1];
+        $fields['epoch'] = (int)$matches[2];
+      } else {
+        $fields['author'] = null;
+        $fields['epoch'] = null;
+      }
+
+      $commit = nonempty($fields['*objectname'], $fields['objectname']);
+
+      $short = substr($fields['refname'], strlen($prefix));
+      if ($short == 'HEAD') {
+        continue;
+      }
+
+      $ref = id(new DiffusionRepositoryRef())
+        ->setShortName($short)
+        ->setCommitIdentifier($commit)
+        ->setRawFields($fields);
+
+      $results[] = $ref;
+    }
+
+    return $results;
+  }
+
+  /**
+   * List of git `--format` fields we want to grab.
+   */
+  private function getFields() {
+    return array(
+      'objectname',
+      'objecttype',
+      'refname',
+      '*objectname',
+      '*objecttype',
+      'subject',
+      'creator',
+    );
+  }
+
+  /**
+   * Get a string for `--format` which enumerates all the fields we want.
+   */
+  private function getFormatString() {
+    $fields = $this->getFields();
+
+    foreach ($fields as $key => $field) {
+      $fields[$key] = '%('.$field.')';
+    }
+
+    return implode("%01", $fields);
+  }
+
+  /**
+   * Parse a line back into fields.
+   */
+  private function extractFields($line) {
+    $fields = $this->getFields();
+    $parts = explode("\1", $line, count($fields));
+
+    $dict = array();
+    foreach ($fields as $index => $field) {
+      $dict[$field] = idx($parts, $index);
+    }
+
+    return $dict;
+  }
+
+}

src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php

@@ -0,0 +1,22 @@
+<?php
+
+abstract class DiffusionLowLevelQuery extends Phobject {
+
+  private $repository;
+
+  abstract protected function executeQuery();
+
+  public function setRepository(PhabricatorRepository $repository) {
+    $this->repository = $repository;
+    return $this;
+  }
+
+  public function getRepository() {
+    return $this->repository;
+  }
+
+  public function execute() {
+    return $this->executeQuery();
+  }
+
+}

src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php

@@ -526,18 +526,12 @@ final class PhabricatorRepositoryPullLocalDaemon
       $repository->getRemoteURI(),
       $repository->getLocalPath());
 
-    if ($repository->isWorkingCopyBare()) {
-      list($stdout) = $repository->execxLocalCommand(
-        'branch --verbose --no-abbrev');
-      $branches = DiffusionGitBranch::parseLocalBranchOutput(
-        $stdout);
-    } else {
-      list($stdout) = $repository->execxLocalCommand(
-        'branch -r --verbose --no-abbrev');
-      $branches = DiffusionGitBranch::parseRemoteBranchOutput(
-        $stdout,
-        $only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
-    }
+    $refs = id(new DiffusionLowLevelGitRefQuery())
+      ->setRepository($repository)
+      ->withIsOriginBranch(true)
+      ->execute();
+
+    $branches = mpull($refs, 'getCommitIdentifier', 'getShortName');
 
     if (!$branches) {
       // This repository has no branches at all, so we don't need to do