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
diff --git a/git_cl.py b/git_cl.py
index d4486e5..b94be71 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -2583,8 +2583,9 @@
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 @@
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 @@
# 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
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index c1eebeb..f633581 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -110,7 +110,11 @@
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 @@
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(
