From 2d269aefedce5a9a17cbab167f66eaa94a35d88a Mon Sep 17 00:00:00 2001
From: Edward Speyer <edward@fb.com>
Date: Tue, 16 Oct 2012 16:46:21 -0700
Subject: [PATCH] Use git-cherry-pick for arc patch

Summary:
This changes what --nobranch does, though not what it means.  In git, the patch
is applied to the git commit it is based off where it will commit cleanly, and
if you specify --nobranch then arc-patch will then switch back to your original
branch and try to cherry-pick the commit there.

If this fails, you end up with merge-conflict markers in the file which can be
handy.

Test Plan: https://secure.phabricator.com/P572

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3712
---
 src/workflow/ArcanistPatchWorkflow.php | 52 ++++++++++++++++++++------
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php
index 4224c23c..d2c82063 100644
--- a/src/workflow/ArcanistPatchWorkflow.php
+++ b/src/workflow/ArcanistPatchWorkflow.php
@@ -111,9 +111,9 @@ EOTEXT
         ),
         'help' =>
           "Normally under git, a new branch is created and then the patch ".
-          "is applied and committed in the new branch. This flag skips the ".
-          "branch creation step and applies and commits the patch to the ".
-          "current branch.",
+          "is applied and committed in the new branch. This flag ".
+          "cherry-picks the resultant commit onto the original branch and ".
+          "deletes the temporary branch.",
         'conflicts' => array(
           'update' => true,
         ),
@@ -219,18 +219,20 @@ EOTEXT
     return true;
   }
 
-  private function shouldBranch() {
+  private function canBranch() {
     // git only for now
     $repository_api = $this->getRepositoryAPI();
     if (!($repository_api instanceof ArcanistGitAPI)) {
       return false;
     }
+    return true;
+  }
 
+  private function shouldBranch() {
     $no_branch = $this->getArgument('nobranch', false);
     if ($no_branch) {
       return false;
     }
-
     return true;
   }
 
@@ -328,10 +330,9 @@ EOTEXT
     return $bookmark_name;
   }
 
-  private function createBranch(ArcanistBundle $bundle) {
-    $branch_name    = $this->getBranchName($bundle);
+  private function hasBaseRevision(ArcanistBundle $bundle) {
+    $base_revision = $bundle->getBaseRevision();
     $repository_api = $this->getRepositoryAPI();
-    $base_revision  = $bundle->getBaseRevision();
 
     // verify the base revision is valid
     // in a working copy that uses the git-svn bridge, the base revision might
@@ -343,8 +344,15 @@ EOTEXT
     list($err) = $repository_api->execManualLocal(
       'cat-file -t %s',
       $base_revision);
+    return !$err;
+  }
 
-    if ($base_revision && !$err) {
+  private function createBranch(ArcanistBundle $bundle, $has_base_revision) {
+    $branch_name = $this->getBranchName($bundle);
+    $base_revision = $bundle->getBaseRevision();
+    $repository_api = $this->getRepositoryAPI();
+
+    if ($base_revision && $has_base_revision) {
       $repository_api->execxLocal(
         'checkout -b %s %s',
         $branch_name,
@@ -358,6 +366,8 @@ EOTEXT
     echo phutil_console_format(
       "Created and checked out branch %s.\n",
       $branch_name);
+
+    return $branch_name;
   }
 
   private function createBookmark(ArcanistBundle $bundle) {
@@ -454,15 +464,18 @@ EOTEXT
       $this->updateWorkingCopy();
     }
 
-    if ($this->shouldBranch()) {
-      $this->createBranch($bundle);
+    $repository_api = $this->getRepositoryAPI();
+
+    $has_base_revision = $this->hasBaseRevision($bundle);
+    if ($this->canBranch() && ($this->shouldBranch() || $has_base_revision)) {
+      $original_branch = $repository_api->getBranchName();
+      $new_branch = $this->createBranch($bundle, $has_base_revision);
     }
 
     if ($this->shouldBookmark()) {
       $this->createBookmark($bundle);
     }
 
-    $repository_api = $this->getRepositoryAPI();
     if ($repository_api instanceof ArcanistSubversionAPI) {
       $patch_err = 0;
 
@@ -706,6 +719,21 @@ EOTEXT
       } else {
         $verb = 'applied';
       }
+
+      if ($this->canBranch() && !$this->shouldBranch() && $has_base_revision) {
+        $repository_api->execxLocal('checkout %s', $original_branch);
+        $ex = null;
+        try {
+          $repository_api->execxLocal('cherry-pick %s', $new_branch);
+        } catch (Exception $ex) {}
+        $repository_api->execxLocal('branch -D %s', $new_branch);
+        if ($ex) {
+          echo phutil_console_format(
+            "\n<bg:red>** Cherry Pick Failed!**</bg>\n");
+          throw $ex;
+        }
+      }
+
       echo phutil_console_format(
         "<bg:green>** OKAY **</bg> Successfully {$verb} patch.\n");
     } else if ($repository_api instanceof ArcanistMercurialAPI) {