From e4be03177810b61d43d5f7f2db38cb578b82cfb2 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 3 Feb 2015 06:54:22 +1100
Subject: [PATCH] Explicitly check for supported VCS

Summary: Instead of having an `ArcanistWorkflow` subclass explicitly throw an exception when run in an unsupported VCS, consolidate this code and move it to `arcanist.php`. In doing so, we lose some specificity in some of the error messages, but this otherwise feels cleaner. We could consider adding a `getUnsupportedRevisionControlSystemMessage()` method to provide a more tailored error message. Depends on D11604.

Test Plan:
Ran `arc bookmark` in a `git` working copy:

```
Usage Exception: `arc bookmark` is only supported under hg.
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11550
---
 scripts/arcanist.php                      | 10 ++++++++++
 src/workflow/ArcanistAmendWorkflow.php    |  2 +-
 src/workflow/ArcanistBookmarkWorkflow.php |  9 ++++-----
 src/workflow/ArcanistCommitWorkflow.php   |  8 +-------
 src/workflow/ArcanistLandWorkflow.php     |  9 +--------
 src/workflow/ArcanistRevertWorkflow.php   |  2 +-
 src/workflow/ArcanistWorkflow.php         |  2 +-
 7 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/scripts/arcanist.php b/scripts/arcanist.php
index dc22d7b6..737a01a8 100755
--- a/scripts/arcanist.php
+++ b/scripts/arcanist.php
@@ -202,6 +202,16 @@ try {
     $workflow->setConduitTimeout($conduit_timeout);
   }
 
+  $supported_vcs_types = $workflow->getSupportedRevisionControlSystems();
+  if (!in_array($working_copy->getVCSType(), $supported_vcs_types)) {
+    throw new ArcanistUsageException(
+      pht(
+        '`%s %s` is only supported under %s.',
+        'arc',
+        $workflow->getWorkflowName(),
+        implode(', ', $supported_vcs_types)));
+  }
+
   $need_working_copy    = $workflow->requiresWorkingCopy();
   $need_conduit         = $workflow->requiresConduit();
   $need_auth            = $workflow->requiresAuthentication();
diff --git a/src/workflow/ArcanistAmendWorkflow.php b/src/workflow/ArcanistAmendWorkflow.php
index d9be2865..334ff9d2 100644
--- a/src/workflow/ArcanistAmendWorkflow.php
+++ b/src/workflow/ArcanistAmendWorkflow.php
@@ -188,7 +188,7 @@ EOTEXT
     return 0;
   }
 
-  protected function getSupportedRevisionControlSystems() {
+  public function getSupportedRevisionControlSystems() {
     return array('git', 'hg');
   }
 
diff --git a/src/workflow/ArcanistBookmarkWorkflow.php b/src/workflow/ArcanistBookmarkWorkflow.php
index 16c71acc..131c21b3 100644
--- a/src/workflow/ArcanistBookmarkWorkflow.php
+++ b/src/workflow/ArcanistBookmarkWorkflow.php
@@ -25,12 +25,11 @@ EOTEXT
       );
   }
 
+  public function getSupportedRevisionControlSystems() {
+    return array('hg');
+  }
+
   public function run() {
-    $repository_api = $this->getRepositoryAPI();
-    if (!($repository_api instanceof ArcanistMercurialAPI)) {
-      throw new ArcanistUsageException(
-        'arc bookmark is only supported under Mercurial.');
-    }
     return parent::run();
   }
 
diff --git a/src/workflow/ArcanistCommitWorkflow.php b/src/workflow/ArcanistCommitWorkflow.php
index dbd2b791..6ac44102 100644
--- a/src/workflow/ArcanistCommitWorkflow.php
+++ b/src/workflow/ArcanistCommitWorkflow.php
@@ -65,12 +65,6 @@ EOTEXT
   public function run() {
     $repository_api = $this->getRepositoryAPI();
 
-    if (!($repository_api instanceof ArcanistSubversionAPI)) {
-      throw new ArcanistUsageException(
-        "'arc commit' is only supported under svn.");
-    }
-
-
     $revision_id = $this->normalizeRevisionID($this->getArgument('revision'));
     if (!$revision_id) {
       $revisions = $repository_api->loadWorkingCopyDifferentialRevisions(
@@ -271,7 +265,7 @@ EOTEXT
     }
   }
 
-  protected function getSupportedRevisionControlSystems() {
+  public function getSupportedRevisionControlSystems() {
     return array('svn');
   }
 
diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php
index d18af183..69c498ec 100644
--- a/src/workflow/ArcanistLandWorkflow.php
+++ b/src/workflow/ArcanistLandWorkflow.php
@@ -239,13 +239,6 @@ EOTEXT
     $this->isGit = $repository_api instanceof ArcanistGitAPI;
     $this->isHg = $repository_api instanceof ArcanistMercurialAPI;
 
-    if (!$this->isGit && !$this->isHg) {
-      throw new ArcanistUsageException(
-        pht(
-          "'arc land' only supports Git and Mercurial. For Subversion, try ".
-          "'arc commit'."));
-    }
-
     if ($this->isGit) {
       $repository = $this->loadProjectRepository();
       $this->isGitSvn = (idx($repository, 'vcs') == 'svn');
@@ -1141,7 +1134,7 @@ EOTEXT
     }
   }
 
-  protected function getSupportedRevisionControlSystems() {
+  public function getSupportedRevisionControlSystems() {
     return array('git', 'hg');
   }
 
diff --git a/src/workflow/ArcanistRevertWorkflow.php b/src/workflow/ArcanistRevertWorkflow.php
index 823a2802..205c91d3 100644
--- a/src/workflow/ArcanistRevertWorkflow.php
+++ b/src/workflow/ArcanistRevertWorkflow.php
@@ -29,7 +29,7 @@ EOTEXT
     );
   }
 
-  protected function getSupportedRevisionControlSystems() {
+  public function getSupportedRevisionControlSystems() {
     return array('git', 'hg');
   }
 
diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php
index 56fad6dc..7c571986 100644
--- a/src/workflow/ArcanistWorkflow.php
+++ b/src/workflow/ArcanistWorkflow.php
@@ -1215,7 +1215,7 @@ abstract class ArcanistWorkflow extends Phobject {
     return array();
   }
 
-  protected function getSupportedRevisionControlSystems() {
+  public function getSupportedRevisionControlSystems() {
     return array('git', 'hg', 'svn');
   }