From d343c63519c179f158c522f27f332e129d27a26c Mon Sep 17 00:00:00 2001
From: Aaron Gable <agable@chromium.org>
Date: Wed, 15 Mar 2017 11:02:26 -0700
Subject: [PATCH] git-cl patch: set metadata before (failing to) apply patch

If the commit or cherry-pick command fails, git automatically drops
the user into a state where they can try to manually fix it. That's
great. But with the old call order, it would also leave the metadata
unset. This sets the issue and patchset number before doing the
potentially-failing heavy lifting, so that the branch will be in a
consistent state after the user finishes fixing up the failed command.

BUG=701130

Change-Id: I792b9fb9e61ba62626c19aa1837d21f8cd8f594e
Reviewed-on: https://chromium-review.googlesource.com/456039
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
---
 git_cl.py            |  6 +++---
 tests/git_cl_test.py | 19 ++++++++++++-------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/git_cl.py b/git_cl.py
index b0f169fad..8dea2d2b2 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -2070,12 +2070,12 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
 
     # If we had an issue, commit the current state and register the issue.
     if not nocommit:
+      self.SetIssue(self.GetIssue())
+      self.SetPatchset(patchset)
       RunGit(['commit', '-m', (self.GetDescription() + '\n\n' +
                                'patch from issue %(i)s at patchset '
                                '%(p)s (http://crrev.com/%(i)s#ps%(p)s)'
                                % {'i': self.GetIssue(), 'p': patchset})])
-      self.SetIssue(self.GetIssue())
-      self.SetPatchset(patchset)
       print('Committed patch locally.')
     else:
       print('Patch applied to index.')
@@ -2635,9 +2635,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
 
     fetch_info = revision_info['fetch']['http']
     RunGit(['fetch', fetch_info['url'], fetch_info['ref']])
-    RunGit(['cherry-pick', 'FETCH_HEAD'])
     self.SetIssue(self.GetIssue())
     self.SetPatchset(patchset)
+    RunGit(['cherry-pick', 'FETCH_HEAD'])
     print('Committed patch for change %i patchset %i locally' %
           (self.GetIssue(), self.GetPatchset()))
     return 0
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 588c016ac..b52ee6050 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -1815,16 +1815,16 @@ class TestGitCl(TestCase):
   def _common_patch_successful(self, new_branch=False):
     self._patch_common(new_branch=new_branch)
     self.calls += [
-      ((['git', 'commit', '-m',
-         'Description\n\n' +
-         'patch from issue 123456 at patchset 60001 ' +
-         '(http://crrev.com/123456#ps60001)'],), ''),
       ((['git', 'config', 'branch.master.rietveldissue', '123456'],),
        ''),
       ((['git', 'config', 'branch.master.rietveldserver',
          'https://codereview.example.com'],), ''),
       ((['git', 'config', 'branch.master.rietveldpatchset', '60001'],),
        ''),
+      ((['git', 'commit', '-m',
+         'Description\n\n' +
+         'patch from issue 123456 at patchset 60001 ' +
+         '(http://crrev.com/123456#ps60001)'],), ''),
     ]
 
   def test_patch_successful(self):
@@ -1846,12 +1846,12 @@ class TestGitCl(TestCase):
     self.calls += [
       ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
          'refs/changes/56/123456/7'],), ''),
-      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
       ((['git', 'config', 'branch.master.gerritissue', '123456'],),
        ''),
       ((['git', 'config', 'branch.master.gerritserver',
         'https://chromium-review.googlesource.com'],), ''),
       ((['git', 'config', 'branch.master.gerritpatchset', '7'],), ''),
+      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
     ]
     self.assertEqual(git_cl.main(['patch', '123456']), 0)
 
@@ -1861,12 +1861,12 @@ class TestGitCl(TestCase):
     self.calls += [
       ((['git', 'fetch', 'https://host.googlesource.com/my/repo',
          'refs/changes/56/123456/7'],), ''),
-      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
       ((['git', 'config', 'branch.master.gerritissue', '123456'],),
        ''),
       ((['git', 'config', 'branch.master.gerritserver',
         'https://host-review.googlesource.com'],), ''),
       ((['git', 'config', 'branch.master.gerritpatchset', '7'],), ''),
+      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
     ]
     self.assertEqual(git_cl.main(['patch', '--gerrit', '123456']), 0)
 
@@ -1876,12 +1876,12 @@ class TestGitCl(TestCase):
     self.calls += [
       ((['git', 'fetch', 'https://else.googlesource.com/my/repo',
          'refs/changes/56/123456/1'],), ''),
-      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
       ((['git', 'config', 'branch.master.gerritissue', '123456'],),
        ''),
       ((['git', 'config', 'branch.master.gerritserver',
         'https://else-review.googlesource.com'],), ''),
       ((['git', 'config', 'branch.master.gerritpatchset', '1'],), ''),
+      ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
     ]
     self.assertEqual(git_cl.main(
       ['patch', 'https://else-review.googlesource.com/#/c/123456/1']), 0)
@@ -1892,6 +1892,11 @@ class TestGitCl(TestCase):
     self.calls += [
       ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
          'refs/changes/56/123456/1'],), ''),
+      ((['git', 'config', 'branch.master.gerritissue', '123456'],),
+       ''),
+      ((['git', 'config', 'branch.master.gerritserver',
+        'https://chromium-review.googlesource.com'],), ''),
+      ((['git', 'config', 'branch.master.gerritpatchset', '1'],), ''),
       ((['git', 'cherry-pick', 'FETCH_HEAD'],), CERR1),
       ((['DieWithError', 'Command "git cherry-pick FETCH_HEAD" failed.\n'],),
        SystemExitMock()),