From 886c1721dda481a19c89d7ba834e565c943cc55f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 3 Dec 2012 16:31:45 -0800
Subject: [PATCH] Restore order of operations after D4056

Summary:
See discussion in D4056. In `findRevision()` we call `loadWorkingCopyDifferentialRevisions()`. However, this method depends upon the state of the working copy, because the end of the commit range it examines is HEAD. Prior to D4056 we checked out the target branch before calling `findRevision()`; after D4056 we call it earlier.

This isn't problematic in the `arc land` case, but in the `arc land <branch>` case it means we may fail to identify a revision, or identify the wrong revision, because HEAD isn't where we expect it to be.

Instead, unconditionally check out the target branch before finding the revision.

See <http://dl.dropbox.com/u/116385/Slingshot/Pictures/Screen%20Shot%202012-12-03%20at%203.43.45%20PM.png> for a transcript of the issue.

Test Plan: Reproduced issue as per link above. Ran `arc land --keep-branch --hold somebranch` successfully after this patch.

Reviewers: DurhamGoode, zeeg

Reviewed By: DurhamGoode

CC: aran

Differential Revision: https://secure.phabricator.com/D4072
---
 src/workflow/ArcanistLandWorkflow.php | 46 +++++++++++++--------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php
index 2ce4212f..4a650eef 100644
--- a/src/workflow/ArcanistLandWorkflow.php
+++ b/src/workflow/ArcanistLandWorkflow.php
@@ -117,9 +117,11 @@ EOTEXT
   public function run() {
     $this->readArguments();
     $this->validate();
-    $this->findRevision();
     $this->pullFromRemote();
 
+    $this->checkoutBranch();
+    $this->findRevision();
+
     if ($this->useSquash) {
       $this->rebase();
       $this->squash();
@@ -221,6 +223,17 @@ EOTEXT
     $this->requireCleanWorkingCopy();
   }
 
+  private function checkoutBranch() {
+    $repository_api = $this->getRepositoryAPI();
+    $repository_api->execxLocal(
+      'checkout %s',
+      $this->branch);
+
+    echo phutil_console_format(
+      "Switched to branch **%s**. Identifying and merging...\n",
+      $this->branch);
+  }
+
   private function findRevision() {
     $repository_api = $this->getRepositoryAPI();
 
@@ -313,30 +326,17 @@ EOTEXT
 
   private function rebase() {
     $repository_api = $this->getRepositoryAPI();
-    $repository_api->execxLocal(
-      'checkout %s',
-      $this->branch);
 
-    echo phutil_console_format(
-      "Switched to branch **%s**. Identifying and merging...\n",
-      $this->branch);
+    chdir($repository_api->getPath());
+    $err = phutil_passthru('git rebase %s', $this->onto);
 
-    if ($this->useSquash) {
-      chdir($repository_api->getPath());
-      $err = phutil_passthru('git rebase %s', $this->onto);
-
-      if ($err) {
-        throw new ArcanistUsageException(
-          "'git rebase {$this->onto}' failed. ".
-          "You can abort with 'git rebase --abort', ".
-          "or resolve conflicts and use 'git rebase ".
-          "--continue' to continue forward. After resolving the rebase, ".
-          "run 'arc land' again.");
-      }
-
-      // Now that we've rebased, the merge-base of origin/master and HEAD may
-      // be different. Reparse the relative commit.
-      $repository_api->parseRelativeLocalCommit(array($this->ontoRemoteBranch));
+    if ($err) {
+      throw new ArcanistUsageException(
+        "'git rebase {$this->onto}' failed. ".
+        "You can abort with 'git rebase --abort', ".
+        "or resolve conflicts and use 'git rebase ".
+        "--continue' to continue forward. After resolving the rebase, ".
+        "run 'arc land' again.");
     }
   }