Verify remotes ("paths") in Mercurial during "arc land"

Summary: Ref T13546. Parse "hg paths" and validate that the remotes "arc land" plans to interact with actually exist.

Test Plan: Ran "arc land" with good and bad "--into-remote" and "--onto-remote" arguments, got sensible validation behavior.

Maniphest Tasks: T13546

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

src/__phutil_library_map__.php

@@ -333,6 +333,7 @@ phutil_register_library_map(array(
     'ArcanistMercurialParser' => 'repository/parser/ArcanistMercurialParser.php',
     'ArcanistMercurialParserTestCase' => 'repository/parser/__tests__/ArcanistMercurialParserTestCase.php',
     'ArcanistMercurialRepositoryMarkerQuery' => 'repository/marker/ArcanistMercurialRepositoryMarkerQuery.php',
+    'ArcanistMercurialRepositoryRemoteQuery' => 'repository/remote/ArcanistMercurialRepositoryRemoteQuery.php',
     'ArcanistMercurialWorkEngine' => 'work/ArcanistMercurialWorkEngine.php',
     'ArcanistMercurialWorkingCopy' => 'workingcopy/ArcanistMercurialWorkingCopy.php',
     'ArcanistMercurialWorkingCopyRevisionHardpointQuery' => 'query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php',
@@ -414,12 +415,15 @@ phutil_register_library_map(array(
     'ArcanistRaggedClassTreeEdgeXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistRaggedClassTreeEdgeXHPASTLinterRuleTestCase.php',
     'ArcanistRef' => 'ref/ArcanistRef.php',
     'ArcanistRefInspector' => 'inspector/ArcanistRefInspector.php',
+    'ArcanistRemoteRef' => 'repository/remote/ArcanistRemoteRef.php',
     'ArcanistRepositoryAPI' => 'repository/api/ArcanistRepositoryAPI.php',
     'ArcanistRepositoryAPIMiscTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIMiscTestCase.php',
     'ArcanistRepositoryAPIStateTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php',
     'ArcanistRepositoryLocalState' => 'repository/state/ArcanistRepositoryLocalState.php',
     'ArcanistRepositoryMarkerQuery' => 'repository/marker/ArcanistRepositoryMarkerQuery.php',
+    'ArcanistRepositoryQuery' => 'repository/query/ArcanistRepositoryQuery.php',
     'ArcanistRepositoryRef' => 'ref/ArcanistRepositoryRef.php',
+    'ArcanistRepositoryRemoteQuery' => 'repository/remote/ArcanistRepositoryRemoteQuery.php',
     'ArcanistReusedAsIteratorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistReusedAsIteratorXHPASTLinterRule.php',
     'ArcanistReusedAsIteratorXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistReusedAsIteratorXHPASTLinterRuleTestCase.php',
     'ArcanistReusedIteratorReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistReusedIteratorReferenceXHPASTLinterRule.php',
@@ -1365,6 +1369,7 @@ phutil_register_library_map(array(
     'ArcanistMercurialParser' => 'Phobject',
     'ArcanistMercurialParserTestCase' => 'PhutilTestCase',
     'ArcanistMercurialRepositoryMarkerQuery' => 'ArcanistRepositoryMarkerQuery',
+    'ArcanistMercurialRepositoryRemoteQuery' => 'ArcanistRepositoryRemoteQuery',
     'ArcanistMercurialWorkEngine' => 'ArcanistWorkEngine',
     'ArcanistMercurialWorkingCopy' => 'ArcanistWorkingCopy',
     'ArcanistMercurialWorkingCopyRevisionHardpointQuery' => 'ArcanistWorkflowMercurialHardpointQuery',
@@ -1449,12 +1454,15 @@ phutil_register_library_map(array(
     'ArcanistRaggedClassTreeEdgeXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistRef' => 'ArcanistHardpointObject',
     'ArcanistRefInspector' => 'Phobject',
+    'ArcanistRemoteRef' => 'ArcanistRef',
     'ArcanistRepositoryAPI' => 'Phobject',
     'ArcanistRepositoryAPIMiscTestCase' => 'PhutilTestCase',
     'ArcanistRepositoryAPIStateTestCase' => 'PhutilTestCase',
     'ArcanistRepositoryLocalState' => 'Phobject',
-    'ArcanistRepositoryMarkerQuery' => 'Phobject',
+    'ArcanistRepositoryMarkerQuery' => 'ArcanistRepositoryQuery',
+    'ArcanistRepositoryQuery' => 'Phobject',
     'ArcanistRepositoryRef' => 'ArcanistRef',
+    'ArcanistRepositoryRemoteQuery' => 'ArcanistRepositoryQuery',
     'ArcanistReusedAsIteratorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistReusedAsIteratorXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistReusedIteratorReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',

src/land/engine/ArcanistMercurialLandEngine.php

@@ -48,6 +48,7 @@ final class ArcanistMercurialLandEngine
     }
 
     $commit = $api->getCanonicalRevisionName('.');
+    $commit = $this->getDisplayHash($commit);
 
     $log->writeStatus(
       pht('SOURCE'),
@@ -124,9 +125,21 @@ final class ArcanistMercurialLandEngine
 
   protected function selectOntoRemote(array $symbols) {
     assert_instances_of($symbols, 'ArcanistLandSymbol');
+    $api = $this->getRepositoryAPI();
+
     $remote = $this->newOntoRemote($symbols);
 
-    // TODO: Verify this remote actually exists.
+    $remote_ref = $api->newRemoteRefQuery()
+      ->withNames(array($remote))
+      ->executeOne();
+    if (!$remote_ref) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'No remote "%s" exists in this repository.',
+          $remote));
+    }
+
+    // TODO: Allow selection of a bare URI.
 
     return $remote;
   }
@@ -261,8 +274,17 @@ final class ArcanistMercurialLandEngine
     $into = $this->getIntoRemoteArgument();
     if ($into !== null) {
 
-      // TODO: Verify that this is a valid path.
+      $remote_ref = $api->newRemoteRefQuery()
-      // TODO: Allow a raw URI?
+        ->withNames(array($into))
+        ->executeOne();
+      if (!$remote_ref) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'No remote "%s" exists in this repository.',
+            $into));
+      }
+
+      // TODO: Allow a raw URI.
 
       $this->setIntoRemote($into);
 

src/repository/api/ArcanistMercurialAPI.php

@@ -902,6 +902,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function getRemoteURI() {
+    // TODO: Remove this method in favor of RemoteRefQuery.
+
     list($stdout) = $this->execxLocal('paths default');
 
     $stdout = trim($stdout);
@@ -1006,4 +1008,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     return new ArcanistMercurialRepositoryMarkerQuery();
   }
 
+  protected function newRemoteRefQueryTemplate() {
+    return new ArcanistMercurialRepositoryRemoteQuery();
+  }
+
 }

src/repository/api/ArcanistRepositoryAPI.php

@@ -785,6 +785,15 @@ abstract class ArcanistRepositoryAPI extends Phobject {
     throw new PhutilMethodNotImplementedException();
   }
 
+  final public function newRemoteRefQuery() {
+    return id($this->newRemoteRefQueryTemplate())
+      ->setRepositoryAPI($this);
+  }
+
+  protected function newRemoteRefQueryTemplate() {
+    throw new PhutilMethodNotImplementedException();
+  }
+
   final public function getDisplayHash($hash) {
     return substr($hash, 0, 12);
   }

src/repository/marker/ArcanistRepositoryMarkerQuery.php

@@ -1,24 +1,14 @@
 <?php
 
 abstract class ArcanistRepositoryMarkerQuery
-  extends Phobject {
+  extends ArcanistRepositoryQuery {
 
-  private $repositoryAPI;
   private $isActive;
   private $markerTypes;
   private $names;
   private $commitHashes;
   private $ancestorCommitHashes;
 
-  final public function setRepositoryAPI(ArcanistRepositoryAPI $api) {
-    $this->repositoryAPI = $api;
-    return $this;
-  }
-
-  final public function getRepositoryAPI() {
-    return $this->repositoryAPI;
-  }
-
   final public function withMarkerTypes(array $types) {
     $this->markerTypes = array_fuse($types);
     return $this;
@@ -34,22 +24,6 @@ abstract class ArcanistRepositoryMarkerQuery
     return $this;
   }
 
-  final public function executeOne() {
-    $markers = $this->execute();
-
-    if (!$markers) {
-      return null;
-    }
-
-    if (count($markers) > 1) {
-      throw new Exception(
-        pht(
-          'Query matched multiple markers, expected zero or one.'));
-    }
-
-    return head($markers);
-  }
-
   final public function execute() {
     $markers = $this->newRefMarkers();
 

src/repository/query/ArcanistRepositoryQuery.php

@@ -0,0 +1,35 @@
+<?php
+
+abstract class ArcanistRepositoryQuery
+  extends Phobject {
+
+  private $repositoryAPI;
+
+  final public function setRepositoryAPI(ArcanistRepositoryAPI $api) {
+    $this->repositoryAPI = $api;
+    return $this;
+  }
+
+  final public function getRepositoryAPI() {
+    return $this->repositoryAPI;
+  }
+
+  abstract public function execute();
+
+  final public function executeOne() {
+    $refs = $this->execute();
+
+    if (!$refs) {
+      return null;
+    }
+
+    if (count($refs) > 1) {
+      throw new Exception(
+        pht(
+          'Query matched multiple refs, expected zero or one.'));
+    }
+
+    return head($refs);
+  }
+
+}

src/repository/remote/ArcanistMercurialRepositoryRemoteQuery.php

@@ -0,0 +1,45 @@
+<?php
+
+final class ArcanistMercurialRepositoryRemoteQuery
+  extends ArcanistRepositoryRemoteQuery {
+
+  protected function newRemoteRefs() {
+    $api = $this->getRepositoryAPI();
+
+    $future = $api->newFuture('paths');
+    list($lines) = $future->resolve();
+
+    $refs = array();
+
+    $pattern = '(^(?P<name>.*?) = (?P<uri>.*)\z)';
+
+    $lines = phutil_split_lines($lines, false);
+    foreach ($lines as $line) {
+      $matches = null;
+      if (!preg_match($pattern, $line, $matches)) {
+        throw new Exception(
+          pht(
+            'Failed to match remote pattern against line "%s".',
+            $line));
+      }
+
+      $name = $matches['name'];
+      $uri = $matches['uri'];
+
+      // NOTE: Mercurial gives some special behavior to "default" and
+      // "default-push", but these remotes are both fully-formed remotes that
+      // are fetchable and pushable, they just have rules around selection
+      // as default targets for operations.
+
+      $ref = id(new ArcanistRemoteRef())
+        ->setRemoteName($name)
+        ->setFetchURI($uri)
+        ->setPushURI($uri);
+
+      $refs[] = $ref;
+    }
+
+    return $refs;
+  }
+
+}

src/repository/remote/ArcanistRemoteRef.php

@@ -0,0 +1,41 @@
+<?php
+
+final class ArcanistRemoteRef
+  extends ArcanistRef {
+
+  private $remoteName;
+  private $fetchURI;
+  private $pushURI;
+
+  public function getRefDisplayName() {
+    return pht('Remote "%s"', $this->getRemoteName());
+  }
+
+  public function setRemoteName($remote_name) {
+    $this->remoteName = $remote_name;
+    return $this;
+  }
+
+  public function getRemoteName() {
+    return $this->remoteName;
+  }
+
+  public function setFetchURI($fetch_uri) {
+    $this->fetchURI = $fetch_uri;
+    return $this;
+  }
+
+  public function getFetchURI() {
+    return $this->fetchURI;
+  }
+
+  public function setPushURI($push_uri) {
+    $this->pushURI = $push_uri;
+    return $this;
+  }
+
+  public function getPushURI() {
+    return $this->pushURI;
+  }
+
+}

src/repository/remote/ArcanistRepositoryRemoteQuery.php

@@ -0,0 +1,31 @@
+<?php
+
+abstract class ArcanistRepositoryRemoteQuery
+  extends ArcanistRepositoryQuery {
+
+  private $names;
+
+  final public function withNames(array $names) {
+    $this->names = $names;
+    return $this;
+  }
+
+  final public function execute() {
+    $refs = $this->newRemoteRefs();
+
+    $names = $this->names;
+    if ($names !== null) {
+      $names = array_fuse($names);
+      foreach ($refs as $key => $ref) {
+        if (!isset($names[$ref->getRemoteName()])) {
+          unset($refs[$key]);
+        }
+      }
+    }
+
+    return $refs;
+  }
+
+  abstract protected function newRemoteRefs();
+
+}

src/workflow/ArcanistWorkflow.php

@@ -2036,6 +2036,8 @@ abstract class ArcanistWorkflow extends Phobject {
         'This repository has no VCS UUID (this is normal for git/hg).');
     }
 
+    // TODO: Swap this for a RemoteRefQuery.
+
     $remote_uri = $this->getRepositoryAPI()->getRemoteURI();
     if ($remote_uri !== null) {
       $query = array(