Use "arc which" in "arc commit", modernize checks

Summary:
  - Use differential.query, not differential.find.
  - Use loadWorkingCopyDifferentialRevisions ("arc which").
  - Some general cleanup.

Test Plan:
oh man

  $ arc commit --revision 999 # Does not exist
  Usage Exception: Revision 'D999' does not exist.

  $ arc commit --revision 1 # Exists, not accepted.

      Revision 'D1: bleorp' has not been accepted. Commit this revision anyway?
      [y/N] y

  Committing 'D1: bleorp'...
  A locally modified path is not included in this revision:

      DERP

      It will NOT be committed. Commit this revision anyway? [y/N] y

  Done.

  $ arc commit --revision 3 # Not mine, from a git repo

      You are not the author of 'D3: bloop'. Commit this revision anyway? [y/N]
y

      Revision 'D3: bloop' was generated from
      '/INSECURE/repos/git-working-copy/', but current working copy root is
      '/INSECURE/repos/svn-working-copy/'. Commit this revision anyway? [y/N] y

  Committing 'D3: bloop'...
  A locally modified path is not included in this revision:

      DERP

      It will NOT be committed. Commit this revision anyway? [y/N] y

  svn: Commit failed (details follow):
  svn: '/INSECURE/repos/svn-working-copy/derp' is not under version control

  Exception:
  Executing 'svn commit' failed!
  (Run with --trace for a full exception trace.)

  $ arc commit # Nothing accepted
  Usage Exception: Unable to identify the revision in the working copy. Use
'--revision <revision_id>' to select a revision.

  $ arc commit # Now accepted
  Committing 'D1: bleorp'...
  Marking revision D1 'bleorp' committed...
  Done.

  $ svn st # Complicated test for a bizarre SVN edge case
  A       svnderp
  A       svnderp/A
  A       svnderp/B
  $ arc diff --create
  Linting...
   LINT OKAY  No lint problems.
  Running unit tests...
  No unit test engine is configured for this project.
  Created a new Differential revision:
          Revision URI: http://local.aphront.com/D28

  Included changes:
    A (dir) svnderp
    A       svnderp/A
    A       svnderp/B
  $ touch svnderp/C
  $ svn add svnderp/C
  A         svnderp/C
  $ arc commit
  Usage Exception: Unable to identify the revision in the working copy. Use
'--revision <revision_id>' to select a revision.
  $ arc commit --revision 28

      Revision 'D28: derp' has not been accepted. Commit this revision anyway?
      [y/N] y

  Committing 'D28: derp'...
  Usage Exception: This commit includes the directory 'svnderp', but it contains
a modified path ('svnderp/C') which is NOT included in the commit. Subversion
can not handle this operation and will commit the path anyway. You need to sort
out the working copy changes to 'svnderp/C' before you may proceed with the
commit.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

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

src/workflow/commit/ArcanistCommitWorkflow.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -72,69 +72,53 @@ EOTEXT
 
   public function run() {
     $repository_api = $this->getRepositoryAPI();
-    $conduit = $this->getConduit();
+
+    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) {
-      $revision_data = $conduit->callMethodSynchronous(
+      $revisions = $repository_api->loadWorkingCopyDifferentialRevisions(
-        'differential.find',
+        $this->getConduit(),
         array(
-          'query' => 'committable',
+          'authors' => array($this->getUserPHID()),
-          'guids' => array(
+          'status'  => 'status-accepted',
-            $this->getUserPHID(),
+        ));
-          ),
-        )
-      );
 
-      try {
+      if (count($revisions) == 0) {
-        $revision = $this->chooseRevision(
-          $revision_data,
-          null,
-          'Which revision do you want to commit?'
-        );
-        $revision_id = $revision->getID();
-      } catch (ArcanistChooseNoRevisionsException $ex) {
         throw new ArcanistUsageException(
-          "You have no committable Differential revisions. You can only ".
+          "Unable to identify the revision in the working copy. Use ".
-          "commit revisions which have been 'accepted'.");
+          "'--revision <revision_id>' to select a revision.");
+      } else if (count($revisions) > 1) {
+        throw new ArcanistUsageException(
+          "More than one revision exists in the working copy:\n\n".
+          $this->renderRevisionList($revisions)."\n".
+          "Use '--revision <revision_id>' to select a revision.");
       }
-    }
 
-    $this->revisionID = $revision_id;
+    } else {
-
+      $revisions = $this->getConduit()->callMethodSynchronous(
-    $revision = null;
+        'differential.query',
-    try {
-      $revision = $conduit->callMethodSynchronous(
-        'differential.getrevision',
         array(
-          'revision_id' => $revision_id,
+          'ids' => array($revision_id),
-        )
+        ));
-      );
-    } catch (Exception $ex) {
-      throw new ArcanistUsageException(
-        "Revision D{$revision_id} does not exist."
-      );
-    }
 
-    if ($revision['statusName'] != 'Accepted') {
+      if (count($revisions) == 0) {
-      throw new ArcanistUsageException(
+        throw new ArcanistUsageException(
-        "Revision D{$revision_id} is not committable. You can only commit ".
+          "Revision 'D{$revision_id}' does not exist.");
-        "revisions which have been 'accepted'."
-      );
-    }
-
-    if ($revision['authorPHID'] != $this->getUserPHID()) {
-      $prompt = "You are not the author of revision D{$revision_id}, ".
-        'are you sure you want to commit it?';
-      if (!phutil_console_confirm($prompt)) {
-        throw new ArcanistUserAbortException();
       }
     }
 
-    $revision_name  = $revision['title'];
+    $revision = head($revisions);
+    $this->revisionID = $revision['id'];
+    $revision_id = $revision['id'];
 
-    $message = $conduit->callMethodSynchronous(
+    $this->runSanityChecks($revision);
+
+    $message = $this->getConduit()->callMethodSynchronous(
       'differential.getcommitmessage',
       array(
         'revision_id' => $revision_id,
@@ -144,8 +128,8 @@ EOTEXT
     $event = new PhutilEvent(
       ArcanistEventType::TYPE_COMMIT_WILLCOMMITSVN,
       array(
-        'message' => $message,
+        'message'   => $message,
-        'workflow' => $this
+        'workflow'  => $this,
       )
     );
     PhutilEventEngine::dispatchEvent($event);
@@ -157,7 +141,8 @@ EOTEXT
       return 0;
     }
 
-    echo "Committing D{$revision_id} '{$revision_name}'...\n";
+    $revision_title = $revision['title'];
+    echo "Committing 'D{$revision_id}: {$revision_title}'...\n";
 
     $files = $this->getCommitFileList($revision);
 
@@ -169,8 +154,7 @@ EOTEXT
 
     // Specify LANG explicitly so that UTF-8 commit messages don't break
     // subversion.
-    $command =
+    $command = "(cd {$root} && LANG={$lang} svn commit {$files} -m {$message})";
-      "(cd {$root} && LANG={$lang} svn commit {$files} -m {$message})";
 
     $err = phutil_passthru('%C', $command);
 
@@ -191,41 +175,9 @@ EOTEXT
 
   protected function getCommitFileList(array $revision) {
     $repository_api = $this->getRepositoryAPI();
-
-    if (!($repository_api instanceof ArcanistSubversionAPI)) {
-      throw new ArcanistUsageException(
-        "arc commit is only supported under SVN. Use arc amend under git.");
-    }
-
-    $conduit = $this->getConduit();
-
     $revision_id = $revision['id'];
 
-    $revision = reset($conduit->callMethodSynchronous(
+    $commit_paths = $this->getConduit()->callMethodSynchronous(
-      'differential.find',
-      array(
-        'query' => 'revision-ids',
-        'guids' => array($revision_id,)
-      )
-    ));
-    if (!$revision) {
-      throw new ArcanistUsageException(
-        "Revision D{$revision_id} does not exist."
-      );
-    }
-    $revision_source = $revision['sourcePath'];
-
-    $working_copy = $repository_api->getPath();
-    if ($revision_source != $working_copy) {
-      $prompt =
-        "Revision was generated from '{$revision_source}', but the current ".
-        "working copy root is '{$working_copy}'. Commit anyway?";
-      if (!phutil_console_confirm($prompt)) {
-        throw new ArcanistUserAbortException();
-      }
-    }
-
-    $commit_paths = $conduit->callMethodSynchronous(
       'differential.getcommitpaths',
       array(
         'revision_id' => $revision_id,
@@ -351,4 +303,40 @@ EOTEXT
     return $locale;
   }
 
+  private function runSanityChecks(array $revision) {
+    $repository_api = $this->getRepositoryAPI();
+    $revision_id = $revision['id'];
+    $revision_title = $revision['title'];
+
+    $confirm = array();
+
+    if ($revision['status'] != ArcanistDifferentialRevisionStatus::ACCEPTED) {
+      $confirm[] =
+        "Revision 'D{$revision_id}: {$revision_title}' has not been accepted. ".
+        "Commit this revision anyway?";
+    }
+
+    if ($revision['authorPHID'] != $this->getUserPHID()) {
+      $confirm[] =
+        "You are not the author of 'D{$revision_id}: {$revision_title}'. ".
+        "Commit this revision anyway?";
+    }
+
+    $revision_source = $revision['sourcePath'];
+    $current_source = $repository_api->getPath();
+    if ($revision_source != $current_source) {
+      $confirm[] =
+        "Revision 'D{$revision_id}: {$revision_title}' was generated from ".
+        "'{$revision_source}', but current working copy root is ".
+        "'{$current_source}'. Commit this revision anyway?";
+    }
+
+    foreach ($confirm as $thing) {
+      if (!phutil_console_confirm($thing)) {
+        throw new ArcanistUserAbortException();
+      }
+    }
+  }
+
+
 }

src/workflow/commit/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('arcanist', 'differential/constants/revisionstatus');
 phutil_require_module('arcanist', 'events/constant/type');
 phutil_require_module('arcanist', 'exception/usage');
 phutil_require_module('arcanist', 'exception/usage/userabort');