Fetch and discover all Git ref types, not just branches

Summary:
Ref T9028. Fixes T6878. Currently, we only fetch and discover branches. This is fine 99% of the time but sometimes commits are pushed to just a tag, e.g.:

```
git checkout <some hash>
nano file.c
git commit -am '...'
git tag wild-wild-west
git push origin wild-wild-west
```

Through a similar process, commits can also be pushed to some arbitrary named ref (we do this for staging areas).

With the current rules, we don't fetch tag refs and won't discover these commits.

Change the rules so:

  - we fetch all refs; and
  - we discover ancestors of all refs.

Autoclose rules for tags and arbitrary refs are just hard-coded for now. We might make these more flexible in the future, or we might do forks instead, or maybe we'll have to do both.

Test Plan:
Pushed a commit to a tag ONLY (`vegetable1`).

<cf508b8de6>

On `master`, prior to the change:

  - Used `update` + `refs` + `discover`.
  - Verified tag was not fetched with `git for-each-ref` in local working copy and the web UI.
  - Verified commit was not discovered using the web UI.

With this patch applied:

  - Used `update`, saw a `refs/*` fetch instead of a `refs/heads/*` fetch.
  - Used `git for-each-ref` to verify that tag fetched.
  - Used `repository refs`.
  - Saw new tag appear in the tags list in the web UI.
  - Saw new refcursor appear in refcursor table.
  - Used `repository discover --verbose` and examine refs for sanity.
  - Saw commit row appear in database.
  - Saw commit skeleton appear in web UI.
  - Ran `bin/phd debug task`.
  - Saw commit fully parse.

{F1689319}

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Maniphest Tasks: T6878, T9028

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

src/applications/diffusion/data/DiffusionRepositoryRef.php

@@ -7,6 +7,7 @@ final class DiffusionRepositoryRef extends Phobject {
 
   private $shortName;
   private $commitIdentifier;
+  private $refType;
   private $rawFields = array();
 
   public function setRawFields(array $raw_fields) {
@@ -36,6 +37,20 @@ final class DiffusionRepositoryRef extends Phobject {
     return $this->shortName;
   }
 
+  public function setRefType($ref_type) {
+    $this->refType = $ref_type;
+    return $this;
+  }
+
+  public function getRefType() {
+    return $this->refType;
+  }
+
+  public function isBranch() {
+    $type_branch = PhabricatorRepositoryRefCursor::TYPE_BRANCH;
+    return ($this->getRefType() === $type_branch);
+  }
+
 
 /* -(  Serialization  )------------------------------------------------------ */
 
@@ -44,6 +59,7 @@ final class DiffusionRepositoryRef extends Phobject {
     return array(
       'shortName' => $this->shortName,
       'commitIdentifier' => $this->commitIdentifier,
+      'refType' => $this->refType,
       'rawFields' => $this->rawFields,
     );
   }
@@ -52,6 +68,7 @@ final class DiffusionRepositoryRef extends Phobject {
     return id(new DiffusionRepositoryRef())
       ->setShortName($dict['shortName'])
       ->setCommitIdentifier($dict['commitIdentifier'])
+      ->setRefType($dict['refType'])
       ->setRawFields($dict['rawFields']);
   }
 

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

@@ -14,96 +14,112 @@ final class DiffusionLowLevelGitRefQuery extends DiffusionLowLevelQuery {
   }
 
   protected function executeQuery() {
+    $type_branch = PhabricatorRepositoryRefCursor::TYPE_BRANCH;
+    $type_tag = PhabricatorRepositoryRefCursor::TYPE_TAG;
+    $type_ref = PhabricatorRepositoryRefCursor::TYPE_REF;
+
     $ref_types = $this->refTypes;
-    if ($ref_types) {
-      $type_branch = PhabricatorRepositoryRefCursor::TYPE_BRANCH;
-      $type_tag = PhabricatorRepositoryRefCursor::TYPE_TAG;
-
-      $ref_types = array_fuse($ref_types);
-
-      $with_branches = isset($ref_types[$type_branch]);
-      $with_tags = isset($ref_types[$type_tag]);
-    } else {
-      $with_branches = true;
-      $with_tags = true;
+    if (!$ref_types) {
+      $ref_types = array($type_branch, $type_tag, $type_ref);
     }
 
+    $ref_types = array_fuse($ref_types);
+
+    $with_branches = isset($ref_types[$type_branch]);
+    $with_tags = isset($ref_types[$type_tag]);
+    $with_refs = isset($refs_types[$type_ref]);
+
     $repository = $this->getRepository();
 
     $prefixes = array();
 
-    if ($with_branches) {
-      if ($repository->isWorkingCopyBare()) {
-        $prefix = 'refs/heads/';
-      } else {
-        $remote = DiffusionGitBranch::DEFAULT_GIT_REMOTE;
-        $prefix = 'refs/remotes/'.$remote.'/';
+    if ($repository->isWorkingCopyBare()) {
+      $branch_prefix = 'refs/heads/';
+    } else {
+      $remote = DiffusionGitBranch::DEFAULT_GIT_REMOTE;
+      $branch_prefix = 'refs/remotes/'.$remote.'/';
+    }
+
+    $tag_prefix = 'refs/tags/';
+
+
+    if ($with_refs || count($ref_types) > 1) {
+      // If we're loading refs or more than one type of ref, just query
+      // everything.
+      $prefix = 'refs/';
+    } else {
+      if ($with_branches) {
+        $prefix = $branch_prefix;
+      }
+      if ($with_tags) {
+        $prefix = $tag_prefix;
       }
-      $prefixes[] = $prefix;
     }
 
-    if ($with_tags) {
-      $prefixes[] = 'refs/tags/';
+    $branch_len = strlen($branch_prefix);
+    $tag_len = strlen($tag_prefix);
+
+    list($stdout) = $repository->execxLocalCommand(
+      'for-each-ref --sort=%s --format=%s -- %s',
+      '-creatordate',
+      $this->getFormatString(),
+      $prefix);
+
+    $stdout = rtrim($stdout);
+    if (!strlen($stdout)) {
+      return array();
     }
 
-    $order = '-creatordate';
-
-    $futures = array();
-    foreach ($prefixes as $prefix) {
-      $futures[$prefix] = $repository->getLocalCommandFuture(
-        'for-each-ref --sort=%s --format=%s %s',
-        $order,
-        $this->getFormatString(),
-        $prefix);
-    }
-
-    // Resolve all the futures first. We want to iterate over them in prefix
-    // order, not resolution order.
-    foreach (new FutureIterator($futures) as $prefix => $future) {
-      $future->resolvex();
-    }
+    // 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 ($futures as $prefix => $future) {
-      list($stdout) = $future->resolvex();
+    foreach ($lines as $line) {
+      $fields = $this->extractFields($line);
 
-      $stdout = rtrim($stdout);
-      if (!strlen($stdout)) {
+      $refname = $fields['refname'];
+      if (!strncmp($refname, $branch_prefix, $branch_len)) {
+        $short = substr($refname, $branch_len);
+        $type = $type_branch;
+      } else if (!strncmp($refname, $tag_prefix, $tag_len)) {
+        $short = substr($refname, $tag_len);
+        $type = $type_tag;
+      } else {
+        $short = $refname;
+        $type = $type_ref;
+      }
+
+      // If this isn't a type of ref we care about, skip it.
+      if (empty($ref_types[$type])) {
         continue;
       }
 
-      // 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);
-      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;
+      // If this is the local HEAD, skip it.
+      if ($short == 'HEAD') {
+        continue;
       }
+
+      $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']);
+
+      $ref = id(new DiffusionRepositoryRef())
+        ->setRefType($type)
+        ->setShortName($short)
+        ->setCommitIdentifier($commit)
+        ->setRawFields($fields);
+
+      $results[] = $ref;
     }
 
     return $results;

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -130,39 +130,35 @@ final class PhabricatorRepositoryDiscoveryEngine
       $this->verifyGitOrigin($repository);
     }
 
-    // TODO: This should also import tags, but some of the logic is still
-    // branch-specific today.
-
-    $branches = id(new DiffusionLowLevelGitRefQuery())
+    $heads = id(new DiffusionLowLevelGitRefQuery())
       ->setRepository($repository)
-      ->withRefTypes(
-        array(
-          PhabricatorRepositoryRefCursor::TYPE_BRANCH,
-        ))
       ->execute();
 
-    if (!$branches) {
-      // This repository has no branches at all, so we don't need to do
+    if (!$heads) {
+      // This repository has no heads at all, so we don't need to do
       // anything. Generally, this means the repository is empty.
       return array();
     }
 
-    $branches = $this->sortBranches($branches);
-    $branches = mpull($branches, 'getCommitIdentifier', 'getShortName');
+    $heads = $this->sortRefs($heads);
+    $head_commits = mpull($heads, 'getCommitIdentifier');
 
     $this->log(
       pht(
         'Discovering commits in repository "%s".',
         $repository->getDisplayName()));
 
-    $this->fillCommitCache(array_values($branches));
+    $this->fillCommitCache($head_commits);
 
     $refs = array();
-    foreach ($branches as $name => $commit) {
-      $this->log(pht('Examining branch "%s", at "%s".', $name, $commit));
+    foreach ($heads as $ref) {
+      $name = $ref->getShortName();
+      $commit = $ref->getCommitIdentifier();
 
-      if (!$repository->shouldTrackBranch($name)) {
-        $this->log(pht('Skipping, branch is untracked.'));
+      $this->log(pht('Examining ref "%s", at "%s".', $name, $commit));
+
+      if (!$repository->shouldTrackRef($ref)) {
+        $this->log(pht('Skipping, ref is untracked.'));
         continue;
       }
 
@@ -173,14 +169,14 @@ final class PhabricatorRepositoryDiscoveryEngine
 
       $this->log(pht('Looking for new commits.'));
 
-      $branch_refs = $this->discoverStreamAncestry(
+      $head_refs = $this->discoverStreamAncestry(
         new PhabricatorGitGraphStream($repository, $commit),
         $commit,
-        $repository->shouldAutocloseBranch($name));
+        $repository->shouldAutocloseRef($ref));
 
-      $this->didDiscoverRefs($branch_refs);
+      $this->didDiscoverRefs($head_refs);
 
-      $refs[] = $branch_refs;
+      $refs[] = $head_refs;
     }
 
     return array_mergev($refs);
@@ -469,25 +465,23 @@ final class PhabricatorRepositoryDiscoveryEngine
    *
    * @task internal
    *
-   * @param   list<DiffusionRepositoryRef> List of branch heads.
-   * @return  list<DiffusionRepositoryRef> Sorted list of branch heads.
+   * @param   list<DiffusionRepositoryRef> List of refs.
+   * @return  list<DiffusionRepositoryRef> Sorted list of refs.
    */
-  private function sortBranches(array $branches) {
+  private function sortRefs(array $refs) {
     $repository = $this->getRepository();
 
-    $head_branches = array();
-    $tail_branches = array();
-    foreach ($branches as $branch) {
-      $name = $branch->getShortName();
-
-      if ($repository->shouldAutocloseBranch($name)) {
-        $head_branches[] = $branch;
+    $head_refs = array();
+    $tail_refs = array();
+    foreach ($refs as $ref) {
+      if ($repository->shouldAutocloseRef($ref)) {
+        $head_refs[] = $ref;
       } else {
-        $tail_branches[] = $branch;
+        $tail_refs[] = $ref;
       }
     }
 
-    return array_merge($head_branches, $tail_branches);
+    return array_merge($head_refs, $tail_refs);
   }
 
 

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -347,7 +347,7 @@ final class PhabricatorRepositoryPullEngine
         // For bare working copies, we need this magic incantation.
         $future = $repository->getRemoteCommandFuture(
           'fetch origin %s --prune',
-          '+refs/heads/*:refs/heads/*');
+          '+refs/*:refs/*');
       } else {
         $future = $repository->getRemoteCommandFuture(
           'fetch --all --prune');

src/applications/repository/engine/PhabricatorRepositoryRefEngine.php

@@ -25,29 +25,31 @@ final class PhabricatorRepositoryRefEngine
     switch ($vcs) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
         // No meaningful refs of any type in Subversion.
-        $branches = array();
-        $bookmarks = array();
-        $tags = array();
+        $maps = array();
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
         $branches = $this->loadMercurialBranchPositions($repository);
         $bookmarks = $this->loadMercurialBookmarkPositions($repository);
-        $tags = array();
+        $maps = array(
+          PhabricatorRepositoryRefCursor::TYPE_BRANCH => $branches,
+          PhabricatorRepositoryRefCursor::TYPE_BOOKMARK => $bookmarks,
+        );
+
         $branches_may_close = true;
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-        $branches = $this->loadGitBranchPositions($repository);
-        $bookmarks = array();
-        $tags = $this->loadGitTagPositions($repository);
+        $maps = $this->loadGitRefPositions($repository);
         break;
       default:
         throw new Exception(pht('Unknown VCS "%s"!', $vcs));
     }
 
-    $maps = array(
-      PhabricatorRepositoryRefCursor::TYPE_BRANCH => $branches,
-      PhabricatorRepositoryRefCursor::TYPE_TAG => $tags,
-      PhabricatorRepositoryRefCursor::TYPE_BOOKMARK => $bookmarks,
+    // Fill in any missing types with empty lists.
+    $maps = $maps + array(
+      PhabricatorRepositoryRefCursor::TYPE_BRANCH => array(),
+      PhabricatorRepositoryRefCursor::TYPE_TAG => array(),
+      PhabricatorRepositoryRefCursor::TYPE_BOOKMARK => array(),
+      PhabricatorRepositoryRefCursor::TYPE_REF => array(),
     );
 
     $all_cursors = id(new PhabricatorRepositoryRefCursorQuery())
@@ -91,6 +93,7 @@ final class PhabricatorRepositoryRefEngine
       $this->deadRefs = array();
     }
 
+    $branches = $maps[PhabricatorRepositoryRefCursor::TYPE_BRANCH];
     if ($branches && $branches_may_close) {
       $this->updateBranchStates($repository, $branches);
     }
@@ -449,28 +452,12 @@ final class PhabricatorRepositoryRefEngine
   /**
    * @task git
    */
-  private function loadGitBranchPositions(PhabricatorRepository $repository) {
-    return id(new DiffusionLowLevelGitRefQuery())
+  private function loadGitRefPositions(PhabricatorRepository $repository) {
+    $refs = id(new DiffusionLowLevelGitRefQuery())
       ->setRepository($repository)
-      ->withRefTypes(
-        array(
-          PhabricatorRepositoryRefCursor::TYPE_BRANCH,
-        ))
       ->execute();
-  }
 
-
-  /**
-   * @task git
-   */
-  private function loadGitTagPositions(PhabricatorRepository $repository) {
-    return id(new DiffusionLowLevelGitRefQuery())
-      ->setRepository($repository)
-      ->withRefTypes(
-        array(
-          PhabricatorRepositoryRefCursor::TYPE_TAG,
-        ))
-      ->execute();
+    return mgroup($refs, 'getRefType');
   }
 
 

src/applications/repository/storage/PhabricatorRepository.php

@@ -910,6 +910,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return null;
   }
 
+  public function shouldTrackRef(DiffusionRepositoryRef $ref) {
+    if (!$ref->isBranch()) {
+      return true;
+    }
+
+    return $this->shouldTrackBranch($ref->getShortName());
+  }
+
   public function shouldTrackBranch($branch) {
     return $this->isBranchInFilter($branch, 'branch-filter');
   }
@@ -1020,6 +1028,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
 /* -(  Autoclose  )---------------------------------------------------------- */
 
 
+  public function shouldAutocloseRef(DiffusionRepositoryRef $ref) {
+    if (!$ref->isBranch()) {
+      return false;
+    }
+
+    return $this->shouldAutocloseBranch($ref->getShortName());
+  }
+
   /**
    * Determine if autoclose is active for a branch.
    *

src/applications/repository/storage/PhabricatorRepositoryRefCursor.php

@@ -12,6 +12,7 @@ final class PhabricatorRepositoryRefCursor
   const TYPE_BRANCH = 'branch';
   const TYPE_TAG = 'tag';
   const TYPE_BOOKMARK = 'bookmark';
+  const TYPE_REF = 'ref';
 
   protected $repositoryPHID;
   protected $refType;