Move Mercurial discovery to PhabricatorRepositoryDiscoveryEngine

Summary: Ref T4068. Partly, this moves discovery to the more unit-testable PhabricatorRepositoryDiscoveryEngine. It also fixes some issues, see inlines.

Test Plan: In a Mercurial repository, ran `bin/repository discover --repair`, verified commits came out topographically sorted. Ran without `--repair` and in various other contexts, like with no commits to discover and some-but-not-all commits to discover.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4068

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

src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php

@@ -232,12 +232,10 @@ final class PhabricatorRepositoryPullLocalDaemon
         $result = $this->executeGitDiscover($repository);
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
         $refs = $this->getDiscoveryEngine($repository)
           ->discoverCommits();
         break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        $result = $this->executeHgDiscover($repository);
-        break;
       default:
         throw new Exception("Unknown VCS '{$vcs}'!");
     }
@@ -739,63 +737,4 @@ final class PhabricatorRepositoryPullLocalDaemon
   }
 
 
-/* -(  Mercurial Implementation  )------------------------------------------- */
-
-
-  private function executeHgDiscover(PhabricatorRepository $repository) {
-
-    $branches = id(new DiffusionLowLevelMercurialBranchesQuery())
-      ->setRepository($repository)
-      ->execute();
-    $branches = mpull($branches, 'getHeadCommitIdentifier', 'getName');
-
-    $got_something = false;
-    foreach ($branches as $name => $commit) {
-      if ($this->isKnownCommit($repository, $commit)) {
-        continue;
-      } else {
-        $this->executeHgDiscoverCommit($repository, $commit);
-        $got_something = true;
-      }
-    }
-
-    return $got_something;
-  }
-
-  private function executeHgDiscoverCommit(
-    PhabricatorRepository $repository,
-    $commit) {
-
-    $discover = array($commit);
-    $insert = array($commit);
-
-    $seen_parent = array();
-
-    $stream = new PhabricatorMercurialGraphStream($repository);
-
-    // For all the new commits at the branch heads, walk backward until we
-    // find only commits we've aleady seen.
-    while ($discover) {
-      $target = array_pop($discover);
-
-      $parents = $stream->getParents($target);
-
-      foreach ($parents as $parent) {
-        if (isset($seen_parent[$parent])) {
-          continue;
-        }
-        $seen_parent[$parent] = true;
-        if (!$this->isKnownCommit($repository, $parent)) {
-          $discover[] = $parent;
-          $insert[] = $parent;
-        }
-      }
-    }
-
-    foreach ($insert as $target) {
-      $epoch = $stream->getCommitDate($target);
-      $this->recordCommit($repository, $target, $epoch);
-    }
-  }
-
 }

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -40,17 +40,15 @@ final class PhabricatorRepositoryDiscoveryEngine
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
         $refs = $this->discoverSubversionCommits();
         break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $refs = $this->discoverMercurialCommits();
+        break;
 /*
-
+  TODO: Implement this!
-      TODO: Implement these!
 
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-        $refs = $this->executeGitDiscovery();
+        $refs = $this->discoverGitCommits();
         break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        $refs = $this->executeMercurialDiscovery();
-        break;
-
 */
       default:
         throw new Exception("Unknown VCS '{$vcs}'!");
@@ -138,9 +136,113 @@ final class PhabricatorRepositoryDiscoveryEngine
   }
 
 
+/* -(  Discovering Mercurial Repositories  )--------------------------------- */
+
+
+  /**
+   * @task hg
+   */
+  private function discoverMercurialCommits() {
+    $repository = $this->getRepository();
+
+    $branches = id(new DiffusionLowLevelMercurialBranchesQuery())
+      ->setRepository($repository)
+      ->execute();
+    $branches = mpull($branches, 'getHeadCommitIdentifier', 'getName');
+
+    $refs = array();
+    foreach ($branches as $name => $commit) {
+      $this->log("Examining branch '{$name}', at {$commit}'.");
+      if (!$repository->shouldTrackBranch($name)) {
+        $this->log("Skipping, branch is untracked.");
+        continue;
+      }
+
+      if ($this->isKnownCommit($commit)) {
+        $this->log("Skipping, tip is a known commit.");
+        continue;
+      }
+
+      $this->log("Looking for new commits.");
+      $refs[] = $this->discoverMercurialAncestry($repository, $commit);
+    }
+
+    return array_mergev($refs);
+  }
+
+
+  /**
+   * @task hg
+   */
+  private function discoverMercurialAncestry(
+    PhabricatorRepository $repository,
+    $commit) {
+
+    $discover = array($commit);
+    $graph = array();
+    $seen = array();
+
+    $stream = new PhabricatorMercurialGraphStream($repository);
+
+    // Find all the reachable, undiscovered commits. Build a graph of the
+    // edges.
+    while ($discover) {
+      $target = array_pop($discover);
+
+      if (empty($graph[$target])) {
+        $graph[$target] = array();
+      }
+
+      $parents = $stream->getParents($target);
+      foreach ($parents as $parent) {
+        if ($this->isKnownCommit($parent)) {
+          continue;
+        }
+
+        $graph[$target][$parent] = true;
+
+        if (empty($seen[$parent])) {
+          $seen[$parent] = true;
+          $discover[] = $parent;
+        }
+      }
+    }
+
+    // Now, sort them topographically.
+    $commits = $this->reduceGraph($graph);
+
+    $refs = array();
+    foreach ($commits as $commit) {
+      $refs[] = id(new PhabricatorRepositoryCommitRef())
+        ->setIdentifier($commit)
+        ->setEpoch($stream->getCommitDate($commit));
+    }
+
+    return $refs;
+  }
+
+
 /* -(  Internals  )---------------------------------------------------------- */
 
 
+  private function reduceGraph(array $edges) {
+    foreach ($edges as $commit => $parents) {
+      $edges[$commit] = array_keys($parents);
+    }
+
+    $graph = new PhutilDirectedScalarGraph();
+    $graph->addNodes($edges);
+
+    $commits = $graph->getTopographicallySortedNodes();
+
+    // NOTE: We want the most ancestral nodes first, so we need to reverse the
+    // list we get out of AbstractDirectedGraph.
+    $commits = array_reverse($commits);
+
+    return $commits;
+  }
+
+
   private function isKnownCommit($identifier) {
     if (isset($this->commitCache[$identifier])) {
       return true;