From 045d107c238b900e392dd07c49b6add6b4db7ab7 Mon Sep 17 00:00:00 2001 From: iannucci Date: Tue, 6 Sep 2016 17:51:18 -0700 Subject: [PATCH] Remove patch_url from bot_update. R=hinoka@chromium.org, martiniss@chromium.org, tandrii@chromium.org BUG=643885 Review-Url: https://codereview.chromium.org/2310113003 --- recipe_modules/bot_update/__init__.py | 1 - recipe_modules/bot_update/api.py | 17 ++----- .../bot_update/example.expected/tryjob.json | 8 +++- .../example.expected/tryjob_fail.json | 8 +++- .../example.expected/tryjob_fail_patch.json | 8 +++- .../tryjob_fail_patch_download.json | 8 +++- .../example.expected/tryjob_v8.json | 8 +++- recipe_modules/bot_update/example.py | 10 ++-- .../bot_update/resources/bot_update.py | 46 ++----------------- 9 files changed, 43 insertions(+), 71 deletions(-) diff --git a/recipe_modules/bot_update/__init__.py b/recipe_modules/bot_update/__init__.py index 11364c78f..7db11595e 100644 --- a/recipe_modules/bot_update/__init__.py +++ b/recipe_modules/bot_update/__init__.py @@ -20,7 +20,6 @@ PROPERTIES = { 'slavename': Property(default=None), 'issue': Property(default=None), 'patchset': Property(default=None), - 'patch_url': Property(default=None), 'patch_project': Property(default=None), 'repository': Property(default=None), 'event.patchSet.ref': Property(default=None, param_name="gerrit_ref"), diff --git a/recipe_modules/bot_update/api.py b/recipe_modules/bot_update/api.py index d88d92993..456722890 100644 --- a/recipe_modules/bot_update/api.py +++ b/recipe_modules/bot_update/api.py @@ -11,15 +11,13 @@ from recipe_engine import recipe_api class BotUpdateApi(recipe_api.RecipeApi): def __init__(self, mastername, buildername, slavename, issue, patchset, - patch_url, repository, gerrit_ref, rietveld, revision, - parent_got_revision, deps_revision_overrides, fail_patch, - *args, **kwargs): + repository, gerrit_ref, rietveld, revision, parent_got_revision, + deps_revision_overrides, fail_patch, *args, **kwargs): self._mastername = mastername self._buildername = buildername self._slavename = slavename self._issue = issue self._patchset = patchset - self._patch_url = patch_url self._repository = repository self._gerrit_ref = gerrit_ref self._rietveld = rietveld @@ -113,13 +111,12 @@ class BotUpdateApi(recipe_api.RecipeApi): if patch: issue = issue or self._issue patchset = patchset or self._patchset - patch_url = self._patch_url gerrit_repo = self._repository gerrit_ref = self._gerrit_ref else: # The trybot recipe sometimes wants to de-apply the patch. In which case - # we pretend the issue/patchset/patch_url never existed. - issue = patchset = patch_url = email_file = key_file = None + # we pretend the issue/patchset never existed. + issue = patchset = email_file = key_file = None gerrit_repo = gerrit_ref = None # Issue and patchset must come together. @@ -127,9 +124,6 @@ class BotUpdateApi(recipe_api.RecipeApi): assert patchset if patchset: assert issue - if patch_url: - # If patch_url is present, bot_update will actually ignore issue/ps. - issue = patchset = None # The gerrit_ref and gerrit_repo must be together or not at all. If one is # missing, clear both of them. @@ -173,10 +167,9 @@ class BotUpdateApi(recipe_api.RecipeApi): ['--revision_mapping_file', self.m.json.input(rev_map)], ['--git-cache-dir', cfg.cache_dir], - # 3. How to find the patch, if any (issue/patchset/patch_url). + # 3. How to find the patch, if any (issue/patchset). ['--issue', issue], ['--patchset', patchset], - ['--patch_url', patch_url], ['--rietveld_server', rietveld or self._rietveld], ['--gerrit_repo', gerrit_repo], ['--gerrit_ref', gerrit_ref], diff --git a/recipe_modules/bot_update/example.expected/tryjob.json b/recipe_modules/bot_update/example.expected/tryjob.json index 17a96e32f..46a097fd4 100644 --- a/recipe_modules/bot_update/example.expected/tryjob.json +++ b/recipe_modules/bot_update/example.expected/tryjob.json @@ -18,8 +18,12 @@ "{\"src\": \"got_cr_revision\"}", "--git-cache-dir", "[GIT_CACHE]", - "--patch_url", - "http://src.chromium.org/foo/bar", + "--issue", + "12345", + "--patchset", + "654321", + "--rietveld_server", + "https://rietveld.example.com/", "--output_json", "/path/to/tmp/json", "--revision", diff --git a/recipe_modules/bot_update/example.expected/tryjob_fail.json b/recipe_modules/bot_update/example.expected/tryjob_fail.json index 091f5eac0..77238b56b 100644 --- a/recipe_modules/bot_update/example.expected/tryjob_fail.json +++ b/recipe_modules/bot_update/example.expected/tryjob_fail.json @@ -18,8 +18,12 @@ "{\"src\": \"got_cr_revision\"}", "--git-cache-dir", "[GIT_CACHE]", - "--patch_url", - "http://src.chromium.org/foo/bar", + "--issue", + "12345", + "--patchset", + "654321", + "--rietveld_server", + "https://rietveld.example.com/", "--output_json", "/path/to/tmp/json", "--revision", diff --git a/recipe_modules/bot_update/example.expected/tryjob_fail_patch.json b/recipe_modules/bot_update/example.expected/tryjob_fail_patch.json index 5b1e89cdc..764ae322b 100644 --- a/recipe_modules/bot_update/example.expected/tryjob_fail_patch.json +++ b/recipe_modules/bot_update/example.expected/tryjob_fail_patch.json @@ -18,8 +18,12 @@ "{\"src\": \"got_cr_revision\"}", "--git-cache-dir", "[GIT_CACHE]", - "--patch_url", - "http://src.chromium.org/foo/bar", + "--issue", + "12345", + "--patchset", + "654321", + "--rietveld_server", + "https://rietveld.example.com/", "--output_json", "/path/to/tmp/json", "--revision", diff --git a/recipe_modules/bot_update/example.expected/tryjob_fail_patch_download.json b/recipe_modules/bot_update/example.expected/tryjob_fail_patch_download.json index 0418ae59d..841dbcfed 100644 --- a/recipe_modules/bot_update/example.expected/tryjob_fail_patch_download.json +++ b/recipe_modules/bot_update/example.expected/tryjob_fail_patch_download.json @@ -18,8 +18,12 @@ "{\"src\": \"got_cr_revision\"}", "--git-cache-dir", "[GIT_CACHE]", - "--patch_url", - "http://src.chromium.org/foo/bar", + "--issue", + "12345", + "--patchset", + "654321", + "--rietveld_server", + "https://rietveld.example.com/", "--output_json", "/path/to/tmp/json", "--revision", diff --git a/recipe_modules/bot_update/example.expected/tryjob_v8.json b/recipe_modules/bot_update/example.expected/tryjob_v8.json index 3d9c1f8fd..9cae37c20 100644 --- a/recipe_modules/bot_update/example.expected/tryjob_v8.json +++ b/recipe_modules/bot_update/example.expected/tryjob_v8.json @@ -18,8 +18,12 @@ "{\"src\": \"got_cr_revision\"}", "--git-cache-dir", "[GIT_CACHE]", - "--patch_url", - "http://src.chromium.org/foo/bar", + "--issue", + "12345", + "--patchset", + "654321", + "--rietveld_server", + "https://rietveld.example.com/", "--output_json", "/path/to/tmp/json", "--revision", diff --git a/recipe_modules/bot_update/example.py b/recipe_modules/bot_update/example.py index 79787f358..a369997c7 100644 --- a/recipe_modules/bot_update/example.py +++ b/recipe_modules/bot_update/example.py @@ -84,7 +84,7 @@ def GenTests(api): slavename='totallyaslave-c4', issue=12345, patchset=654321, - patch_url='http://src.chromium.org/foo/bar' + rietveld='https://rietveld.example.com/', ) yield api.test('trychange') + api.properties( mastername='tryserver.chromium.linux', @@ -104,7 +104,7 @@ def GenTests(api): slavename='totallyaslave-c4', issue=12345, patchset=654321, - patch_url='http://src.chromium.org/foo/bar', + rietveld='https://rietveld.example.com/', ) + api.step_data('bot_update', retcode=1) yield api.test('tryjob_fail_patch') + api.properties( mastername='tryserver.chromium.linux', @@ -112,7 +112,7 @@ def GenTests(api): slavename='totallyaslave-c4', issue=12345, patchset=654321, - patch_url='http://src.chromium.org/foo/bar', + rietveld='https://rietveld.example.com/', fail_patch='apply', ) + api.step_data('bot_update', retcode=88) yield api.test('tryjob_fail_patch_download') + api.properties( @@ -121,7 +121,7 @@ def GenTests(api): slavename='totallyaslave-c4', issue=12345, patchset=654321, - patch_url='http://src.chromium.org/foo/bar', + rietveld='https://rietveld.example.com/', fail_patch='download' ) + api.step_data('bot_update', retcode=87) yield api.test('forced') + api.properties( @@ -172,7 +172,7 @@ def GenTests(api): slavename='totallyaslave-c4', issue=12345, patchset=654321, - patch_url='http://src.chromium.org/foo/bar', + rietveld='https://rietveld.example.com/', patch_project='v8', revisions={'src/v8': 'abc'} ) diff --git a/recipe_modules/bot_update/resources/bot_update.py b/recipe_modules/bot_update/resources/bot_update.py index d3e9dbef4..b69b49ffd 100755 --- a/recipe_modules/bot_update/resources/bot_update.py +++ b/recipe_modules/bot_update/resources/bot_update.py @@ -1006,35 +1006,6 @@ def parse_diff(diff): return result -def get_svn_patch(patch_url): - """Fetch patch from patch_url, return list of (filename, diff)""" - svn_exe = 'svn.bat' if sys.platform.startswith('win') else 'svn' - patch_data = call(svn_exe, 'cat', patch_url) - return parse_diff(patch_data) - - -def apply_svn_patch(patch_root, patches, whitelist=None, blacklist=None): - """Expects a list of (filename, diff), applies it on top of patch_root.""" - if whitelist: - patches = [(name, diff) for name, diff in patches if name in whitelist] - elif blacklist: - patches = [(name, diff) for name, diff in patches if name not in blacklist] - diffs = [diff for _, diff in patches] - patch = ''.join(diffs) - - if patch: - print '===Patching files===' - for filename, _ in patches: - print 'Patching %s' % filename - try: - call(PATCH_TOOL, '-p0', '--remove-empty-files', '--force', '--forward', - stdin_data=patch, cwd=patch_root, tries=1) - for filename, _ in patches: - full_filename = path.abspath(path.join(patch_root, filename)) - git('add', full_filename, cwd=path.dirname(full_filename)) - except SubprocessFailed as e: - raise PatchFailed(e.message, e.code, e.output) - def apply_rietveld_issue(issue, patchset, root, server, _rev_map, _revision, email_file, key_file, whitelist=None, blacklist=None): apply_issue_bin = ('apply_issue.bat' if sys.platform.startswith('win') @@ -1265,7 +1236,7 @@ def ensure_deps_revisions(deps_url_mapping, solutions, revisions): def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, - patch_root, issue, patchset, patch_url, rietveld_server, + patch_root, issue, patchset, rietveld_server, gerrit_repo, gerrit_ref, gerrit_rebase_patch_ref, revision_mapping, apply_issue_email_file, apply_issue_key_file, buildspec, gyp_env, shallow, runhooks, @@ -1277,10 +1248,6 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, git_ref = git_checkout(solutions, revisions, shallow, refs, git_cache_dir) - patches = None - if patch_url: - patches = get_svn_patch(patch_url) - print '===Processing patch solutions===' already_patched = [] patch_root = patch_root or '' @@ -1293,10 +1260,7 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, relative_root = solution['name'][len(patch_root) + 1:] target = '/'.join([relative_root, 'DEPS']).lstrip('/') print ' relative root is %r, target is %r' % (relative_root, target) - if patches: - apply_svn_patch(patch_root, patches, whitelist=[target]) - already_patched.append(target) - elif issue: + if issue: apply_rietveld_issue(issue, patchset, patch_root, rietveld_server, revision_mapping, git_ref, apply_issue_email_file, apply_issue_key_file, whitelist=[target]) @@ -1332,9 +1296,7 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, ensure_deps_revisions(gclient_output.get('solutions', {}), dir_names, revisions) # Apply the rest of the patch here (sans DEPS) - if patches: - apply_svn_patch(patch_root, patches, blacklist=already_patched) - elif issue: + if issue: apply_rietveld_issue(issue, patchset, patch_root, rietveld_server, revision_mapping, git_ref, apply_issue_email_file, apply_issue_key_file, blacklist=already_patched) @@ -1412,7 +1374,6 @@ def parse_args(): parse.add_option('--apply_issue_key_file', help='--private-key-file option passthrough for ' 'apply_patch.py.') - parse.add_option('--patch_url', help='Optional URL to SVN patch.') parse.add_option('--root', dest='patch_root', help='DEPRECATED: Use --patch_root.') parse.add_option('--patch_root', help='Directory to patch on top of.') @@ -1581,7 +1542,6 @@ def checkout(options, git_slns, specs, buildspec, master, patch_root=options.patch_root, issue=options.issue, patchset=options.patchset, - patch_url=options.patch_url, rietveld_server=options.rietveld_server, gerrit_repo=options.gerrit_repo, gerrit_ref=options.gerrit_ref,