Add "nocommit" git-only flag to arc patch workflow

Summary:
without "nocommit" we commit the patch to the working copy. add the nocommit
flag and this commit does not happen.

making this happen required adding revisionID to arc bundle to fetch the proper
commit message.  if we can't get a commit message -- suppose the fetch fails or
the source is self::SOURCE_PATCH, we ask the user for the commit message on the
command line

Test Plan:
git reset --hard <SOME_REV>
arc patch DX, where this got us to <SOME_REV + 1>
observe correct patch committed with correct commit message in working copy

git reset --hard <SOME_REV>
arc patch --nocommit DX, where this got us to <SOME_REV + 1>
observe correct patch landed BUT NOT committed.  note "commit message" does not
exist since there isn't a commit...!

git diff HEAD^1 > ~/file.patch
git reset --hard HEAD^1
arc patch --patch ~/file.patch
observe prompted for commit message and patch committed with commit message i
typed in

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T479

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

src/parser/bundle/ArcanistBundle.php

@@ -29,6 +29,7 @@ class ArcanistBundle {
   private $diskPath;
   private $projectID;
   private $baseRevision;
+  private $revisionID;
 
   public function setConduit(ConduitClient $conduit) {
     $this->conduit = $conduit;
@@ -50,6 +51,15 @@ class ArcanistBundle {
     return $this->baseRevision;
   }
 
+  public function setRevisionID($revision_id) {
+    $this->revisionID = $revision_id;
+    return $this;
+  }
+
+  public function getRevisionID() {
+    return $this->revisionID;
+  }
+
   public static function newFromChanges(array $changes) {
     $obj = new ArcanistBundle();
     $obj->changes = $changes;
@@ -75,11 +85,13 @@ class ArcanistBundle {
       $version       = idx($meta_info, 'version', 0);
       $project_name  = idx($meta_info, 'projectName');
       $base_revision = idx($meta_info, 'baseRevision');
+      $revision_id   = idx($meta_info, 'revisionID');
     // this arc bundle was probably made before we started storing meta info
     } else {
       $version       = 0;
       $project_name  = null;
       $base_revision = null;
+      $revision_id   = null;
     }
 
     $future = new ExecFuture(
@@ -104,6 +116,7 @@ class ArcanistBundle {
     $obj->diskPath = $path;
     $obj->setProjectID($project_name);
     $obj->setBaseRevision($base_revision);
+    $obj->setRevisionID($revision_id);
 
     return $obj;
   }
@@ -151,9 +164,10 @@ class ArcanistBundle {
     }
 
     $meta_info = array(
-      'version'      => 2,
+      'version'      => 3,
       'projectName'  => $this->getProjectID(),
       'baseRevision' => $this->getBaseRevision(),
+      'revisionID'   => $this->getRevisionID(),
     );
 
     $dir = Filesystem::createTemporaryDirectory();

src/workflow/base/ArcanistBaseWorkflow.php

@@ -753,6 +753,7 @@ class ArcanistBaseWorkflow {
     $bundle->setConduit($conduit);
     $bundle->setProjectID($diff['projectName']);
     $bundle->setBaseRevision($diff['sourceControlBaseRevision']);
+    $bundle->setRevisionID($diff['revisionID']);
     return $bundle;
   }
 

src/workflow/export/ArcanistExportWorkflow.php

@@ -189,6 +189,7 @@ EOTEXT
         $bundle->setProjectID($this->getWorkingCopy()->getProjectID());
         $bundle->setBaseRevision(
           $repository_api->getSourceControlBaseRevision());
+        // note we can't get a revision ID for SOURCE_LOCAL
         break;
       case self::SOURCE_REVISION:
         $bundle = $this->loadRevisionBundleFromConduit(

src/workflow/patch/ArcanistPatchWorkflow.php

@@ -38,7 +38,7 @@ final class ArcanistPatchWorkflow extends ArcanistBaseWorkflow {
       **patch** __--diff__ __diff_id__
       **patch** __--patch__ __file__
       **patch** __--arcbundle__ __bundlefile__
-          Supports: git, svn
+          Supports: git, svn, hg
           Apply the changes in a Differential revision, patchfile, or arc
           bundle to the working copy.
 EOTEXT
@@ -75,6 +75,14 @@ EOTEXT
         'help' =>
           "Apply changes from a git patchfile or unified patchfile.",
       ),
+      'nocommit' => array(
+        'supports' => array(
+          'git'
+        ),
+        'help' =>
+          "Normally under git if the patch is successful the changes are ".
+          "committed to the working copy. This flag prevents the commit.",
+      ),
       'force' => array(
         'help' =>
           "Do not run any sanity checks.",
@@ -154,6 +162,15 @@ EOTEXT
     return $this->sourceParam;
   }
 
+  private function shouldCommit() {
+    $no_commit = $this->getArgument('nocommit', false);
+    if ($no_commit) {
+      return false;
+    }
+
+    return true;
+  }
+
   public function run() {
 
     $source = $this->getSource();
@@ -404,14 +421,31 @@ EOTEXT
 
       return $patch_err;
     } else if ($repository_api instanceof ArcanistGitAPI) {
+      // if we're going to commit, we should make sure the working copy
+      // is clean
+      if ($this->shouldCommit()) {
+        $this->requireCleanWorkingCopy();
+      }
+
       $future = new ExecFuture(
         '(cd %s; git apply --index --reject)',
         $repository_api->getPath());
       $future->write($bundle->toGitPatch());
       $future->resolvex();
 
+      if ($this->shouldCommit()) {
+        $commit_message = $this->getCommitMessage($bundle);
+        $future = new ExecFuture(
+          '(cd %s; git commit -a -F -)',
+          $repository_api->getPath());
+        $future->write($commit_message);
+        $future->resolvex();
+        $verb = 'committed';
+      } else {
+        $verb = 'applied';
+      }
       echo phutil_console_format(
-        "<bg:green>** OKAY **</bg> Successfully applied patch.\n");
+        "<bg:green>** OKAY **</bg> Successfully {$verb} patch.\n");
     } else if ($repository_api instanceof ArcanistMercurialAPI) {
       $future = new ExecFuture(
         '(cd %s; hg import --no-commit -)',
@@ -428,6 +462,50 @@ EOTEXT
     return 0;
   }
 
+  private function getCommitMessage(ArcanistBundle $bundle) {
+    $revision_id    = $bundle->getRevisionID();
+    $commit_message = null;
+    $prompt_message = null;
+
+    // if we have a revision id the commit message is in differential
+    if ($revision_id) {
+      $conduit        = $this->getConduit();
+      $commit_message = $conduit->callMethodSynchronous(
+        'differential.getcommitmessage',
+        array(
+          'revision_id' => $revision_id,
+        ));
+      $prompt_message = "  Note arcanist failed to load the commit message ".
+                        "from differential for revision D{$revision_id}.";
+    }
+
+    // no revision id or failed to fetch commit message so get it from the
+    // user on the command line
+    if (!$commit_message) {
+      $template =
+        "\n\n".
+        "# Enter a commit message for this patch.  If you just want to apply ".
+        "the patch to the working copy without committing, re-run arc patch ".
+        "with the --nocommit flag.".
+        $prompt_message.
+        "\n";
+
+      $commit_message = id(new PhutilInteractiveEditor($template))
+        ->setName('arcanist-patch-commit-message')
+        ->editInteractively();
+
+      $commit_message = preg_replace('/^\s*#.*$/m',
+                                     '',
+                                     $commit_message);
+      $commit_message = rtrim($commit_message);
+      if (!strlen($commit_message)) {
+        throw new ArcanistUserAbortException();
+      }
+    }
+
+    return $commit_message;
+  }
+
   public function getShellCompletions(array $argv) {
     // TODO: Pull open diffs from 'arc list'?
     return array('ARGUMENT');

src/workflow/patch/__init__.php

@@ -15,6 +15,7 @@ phutil_require_module('arcanist', 'parser/diff/changetype');
 phutil_require_module('arcanist', 'workflow/base');
 
 phutil_require_module('phutil', 'console');
+phutil_require_module('phutil', 'console/editor');
 phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'filesystem/tempfile');
 phutil_require_module('phutil', 'future/exec');