From 411a4f4a3917f6916e7e4d1811ae1c8129af6a7a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 28 Oct 2015 10:03:51 -0700
Subject: [PATCH] Fix a check when deciding to destroy the local branch after
 "arc land"

Summary: Fixes T9660. The behavior for this check wasn't quite right -- we want to check the "source ref" (what we're landing) against the "target onto" (the branch we're landing it onto).

Test Plan:
  - Landed from `master` (tracking origin/master). No delete.
  - Landed from `feature1` (tracking local/master). Delete.
  - Landed from `feature2` (tracking origin/master). Delete.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9660

Differential Revision: https://secure.phabricator.com/D14358
---
 src/land/ArcanistGitLandEngine.php | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php
index 1a55e14a..945a5f39 100644
--- a/src/land/ArcanistGitLandEngine.php
+++ b/src/land/ArcanistGitLandEngine.php
@@ -36,6 +36,9 @@ final class ArcanistGitLandEngine
         $this->pushChange();
         $this->reconcileLocalState();
 
+        $api = $this->getRepositoryAPI();
+        $api->execxLocal('submodule update --init --recursive');
+
         if ($this->getShouldKeep()) {
           echo tsprintf(
             "%s\n",
@@ -269,7 +272,6 @@ final class ArcanistGitLandEngine
 
       $api->execxLocal('checkout %s --', $this->getTargetOnto());
       $api->execxLocal('pull --');
-      $api->execxLocal('submodule update --init --recursive');
 
       return;
     }
@@ -284,7 +286,6 @@ final class ArcanistGitLandEngine
           $this->getTargetFullRef()));
 
       $api->execxLocal('checkout %s --', $this->getTargetOnto());
-      $api->execxLocal('submodule update --init --recursive');
 
       return;
     }
@@ -303,17 +304,20 @@ final class ArcanistGitLandEngine
 
     $api->execxLocal('checkout %s --', $this->getTargetOnto());
     $api->execxLocal('reset --hard %s --', $this->getTargetFullRef());
-    $api->execxLocal('submodule update --init --recursive');
   }
 
   private function destroyLocalBranch() {
     $api = $this->getRepositoryAPI();
 
-    if ($this->localRef == $this->getSourceRef()) {
-      // If we landed a branch onto itself, don't destroy it.
+    if ($this->getSourceRef() == $this->getTargetOnto()) {
+      // If we landed a branch into a branch with the same name, so don't
+      // destroy it. This prevents us from cleaning up "master" if you're
+      // landing master into itself.
       return;
     }
 
+    // TODO: Maybe this should also recover the proper upstream?
+
     $recovery_command = csprintf(
       'git checkout -b %R %R',
       $this->getSourceRef(),