From c4396a1b4e231e3b271e36d7b76e280e89245b04 Mon Sep 17 00:00:00 2001 From: "hinoka@chromium.org" Date: Sat, 10 May 2014 02:19:27 +0000 Subject: [PATCH] Re-land of 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 Review URL: https://codereview.chromium.org/280063003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@269489 0039d316-1c4b-4281-b951-d872f2087c98 --- apply_issue.py | 11 +++-------- checkout.py | 41 ++++++++++++----------------------------- tests/checkout_test.py | 4 ++-- 3 files changed, 17 insertions(+), 39 deletions(-) diff --git a/apply_issue.py b/apply_issue.py index 91fe2f7f5..801b0f17b 100755 --- a/apply_issue.py +++ b/apply_issue.py @@ -85,8 +85,7 @@ 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('-b', '--base_ref', help='DEPRECATED do not use.') parser.add_option('--whitelist', action='append', default=[], help='Patch only specified file(s).') parser.add_option('--blacklist', action='append', default=[], @@ -205,8 +204,7 @@ 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, - base_ref=options.base_ref,) + scm_obj = checkout.GitCheckout(full_dir, None, None, None, None) elif scm_type == None: scm_obj = checkout.RawCheckout(full_dir, None, None) else: @@ -223,10 +221,7 @@ def main(): print('\nApplying the patch.') try: - scm_obj.apply_patch( - patchset, verbose=True, - email=properties.get('owner_email', 'chrome-bot@chromium.org'), - name=properties.get('owner', 'chrome-bot')) + scm_obj.apply_patch(patchset, verbose=True) except checkout.PatchApplicationFailed, e: print(str(e)) print('CWD=%s' % os.getcwd()) diff --git a/checkout.py b/checkout.py index f58dedd75..68781cb4c 100644 --- a/checkout.py +++ b/checkout.py @@ -131,8 +131,7 @@ class CheckoutBase(object): """ raise NotImplementedError() - def apply_patch(self, patches, post_processors=None, verbose=False, - name=None, email=None): + def apply_patch(self, patches, post_processors=None, verbose=False): """Applies a patch and returns the list of modified files. This function should throw patch.UnsupportedPatchFormat or @@ -166,8 +165,7 @@ class RawCheckout(CheckoutBase): """Stubbed out.""" pass - def apply_patch(self, patches, post_processors=None, verbose=False, - name=None, email=None): + def apply_patch(self, patches, post_processors=None, verbose=False): """Ignores svn properties.""" post_processors = post_processors or self.post_processors or [] for p in patches: @@ -351,8 +349,7 @@ class SvnCheckout(CheckoutBase, SvnMixIn): (self.project_name, self.project_path)) return self._revert(revision) - def apply_patch(self, patches, post_processors=None, verbose=False, - name=None, email=None): + def apply_patch(self, patches, post_processors=None, verbose=False): post_processors = post_processors or self.post_processors or [] for p in patches: stdout = [] @@ -556,9 +553,8 @@ 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, base_ref=None): + commit_user, post_processors=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 @@ -631,11 +627,10 @@ 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, - name=None, email=None): + def apply_patch(self, patches, post_processors=None, verbose=False): """Applies a patch on 'working_branch' and switches to it. - Also commits the changes on the local branch. + The changes remain staged on the current branch. Ignores svn properties and raise an exception on unexpected ones. """ @@ -709,22 +704,9 @@ 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', base_ref, '--ignore-submodules', - '--name-only']).splitlines(False) + ['diff', '--ignore-submodules', + '--name-only', '--staged']).splitlines(False) assert sorted(patches.filenames) == sorted(found_files), ( 'Found extra %s locally, %s not patched' % ( sorted(set(found_files) - set(patches.filenames)), @@ -732,13 +714,15 @@ 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', '--amend', '-m', commit_message] + commit_cmd = ['commit', '-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. @@ -826,8 +810,7 @@ class ReadOnlyCheckout(object): def get_settings(self, key): return self.checkout.get_settings(key) - def apply_patch(self, patches, post_processors=None, verbose=False, - name=None, email=None): + def apply_patch(self, patches, post_processors=None, verbose=False): return self.checkout.apply_patch( patches, post_processors or self.post_processors, verbose) diff --git a/tests/checkout_test.py b/tests/checkout_test.py index e36aac887..d07f15ac7 100755 --- a/tests/checkout_test.py +++ b/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', 'HEAD~', '--name-status'], cwd=co.project_path) + ['git', 'diff', '--staged', '--name-status'], cwd=co.project_path) out = sorted(out.splitlines()) expected = sorted( [