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

@@ -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"),

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],

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",

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",

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",

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",

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",

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'}
   )

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,