From c44e8d57a2d5f310d404d328dd744df1e73fbb81 Mon Sep 17 00:00:00 2001
From: Kevin Deggelman <kevin@tenxer.com>
Date: Fri, 3 Feb 2012 14:04:41 -0800
Subject: [PATCH] Check for cleanWorkingCopy in sanityCheck, and execute
 sanityCheck first

Summary:
  - Renamed sanityCheckPatch to sanityCheck
  - Move check for clean working copy into sanityCheck
  - Execute sanityCheck before executing any other command

Test Plan:
  - Run ##arc patch <revision id>## from a dirty working copy
and verify that a usage exception is thrown before the new branch is
created.
  - Run ##arc patch <revision id> --force## and verify that it attempts
    to create a new branch, apply, and commit the patch.
  - Then clean your working copy and verify that ##arc patch <revision
    id> works as expected

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T830, T479

Differential Revision: https://secure.phabricator.com/D1546
---
 src/workflow/patch/ArcanistPatchWorkflow.php | 26 +++++++++-----------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/workflow/patch/ArcanistPatchWorkflow.php b/src/workflow/patch/ArcanistPatchWorkflow.php
index 6e0dd8ce..38c89b87 100644
--- a/src/workflow/patch/ArcanistPatchWorkflow.php
+++ b/src/workflow/patch/ArcanistPatchWorkflow.php
@@ -232,7 +232,7 @@ EOTEXT
       // no error means git rev-parse found a branch
       if (!$err) {
         echo phutil_console_format(
-          "Branch name {$proposed_name} alread exists; trying a new name.\n"
+          "Branch name {$proposed_name} already exists; trying a new name.\n"
         );
         continue;
       } else {
@@ -354,6 +354,12 @@ EOTEXT
         throw $ex;
       }
     }
+    $force = $this->getArgument('force', false);
+    if ($force) {
+      // force means don't do any sanity checks about the patch
+    } else {
+      $this->sanityCheck($bundle);
+    }
 
     // we should update the working copy before we do ANYTHING else
     if ($this->shouldUpdateWorkingCopy()) {
@@ -364,13 +370,6 @@ EOTEXT
       $this->createBranch($bundle);
     }
 
-    $force = $this->getArgument('force', false);
-    if ($force) {
-      // force means don't do any sanity checks about the patch
-    } else {
-      $this->sanityCheckPatch($bundle);
-    }
-
     $repository_api = $this->getRepositoryAPI();
     if ($repository_api instanceof ArcanistSubversionAPI) {
       $patch_err = 0;
@@ -570,12 +569,6 @@ 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());
@@ -663,7 +656,10 @@ EOTEXT
   /**
    * Do the best we can to prevent PEBKAC and id10t issues.
    */
-  private function sanityCheckPatch(ArcanistBundle $bundle) {
+  private function sanityCheck(ArcanistBundle $bundle) {
+
+    // Require clean working copy
+    $this->requireCleanWorkingCopy();
 
     // Check to see if the bundle's project id matches the working copy
     // project id