From 83121f78980be7d7089cc9cb5e8e3f2c0eb33573 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 20 Jun 2016 11:07:00 -0700
Subject: [PATCH] (stable) Handle tag tags properly in discovery

Summary:
Fixes T11180. In Git, it's possible to tag a tag (????). When you do, we try to log the tag-object, which automatically resolves to the commit and fails.

Just skip these. If "A" points at "B" which points at "C", it's fine to ignore "A" and "B" since we'll get the same stuff when we process "C".

Test Plan:
  - Tagged a tag.
  - Pushed it.
  - Discovered it.
  - Before patch: got exception similar to the one in T11180.
  - After patch: got tag-tag skipped. Also got slightly better error messages.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11180

Differential Revision: https://secure.phabricator.com/D16149
---
 .../daemon/PhabricatorGitGraphStream.php       | 18 ++++++++++++++----
 .../PhabricatorRepositoryDiscoveryEngine.php   | 16 +++++++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/applications/repository/daemon/PhabricatorGitGraphStream.php b/src/applications/repository/daemon/PhabricatorGitGraphStream.php
index 669cf9c105..b175053528 100644
--- a/src/applications/repository/daemon/PhabricatorGitGraphStream.php
+++ b/src/applications/repository/daemon/PhabricatorGitGraphStream.php
@@ -5,6 +5,7 @@ final class PhabricatorGitGraphStream
 
   private $repository;
   private $iterator;
+  private $startCommit;
 
   private $parents        = array();
   private $dates          = array();
@@ -14,6 +15,7 @@ final class PhabricatorGitGraphStream
     $start_commit = null) {
 
     $this->repository = $repository;
+    $this->startCommit = $start_commit;
 
     if ($start_commit !== null) {
       $future = $repository->getLocalCommandFuture(
@@ -82,10 +84,18 @@ final class PhabricatorGitGraphStream
       }
     }
 
-    throw new Exception(
-      pht(
-        "No such commit '%s' in repository!",
-        $commit));
+    if ($this->startCommit !== null) {
+      throw new Exception(
+        pht(
+          'Commit "%s" is not a reachable ancestor of "%s".',
+          $commit,
+          $this->startCommit));
+    } else {
+      throw new Exception(
+        pht(
+          'Commit "%s" is not a reachable ancestor of any ref.',
+          $commit));
+    }
   }
 
   private function isParsed($commit) {
diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
index cdb7e11d03..fd8c57e9b4 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -157,7 +157,12 @@ final class PhabricatorRepositoryDiscoveryEngine
       $name = $ref->getShortName();
       $commit = $ref->getCommitIdentifier();
 
-      $this->log(pht('Examining ref "%s", at "%s".', $name, $commit));
+      $this->log(
+        pht(
+          'Examining "%s" (%s) at "%s".',
+          $name,
+          $ref->getRefType(),
+          $commit));
 
       if (!$repository->shouldTrackRef($ref)) {
         $this->log(pht('Skipping, ref is untracked.'));
@@ -169,6 +174,15 @@ final class PhabricatorRepositoryDiscoveryEngine
         continue;
       }
 
+      // In Git, it's possible to tag a tag. We just skip these, we'll discover
+      // them when we process the target tag. See T11180.
+      $fields = $ref->getRawFields();
+      $tag_type = idx($fields, '*objecttype');
+      if ($tag_type == 'tag') {
+        $this->log(pht('Skipping, this is a tag of a tag.'));
+        continue;
+      }
+
       $this->log(pht('Looking for new commits.'));
 
       $head_refs = $this->discoverStreamAncestry(