Don't clean up after conflict in "git cl patch"

After crrev.com/896453002, if "git cl diff" ends up having conflict, it
would be cleaned up. However, if "git cl patch" ends up with conflict,
the user should still be able to manually resolve them.

BUG=438362
TEST=tests/git_cl_test.py

Review URL: https://codereview.chromium.org/1091283004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@295051 0039d316-1c4b-4281-b951-d872f2087c98

git_cl.py

@@ -2583,8 +2583,9 @@ def CMDpatch(parser, args):
 
 
 def PatchIssue(issue_arg, reject, nocommit, directory, auth_config):
-  # There's a "reset --hard" when failing to apply the patch. In order
-  # not to destroy users' data, make sure the tree is not dirty here.
+  # PatchIssue should never be called with a dirty tree.  It is up to the
+  # caller to check this, but just in case we assert here since the
+  # consequences of the caller not checking this could be dire.
   assert(not git_common.is_dirty_git_tree('apply'))
 
   if type(issue_arg) is int or issue_arg.isdigit():
@@ -2635,8 +2636,8 @@ def PatchIssue(issue_arg, reject, nocommit, directory, auth_config):
     subprocess2.check_call(cmd, env=GetNoGitPagerEnv(),
                            stdin=patch_data, stdout=subprocess2.VOID)
   except subprocess2.CalledProcessError:
-    RunGit(['reset', '--hard'])
-    DieWithError('Failed to apply the patch')
+    print 'Failed to apply the patch'
+    return 1
 
   # If we had an issue, commit the current state and register the issue.
   if not nocommit:
@@ -2975,6 +2976,7 @@ def CMDdiff(parser, args):
     # Patch in the latest changes from rietveld.
     rtn = PatchIssue(issue, False, False, None, auth_config)
     if rtn != 0:
+      RunGit(['reset', '--hard'])
       return rtn
 
     # Switch back to starting branch and diff against the temporary

tests/git_cl_test.py

@@ -110,7 +110,11 @@ class TestGitCl(TestCase):
     self.assertTrue(
         self.calls,
         '@%d  Expected: <Missing>   Actual: %r' % (self._calls_done, args))
-    expected_args, result = self.calls.pop(0)
+    top = self.calls.pop(0)
+    if len(top) > 2 and top[2]:
+      raise top[2]
+    expected_args, result = top
+
     # Also logs otherwise it could get caught in a try/finally and be hard to
     # diagnose.
     if expected_args != args:
@@ -854,6 +858,47 @@ class TestGitCl(TestCase):
                      git_cl.GetTargetRef('origin', 'refs/remotes/origin/master',
                                          None, 'prefix/'))
 
+  def test_patch_when_dirty(self):
+    # Patch when local tree is dirty
+    self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
+    self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
+
+  def test_diff_when_dirty(self):
+    # Do 'git cl diff' when local tree is dirty
+    self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
+    self.assertNotEqual(git_cl.main(['diff']), 0)
+
+  def _patch_common(self):
+    self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda x: '60001')
+    self.mock(git_cl.Changelist, 'GetPatchSetDiff', lambda *args: None)
+    self.mock(git_cl.Changelist, 'SetIssue', lambda *args: None)
+    self.mock(git_cl.Changelist, 'SetPatchset', lambda *args: None)
+    self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True)
+
+    self.calls = [
+      ((['git', 'config', 'rietveld.autoupdate'],), ''),
+      ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'),
+      ((['git', 'rev-parse', '--show-cdup'],), ''),
+      ((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''),
+    ]
+
+  def test_patch_successful(self):
+    self._patch_common()
+    self.calls += [
+      ((['git', 'apply', '--index', '-p0', '--3way'],), ''),
+      ((['git', 'commit', '-m',
+         'patch from issue 123456 at patchset 60001 ' +
+         '(http://crrev.com/123456#ps60001)'],), ''),
+    ]
+    self.assertEqual(git_cl.main(['patch', '123456']), 0)
+
+  def test_patch_conflict(self):
+    self._patch_common()
+    self.calls += [
+      ((['git', 'apply', '--index', '-p0', '--3way'],), '',
+       subprocess2.CalledProcessError(1, '', '', '', '')),
+    ]
+    self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
 
 if __name__ == '__main__':
   git_cl.logging.basicConfig(