Revert of Have apply_patch.py/checkout.py stage git patches instead of...

Revert of Have apply_patch.py/checkout.py stage git patches instead of committing them (https://codereview.chromium.org/273543002/)

Reason for revert:
Borked all the tryjobs due to missing --base_ref, needed to land with https://codereview.chromium.org/273543002/

Original issue's description:
> Have apply_patch.py/checkout.py stage git patches instead of committing them
> 
> For a bot_update Git world, we don't really want to commit patches.  Instead
> we just want to leave them unstaged.
> 
> BUG=370503
> TEST=ran locally with and without flag
> 
> Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=269468

TBR=maruel@chromium.org,agable@chromium.org,hinoka@google.com,iannucci@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=370503

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

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

apply_issue.py

@@ -85,6 +85,8 @@ def main():
   parser.add_option('-f', '--force', action='store_true',
                     help='Really run apply_issue, even if .update.flag '
                          'is detected.')
+  parser.add_option('-b', '--base_ref', help='Base git ref to patch on top of, '
+                    'used for verification.')
   parser.add_option('--whitelist', action='append', default=[],
                     help='Patch only specified file(s).')
   parser.add_option('--blacklist', action='append', default=[],
@@ -203,7 +205,8 @@ def main():
   if scm_type == 'svn':
     scm_obj = checkout.SvnCheckout(full_dir, None, None, None, None)
   elif scm_type == 'git':
-    scm_obj = checkout.GitCheckout(full_dir, None, None, None, None)
+    scm_obj = checkout.GitCheckout(full_dir, None, None, None, None,
+                                   base_ref=options.base_ref,)
   elif scm_type == None:
     scm_obj = checkout.RawCheckout(full_dir, None, None)
   else:
@@ -220,7 +223,10 @@ def main():
 
   print('\nApplying the patch.')
   try:
-    scm_obj.apply_patch(patchset, verbose=True)
+    scm_obj.apply_patch(
+        patchset, verbose=True,
+        email=properties.get('owner_email', 'chrome-bot@chromium.org'),
+        name=properties.get('owner', 'chrome-bot'))
   except checkout.PatchApplicationFailed, e:
     print(str(e))
     print('CWD=%s' % os.getcwd())

checkout.py

@@ -131,7 +131,8 @@ class CheckoutBase(object):
     """
     raise NotImplementedError()
 
-  def apply_patch(self, patches, post_processors=None, verbose=False):
+  def apply_patch(self, patches, post_processors=None, verbose=False,
+                  name=None, email=None):
     """Applies a patch and returns the list of modified files.
 
     This function should throw patch.UnsupportedPatchFormat or
@@ -165,7 +166,8 @@ class RawCheckout(CheckoutBase):
     """Stubbed out."""
     pass
 
-  def apply_patch(self, patches, post_processors=None, verbose=False):
+  def apply_patch(self, patches, post_processors=None, verbose=False,
+                  name=None, email=None):
     """Ignores svn properties."""
     post_processors = post_processors or self.post_processors or []
     for p in patches:
@@ -349,7 +351,8 @@ class SvnCheckout(CheckoutBase, SvnMixIn):
           (self.project_name, self.project_path))
     return self._revert(revision)
 
-  def apply_patch(self, patches, post_processors=None, verbose=False):
+  def apply_patch(self, patches, post_processors=None, verbose=False,
+                  name=None, email=None):
     post_processors = post_processors or self.post_processors or []
     for p in patches:
       stdout = []
@@ -553,8 +556,9 @@ class SvnCheckout(CheckoutBase, SvnMixIn):
 class GitCheckout(CheckoutBase):
   """Manages a git checkout."""
   def __init__(self, root_dir, project_name, remote_branch, git_url,
-      commit_user, post_processors=None):
+      commit_user, post_processors=None, base_ref=None):
     super(GitCheckout, self).__init__(root_dir, project_name, post_processors)
+    self.base_ref = base_ref
     self.git_url = git_url
     self.commit_user = commit_user
     self.remote_branch = remote_branch
@@ -627,10 +631,11 @@ class GitCheckout(CheckoutBase):
     """Gets the current revision (in unicode) from the local branch."""
     return unicode(self._check_output_git(['rev-parse', 'HEAD']).strip())
 
-  def apply_patch(self, patches, post_processors=None, verbose=False):
+  def apply_patch(self, patches, post_processors=None, verbose=False,
+                  name=None, email=None):
     """Applies a patch on 'working_branch' and switches to it.
 
-    The changes remain staged on the current branch.
+    Also commits the changes on the local branch.
 
     Ignores svn properties and raise an exception on unexpected ones.
     """
@@ -704,9 +709,22 @@ class GitCheckout(CheckoutBase):
               ' '.join(e.cmd),
               align_stdout(stdout),
               align_stdout([getattr(e, 'stdout', '')])))
+    # Once all the patches are processed and added to the index, commit the
+    # index.
+    cmd = ['commit', '-m', 'Committed patch']
+    if name and email:
+      cmd = ['-c', 'user.email=%s' % email, '-c', 'user.name=%s' % name] + cmd
+    if verbose:
+      cmd.append('--verbose')
+    self._check_call_git(cmd)
+    if self.base_ref:
+      base_ref = self.base_ref
+    else:
+      base_ref = '%s/%s' % (self.remote,
+                            self.remote_branch or self.master_branch)
     found_files = self._check_output_git(
-        ['diff', '--ignore-submodules',
-         '--name-only', '--staged']).splitlines(False)
+        ['diff', base_ref, '--ignore-submodules',
+         '--name-only']).splitlines(False)
     assert sorted(patches.filenames) == sorted(found_files), (
         'Found extra %s locally, %s not patched' % (
             sorted(set(found_files) - set(patches.filenames)),
@@ -714,15 +732,13 @@ class GitCheckout(CheckoutBase):
 
   def commit(self, commit_message, user):
     """Commits, updates the commit message and pushes."""
-    # TODO(hinoka): CQ no longer uses this, I think its deprecated.
-    #               Delete this.
     assert self.commit_user
     assert isinstance(commit_message, unicode)
     current_branch = self._check_output_git(
         ['rev-parse', '--abbrev-ref', 'HEAD']).strip()
     assert current_branch == self.working_branch
 
-    commit_cmd = ['commit', '-m', commit_message]
+    commit_cmd = ['commit', '--amend', '-m', commit_message]
     if user and user != self.commit_user:
       # We do not have the first or last name of the user, grab the username
       # from the email and call it the original author's name.
@@ -810,7 +826,8 @@ class ReadOnlyCheckout(object):
   def get_settings(self, key):
     return self.checkout.get_settings(key)
 
-  def apply_patch(self, patches, post_processors=None, verbose=False):
+  def apply_patch(self, patches, post_processors=None, verbose=False,
+                  name=None, email=None):
     return self.checkout.apply_patch(
         patches, post_processors or self.post_processors, verbose)
 

tests/checkout_test.py

@@ -423,7 +423,7 @@ class GitBaseTest(BaseTest):
     for k, v in self.FAKE_REPOS.git_hashes[
         self.FAKE_REPOS.TEST_GIT_REPO][1][1].iteritems():
       assert k not in tree
-      tree[k] = v
+      tree[k] = v 
 
     if modified:
       content_lines = tree['chrome/file.cc'].splitlines(True)
@@ -470,7 +470,7 @@ class GitCheckout(GitBaseTest):
     co = self._get_co(None)
     self._check_move(co)
     out = subprocess2.check_output(
-        ['git', 'diff', '--staged', '--name-status'], cwd=co.project_path)
+        ['git', 'diff', 'HEAD~', '--name-status'], cwd=co.project_path)
     out = sorted(out.splitlines())
     expected = sorted(
       [