bot_update: Don't use apply_patch_on_gclient.

It has been True by default for a while, and there is no need to override it.

Bug: 891917
Change-Id: I2598a2230b0ea38a647a533757331c541b871971
Reviewed-on: https://chromium-review.googlesource.com/c/1260057Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

recipes/README.recipes.md

@@ -47,16 +47,16 @@ Recipe module to ensure a checkout is consistent on a bot.
 
 #### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#10)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
 
-— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#27)(self, name, cmd, \*\*kwargs):**
+— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#25)(self, name, cmd, \*\*kwargs):**
 
 Wrapper for easy calling of bot_update.
 
-— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#413)(self, bot_update_step):**
+— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#409)(self, bot_update_step):**
 
 Deapplies a patch, taking care of DEPS and solution revisions properly.
     
 
-— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#68)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, refs=None, patch_oauth2=None, oauth2_json=None, use_site_config_creds=None, clobber=False, root_solution_revision=None, rietveld=None, issue=None, patchset=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, disable_syntax_validation=False, manifest_name=None, patch_refs=None, ignore_input_commit=False, \*\*kwargs):**
+— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#66)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, refs=None, patch_oauth2=None, oauth2_json=None, use_site_config_creds=None, clobber=False, root_solution_revision=None, rietveld=None, issue=None, patchset=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, disable_syntax_validation=False, manifest_name=None, patch_refs=None, ignore_input_commit=False, \*\*kwargs):**
 
 Args:
   gclient_config: The gclient configuration to use when running bot_update.
@@ -67,7 +67,7 @@ Args:
   manifest_name: The name of the manifest to upload to LogDog.  This must
     be unique for the whole build.
 
-— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#390)(self, project_name, gclient_config=None):**
+— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#386)(self, project_name, gclient_config=None):**
 
 Returns all property names used for storing the checked-out revision of
 a given project.
@@ -81,9 +81,9 @@ Args:
 Returns (list of str): All properties that'll hold the checked-out revision
     of the given project. An empty list if no such properties exist.
 
-— **def [initialize](/recipes/recipe_modules/bot_update/api.py#22)(self):**
+— **def [initialize](/recipes/recipe_modules/bot_update/api.py#20)(self):**
 
-&emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#36)(self):**
+&emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#34)(self):**
 ### *recipe_modules* / [cipd](/recipes/recipe_modules/cipd)
 
 [DEPS](/recipes/recipe_modules/cipd/__init__.py#1): [infra\_paths](#recipe_modules-infra_paths), [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]

recipes/recipe_modules/bot_update/api.py

@@ -11,8 +11,6 @@ class BotUpdateApi(recipe_api.RecipeApi):
 
   def __init__(self, properties, deps_revision_overrides, fail_patch, *args,
                **kwargs):
-    self._apply_patch_on_gclient = properties.get(
-        'apply_patch_on_gclient', True)
     self._deps_revision_overrides = deps_revision_overrides
     self._fail_patch = fail_patch
 
@@ -227,8 +225,6 @@ class BotUpdateApi(recipe_api.RecipeApi):
       cmd.append('--gerrit_no_rebase_patch_ref')
     if disable_syntax_validation or cfg.disable_syntax_validation:
       cmd.append('--disable-syntax-validation')
-    if not self._apply_patch_on_gclient:
-      cmd.append('--no-apply-patch-on-gclient')
 
     # Inject Json output for testing.
     first_sln = cfg.solutions[0].name

recipes/recipe_modules/bot_update/examples/full.expected/no_apply_patch_on_gclient.json

@@ -61,8 +61,7 @@
       "src@HEAD",
       "--revision",
       "src/third_party/angle@HEAD",
-      "--disable-syntax-validation",
-      "--no-apply-patch-on-gclient"
+      "--disable-syntax-validation"
     ],
     "env_prefixes": {
       "PATH": [
@@ -162,8 +161,7 @@
       "--revision",
       "src@f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9",
       "--revision",
-      "src/third_party/angle@fac9503c46405f77757b9a728eb85b8d7bc6080c",
-      "--no-apply-patch-on-gclient"
+      "src/third_party/angle@fac9503c46405f77757b9a728eb85b8d7bc6080c"
     ],
     "env_prefixes": {
       "PATH": [

recipes/recipe_modules/bot_update/examples/full.py

@@ -184,8 +184,7 @@ def GenTests(api):
   )
   yield (
       api.test('no_apply_patch_on_gclient') +
-      try_build(git_repo='https://chromium.googlesource.com/angle/angle') +
-      api.bot_update.properties(apply_patch_on_gclient=False)
+      try_build(git_repo='https://chromium.googlesource.com/angle/angle')
   )
   yield (
       api.test('tryjob_gerrit_v8_feature_branch') +

recipes/recipe_modules/bot_update/resources/bot_update.py

@@ -345,7 +345,7 @@ def git_config_if_not_set(key, value):
 def gclient_sync(
     with_branch_heads, with_tags, revisions, break_repo_locks,
     disable_syntax_validation, patch_refs, gerrit_reset,
-    gerrit_rebase_patch_ref, apply_patch_on_gclient):
+    gerrit_rebase_patch_ref):
   # We just need to allocate a filename.
   fd, gclient_output_file = tempfile.mkstemp(suffix='.json')
   os.close(fd)
@@ -366,7 +366,7 @@ def gclient_sync(
       revision = 'origin/master'
     args.extend(['--revision', '%s@%s' % (name, revision)])
 
-  if apply_patch_on_gclient and patch_refs:
+  if patch_refs:
     for patch_ref in patch_refs:
       args.extend(['--patch-ref', patch_ref])
     if not gerrit_reset:
@@ -379,7 +379,7 @@ def gclient_sync(
   except SubprocessFailed as e:
     # If gclient sync is handling patching, parse the output for a patch error
     # message.
-    if apply_patch_on_gclient and 'Failed to apply patch.' in e.output:
+    if 'Failed to apply patch.' in e.output:
       raise PatchFailed(e.message, e.code, e.output)
     # Throw a GclientSyncFailed exception so we can catch this independently.
     raise GclientSyncFailed(e.message, e.code, e.output)
@@ -816,8 +816,7 @@ def emit_json(out_file, did_run, gclient_output=None, **kwargs):
 def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only,
                     target_cpu, patch_root, patch_refs,
                     gerrit_rebase_patch_ref, refs, git_cache_dir,
-                    cleanup_dir, gerrit_reset, disable_syntax_validation,
-                    apply_patch_on_gclient):
+                    cleanup_dir, gerrit_reset, disable_syntax_validation):
   # Get a checkout of each solution, without DEPS or hooks.
   # Calling git directly because there is no way to run Gclient without
   # invoking DEPS.
@@ -825,19 +824,6 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only,
 
   git_checkouts(solutions, revisions, refs, git_cache_dir, cleanup_dir)
 
-  applied_gerrit_patch = False
-  if not apply_patch_on_gclient:
-    print '===Processing patch solutions==='
-    patch_root = patch_root or ''
-    print 'Patch root is %r' % patch_root
-    for solution in solutions:
-      print 'Processing solution %r' % solution['name']
-      if (patch_root == solution['name'] or
-          solution['name'].startswith(patch_root + '/')):
-        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)
-
   # Ensure our build/ directory is set up with the correct .gclient file.
   gclient_configure(solutions, target_os, target_os_only, target_cpu,
                     git_cache_dir)
@@ -869,8 +855,7 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only,
           disable_syntax_validation,
           patch_refs,
           gerrit_reset,
-          gerrit_rebase_patch_ref,
-          apply_patch_on_gclient)
+          gerrit_rebase_patch_ref)
 
   # Now that gclient_sync has finished, we should revert any .DEPS.git so that
   # presubmit doesn't complain about it being modified.
@@ -972,10 +957,6 @@ def parse_args():
   parse.add_option(
       '--disable-syntax-validation', action='store_true',
       help='Disable validation of .gclient and DEPS syntax.')
-  parse.add_option('--no-apply-patch-on-gclient',
-                   dest='apply_patch_on_gclient', action='store_false',
-                   default=True,
-                   help='Patch the gerrit ref in gclient instead of here.')
 
   options, args = parse.parse_args()
 
@@ -1014,10 +995,6 @@ def parse_args():
         % (str(e),)
     )
 
-  if options.patch_refs:
-    if not options.apply_patch_on_gclient:
-      parse.error('--patch_ref cannot be used with --no-apply-patch-on-gclient')
-
   # Because we print CACHE_DIR out into a .gclient file, and then later run
   # eval() on it, backslashes need to be escaped, otherwise "E:\b\build" gets
   # parsed as "E:[\x08][\x08]uild".
@@ -1088,8 +1065,7 @@ def checkout(options, git_slns, specs, revisions, step_text):
           git_cache_dir=options.git_cache_dir,
           cleanup_dir=options.cleanup_dir,
           gerrit_reset=not options.gerrit_no_reset,
-          disable_syntax_validation=options.disable_syntax_validation,
-          apply_patch_on_gclient=options.apply_patch_on_gclient)
+          disable_syntax_validation=options.disable_syntax_validation)
       gclient_output = ensure_checkout(**checkout_parameters)
     except GclientSyncFailed:
       print 'We failed gclient sync, lets delete the checkout and retry.'

recipes/recipe_modules/bot_update/test_api.py

@@ -8,15 +8,6 @@ from recipe_engine import recipe_test_api
 
 
 class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
-  def properties(self, apply_patch_on_gclient):
-    ret = self.test(None)
-    ret.properties = {
-        '$depot_tools/bot_update': {
-            'apply_patch_on_gclient': apply_patch_on_gclient,
-        }
-    }
-    return ret
-
   def output_json(self, root, first_sln, revision_mapping, fail_patch=False,
                   fixed_revisions=None):
     """Deterministically synthesize json.output test data for gclient's
@@ -45,11 +36,11 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
     })
     output.update({
       'manifest': {
-				project_name: {
-					'repository': 'https://fake.org/%s.git' % project_name,
-					'revision': self.gen_revision(project_name),
-				}
-			for project_name in set(revision_mapping.values())}})
+          project_name: {
+              'repository': 'https://fake.org/%s.git' % project_name,
+              'revision': self.gen_revision(project_name),
+          }
+          for project_name in set(revision_mapping.values())}})
 
     output.update({
       'source_manifest': {

tests/bot_update_coverage_test.py

@@ -156,7 +156,6 @@ class BotUpdateUnittests(unittest.TestCase):
       'cleanup_dir': None,
       'gerrit_reset': None,
       'disable_syntax_validation': False,
-      'apply_patch_on_gclient': False,
   }
 
   def setUp(self):
@@ -209,11 +208,10 @@ class BotUpdateUnittests(unittest.TestCase):
     self.assertEquals(args[idx_third_revision+1], 'src/v8@deadbeef')
     return self.call.records
 
-  def testEnableGclientExperiment(self):
+  def testApplyPatchOnGclient(self):
     ref = 'refs/changes/12/345/6'
     repo = 'https://chromium.googlesource.com/v8/v8'
     self.params['patch_refs'] = ['%s@%s' % (repo, ref)]
-    self.params['apply_patch_on_gclient'] = True
     bot_update.ensure_checkout(**self.params)
     args = self.gclient.records[0]
     idx = args.index('--patch-ref')
@@ -228,7 +226,6 @@ class BotUpdateUnittests(unittest.TestCase):
     self.params['patch_refs'] = [
         'https://chromium.googlesource.com/chromium/src@refs/changes/12/345/6',
         'https://chromium.googlesource.com/v8/v8@refs/changes/1/234/56']
-    self.params['apply_patch_on_gclient'] = True
     bot_update.ensure_checkout(**self.params)
     args = self.gclient.records[0]
     patch_refs = set(