Use MarkerRefs to resolve landing symbols in Mercurial

Summary: Ref T13546. Update the Mercurial code which finds default targets and maps symbols to targets under "arc land" to use the new MarkerRef workflow.

Test Plan: Ran "arc land" with (and without) various arguments in Mercurial, saw them resolve in a seemingly sensible way.

Maniphest Tasks: T13546

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

src/land/engine/ArcanistMercurialLandEngine.php

@@ -7,9 +7,19 @@ final class ArcanistMercurialLandEngine
     $api = $this->getRepositoryAPI();
     $log = $this->getLogEngine();
 
-    $bookmark = $api->getActiveBookmark();
+    $markers = $api->newMarkerRefQuery()
-    if ($bookmark !== null) {
+      ->withIsActive(true)
+      ->execute();
 
+    $bookmark = null;
+    foreach ($markers as $marker) {
+      if ($marker->isBookmark()) {
+        $bookmark = $marker->getName();
+        break;
+      }
+    }
+
+    if ($bookmark !== null) {
       $log->writeStatus(
         pht('SOURCE'),
         pht(
@@ -19,50 +29,96 @@ final class ArcanistMercurialLandEngine
       return array($bookmark);
     }
 
-    $branch = $api->getBranchName();
+    $branch = null;
-    if ($branch !== null) {
+    foreach ($markers as $marker) {
+      if ($marker->isBranch()) {
+        $branch = $marker->getName();
+        break;
+      }
+    }
 
+    if ($branch !== null) {
       $log->writeStatus(
         pht('SOURCE'),
         pht(
-          'Landing the current branch, "%s".',
+          'Landing the active branch, "%s".',
           $branch));
 
       return array($branch);
     }
 
-    throw new Exception(pht('TODO: Operate on raw revision.'));
+    $commit = $api->getCanonicalRevisionName('.');
+
+    $log->writeStatus(
+      pht('SOURCE'),
+      pht(
+        'Landing the active commit, "%s".',
+        $this->getDisplayHash($commit)));
+
+    return array($commit);
   }
 
   protected function resolveSymbols(array $symbols) {
     assert_instances_of($symbols, 'ArcanistLandSymbol');
     $api = $this->getRepositoryAPI();
 
-    foreach ($symbols as $symbol) {
+    $marker_types = array(
+      ArcanistMarkerRef::TYPE_BOOKMARK,
+      ArcanistMarkerRef::TYPE_BRANCH,
+    );
+
+    $unresolved = $symbols;
+    foreach ($marker_types as $marker_type) {
+      $markers = $api->newMarkerRefQuery()
+        ->withMarkerTypes(array($marker_type))
+        ->execute();
+
+      $markers = mgroup($markers, 'getName');
+
+      foreach ($unresolved as $key =>  $symbol) {
+        $raw_symbol = $symbol->getSymbol();
+
+        $named_markers = idx($markers, $raw_symbol);
+        if (!$named_markers) {
+          continue;
+        }
+
+        if (count($named_markers) > 1) {
+          throw new PhutilArgumentUsageException(
+            pht(
+              'Symbol "%s" is ambiguous: it matches multiple markers '.
+              '(of type "%s"). Use an unambiguous identifier.',
+              $raw_symbol,
+              $marker_type));
+        }
+
+        $marker = head($named_markers);
+
+        $symbol->setCommit($marker->getCommitHash());
+
+        unset($unresolved[$key]);
+      }
+    }
+
+    foreach ($unresolved as $symbol) {
       $raw_symbol = $symbol->getSymbol();
 
-      if ($api->isBookmark($raw_symbol)) {
+      // TODO: This doesn't have accurate error behavior if the user provides
-        $hash = $api->getBookmarkCommitHash($raw_symbol);
+      // a revset like "x::y".
-        $symbol->setCommit($hash);
+      try {
-
+        $commit = $api->getCanonicalRevisionName($raw_symbol);
-        // TODO: Set that this is a bookmark?
+      } catch (CommandException $ex) {
-
+        $commit = null;
-        continue;
       }
 
-      if ($api->isBranch($raw_symbol)) {
+      if ($commit === null) {
-        $hash = $api->getBranchCommitHash($raw_symbol);
+        throw new PhutilArgumentUsageException(
-        $symbol->setCommit($hash);
+          pht(
-
+            'Symbol "%s" does not identify a bookmark, branch, or commit.',
-        // TODO: Set that this is a branch?
+            $raw_symbol));
-
-        continue;
       }
 
-      throw new PhutilArgumentUsageException(
+      $symbol->setCommit($commit);
-        pht(
-          'Symbol "%s" is not a bookmark or branch name.',
-          $raw_symbol));
     }
   }
 

src/repository/api/ArcanistMercurialAPI.php

@@ -59,6 +59,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       'log -l 1 --template %s -r %s --',
       '{node}',
       $string);
+
     return $stdout;
   }
 

src/repository/marker/ArcanistMarkerRef.php

@@ -109,6 +109,14 @@ final class ArcanistMarkerRef
     return $this->isActive;
   }
 
+  public function isBookmark() {
+    return ($this->getMarkerType() === self::TYPE_BOOKMARK);
+  }
+
+  public function isBranch() {
+    return ($this->getMarkerType() === self::TYPE_BRANCH);
+  }
+
   public function attachCommitRef(ArcanistCommitRef $ref) {
     return $this->attachHardpoint(self::HARDPOINT_COMMITREF, $ref);
   }

src/repository/marker/ArcanistRepositoryMarkerQuery.php

@@ -4,7 +4,8 @@ abstract class ArcanistRepositoryMarkerQuery
   extends Phobject {
 
   private $repositoryAPI;
-  private $types;
+  private $isActive;
+  private $markerTypes;
   private $commitHashes;
   private $ancestorCommitHashes;
 
@@ -17,15 +18,20 @@ abstract class ArcanistRepositoryMarkerQuery
     return $this->repositoryAPI;
   }
 
-  final public function withTypes(array $types) {
+  final public function withMarkerTypes(array $types) {
-    $this->types = array_fuse($types);
+    $this->markerTypes = array_fuse($types);
+    return $this;
+  }
+
+  final public function withIsActive($active) {
+    $this->isActive = $active;
     return $this;
   }
 
   final public function execute() {
     $markers = $this->newRefMarkers();
 
-    $types = $this->types;
+    $types = $this->markerTypes;
     if ($types !== null) {
       foreach ($markers as $key => $marker) {
         if (!isset($types[$marker->getMarkerType()])) {
@@ -34,6 +40,14 @@ abstract class ArcanistRepositoryMarkerQuery
       }
     }
 
+    if ($this->isActive !== null) {
+      foreach ($markers as $key => $marker) {
+        if ($marker->getIsActive() !== $this->isActive) {
+          unset($markers[$key]);
+        }
+      }
+    }
+
     return $this->sortMarkers($markers);
   }
 
@@ -53,11 +67,11 @@ abstract class ArcanistRepositoryMarkerQuery
   }
 
   final protected function shouldQueryMarkerType($marker_type) {
-    if ($this->types === null) {
+    if ($this->markerTypes === null) {
       return true;
     }
 
-    return isset($this->types[$marker_type]);
+    return isset($this->markerTypes[$marker_type]);
   }
 
 }

src/workflow/ArcanistMarkersWorkflow.php

@@ -11,7 +11,7 @@ abstract class ArcanistMarkersWorkflow
     $marker_type = $this->getWorkflowMarkerType();
 
     $markers = $api->newMarkerRefQuery()
-      ->withTypes(array($marker_type))
+      ->withMarkerTypes(array($marker_type))
       ->execute();
 
     $states = array();