From 8a1396cd723c08cac666e06ab60e3f01828be7d7 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Fri, 22 Apr 2011 00:14:24 +0000 Subject: [PATCH] Add post_process argument to Checkout.apply_patch(). This enables doing late modifications like updating the copyright notice or fixing line endings. Also silence the checkout operations a bit more when VOID=subprocess2.VOID. Add corresponding unit tests. Review URL: http://codereview.chromium.org/6891003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@82587 0039d316-1c4b-4281-b951-d872f2087c98 --- checkout.py | 51 +++++++++++++++++++++++++++++------------- patch.py | 2 ++ tests/checkout_test.py | 30 +++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/checkout.py b/checkout.py index fcdea220a..ab449b4da 100644 --- a/checkout.py +++ b/checkout.py @@ -80,11 +80,16 @@ class CheckoutBase(object): """ raise NotImplementedError() - def apply_patch(self, patches): + def apply_patch(self, patches, post_processor=None): """Applies a patch and returns the list of modified files. This function should throw patch.UnsupportedPatchFormat or PatchApplicationFailed when relevant. + + Args: + patches: patch.PatchSet object. + post_processor: list of lambda(checkout, patches) to call on each of the + modified files. """ raise NotImplementedError() @@ -102,7 +107,9 @@ class RawCheckout(CheckoutBase): """Stubbed out.""" pass - def apply_patch(self, patches): + def apply_patch(self, patches, post_processor=None): + """Ignores svn properties.""" + post_processor = post_processor or [] for p in patches: try: stdout = '' @@ -122,7 +129,8 @@ class RawCheckout(CheckoutBase): ['patch', '-p%s' % p.patchlevel], stdin=p.get(), cwd=self.project_path) - # Ignore p.svn_properties. + for post in post_processor: + post(self, p) except OSError, e: raise PatchApplicationFailed(p.filename, '%s%s' % (stdout, e)) except subprocess.CalledProcessError, e: @@ -223,7 +231,6 @@ class SvnCheckout(CheckoutBase, SvnMixIn): assert self.svn_url def prepare(self): - """Creates the initial checkouts for the repo.""" # Will checkout if the directory is not present. if not os.path.isdir(self.project_path): logging.info('Checking out %s in %s' % @@ -234,8 +241,8 @@ class SvnCheckout(CheckoutBase, SvnMixIn): self._last_seen_revision = revision return revision - def apply_patch(self, patches): - """Applies a patch.""" + def apply_patch(self, patches, post_processor=None): + post_processor = post_processor or [] for p in patches: try: stdout = '' @@ -274,6 +281,8 @@ class SvnCheckout(CheckoutBase, SvnMixIn): if fnmatch.fnmatch(p.filename, prop): stdout += self._check_output_svn( ['propset'] + value.split('=', 1) + [p.filename]) + for post in post_processor: + post(self, p) except OSError, e: raise PatchApplicationFailed(p.filename, '%s%s' % (stdout, e)) except subprocess.CalledProcessError, e: @@ -352,13 +361,19 @@ class GitCheckoutBase(CheckoutBase): if self.working_branch in branches: self._call_git(['branch', '-D', self.working_branch]) - def apply_patch(self, patches): - """Applies a patch on 'working_branch' and switch to it.""" + def apply_patch(self, patches, post_processor=None): + """Applies a patch on 'working_branch' and switch to it. + + Also commits the changes on the local branch. + + Ignores svn properties and raise an exception on unexpected ones. + """ + post_processor = post_processor or [] # It this throws, the checkout is corrupted. Maybe worth deleting it and # trying again? self._check_call_git( ['checkout', '-b', self.working_branch, - '%s/%s' % (self.remote, self.remote_branch)]) + '%s/%s' % (self.remote, self.remote_branch), '--quiet']) for p in patches: try: stdout = '' @@ -387,6 +402,8 @@ class GitCheckoutBase(CheckoutBase): p.filename, 'Cannot apply svn property %s to file %s.' % ( prop[0], p.filename)) + for post in post_processor: + post(self, p) except OSError, e: raise PatchApplicationFailed(p.filename, '%s%s' % (stdout, e)) except subprocess.CalledProcessError, e: @@ -492,6 +509,7 @@ class GitSvnCheckoutBase(GitCheckoutBase, SvnMixIn): kwargs = {} if self.commit_pwd: kwargs['stdin'] = self.commit_pwd + '\n' + kwargs['stderr'] = subprocess2.STDOUT self._check_call_git_svn( ['dcommit', '--rmdir', '--find-copies-harder', '--username', self.commit_user], @@ -547,8 +565,9 @@ class GitSvnPremadeCheckout(GitSvnCheckoutBase): assert self.remote == 'origin' # self.project_path doesn't exist yet. self._check_call_git( - ['clone', self.git_url, self.project_name], - cwd=self.root_dir) + ['clone', self.git_url, self.project_name, '--quiet'], + cwd=self.root_dir, + stderr=subprocess2.STDOUT) try: configured_svn_url = self._check_output_git( ['config', 'svn-remote.svn.url']).strip() @@ -591,8 +610,10 @@ class GitSvnCheckout(GitSvnCheckoutBase): ['clone', '--prefix', self.remote + '/', '-T', self.trunk, - self.svn_url, self.project_path], - cwd=self.root_dir) + self.svn_url, self.project_path, + '--quiet'], + cwd=self.root_dir, + stderr=subprocess2.STDOUT) super(GitSvnCheckout, self).prepare() return self._get_revision() @@ -608,8 +629,8 @@ class ReadOnlyCheckout(object): def get_settings(self, key): return self.checkout.get_settings(key) - def apply_patch(self, patches): - return self.checkout.apply_patch(patches) + def apply_patch(self, patches, post_processor=None): + return self.checkout.apply_patch(patches, post_processor) def commit(self, message, user): # pylint: disable=R0201 logging.info('Would have committed for %s with message: %s' % ( diff --git a/patch.py b/patch.py index 316333f29..bac992d43 100644 --- a/patch.py +++ b/patch.py @@ -273,6 +273,8 @@ class PatchSet(object): def __init__(self, patches): self.patches = patches + for p in self.patches: + assert isinstance(p, FilePatchBase) def set_relpath(self, relpath): """Used to offset the patch into a subdirectory.""" diff --git a/tests/checkout_test.py b/tests/checkout_test.py index 749e638ee..16d4d1d11 100755 --- a/tests/checkout_test.py +++ b/tests/checkout_test.py @@ -233,6 +233,15 @@ class BaseTest(fake_repos.FakeReposTestBase): def _log(self): raise NotImplementedError() + def _test_process(self, co): + """Makes sure the process lambda is called correctly.""" + co.prepare() + ps = self.get_patches() + results = [] + co.apply_patch(ps, [lambda *args: results.append(args)]) + expected = [(co, p) for p in ps.patches] + self.assertEquals(expected, results) + class SvnBaseTest(BaseTest): def setUp(self): @@ -377,6 +386,13 @@ class SvnCheckout(SvnBaseTest): cwd=co.project_path) self.assertEquals('LF\n', out) + def testProcess(self): + co = checkout.SvnCheckout( + self.root_dir, self.name, + None, None, + self.svn_url) + self._test_process(co) + class GitSvnCheckout(SvnBaseTest): name = 'foo.git' @@ -442,6 +458,13 @@ class GitSvnCheckout(SvnBaseTest): co.apply_patch( [patch.FilePatchDiff('svn_utils_test.txt', NAKED_PATCH, svn_props)]) + def testProcess(self): + co = checkout.SvnCheckout( + self.root_dir, self.name, + None, None, + self.svn_url) + self._test_process(co) + class RawCheckout(SvnBaseTest): def setUp(self): @@ -502,6 +525,13 @@ class RawCheckout(SvnBaseTest): '1 out of 1 hunk FAILED -- saving rejects to file ' 'svn_utils_test.txt.rej\n') + def testProcess(self): + co = checkout.SvnCheckout( + self.root_dir, self.name, + None, None, + self.svn_url) + self._test_process(co) + if __name__ == '__main__': if '-v' in sys.argv: