bot_update: Add a flag to move applying the patch to gclient.

This adds a --enable-gclient-experiment flags that tells bot_update.py
to skip applying the patch, and instead forward the flags to gclient.

Bug: 643346
Change-Id: Ia4275a126e6adba54dfcc894d224c50c166db90e
Reviewed-on: https://chromium-review.googlesource.com/962938
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

recipes/README.recipes.md

@@ -48,12 +48,12 @@ Wrapper for easy calling of bot_update.
 
 — **def [apply\_gerrit\_ref](/recipes/recipe_modules/bot_update/api.py#45)(self, root, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, gerrit_repo=None, gerrit_ref=None, step_name='apply_gerrit', \*\*kwargs):**
 
-— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#393)(self, bot_update_step):**
+— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#395)(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#67)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, no_shallow=False, 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, \*\*kwargs):**
+— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#67)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, no_shallow=False, 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, enable_gclient_experiment=False, \*\*kwargs):**
 
 Args:
   gclient_config: The gclient configuration to use when running bot_update.
@@ -64,7 +64,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#370)(self, project_name, gclient_config=None):**
+— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#372)(self, project_name, gclient_config=None):**
 
 Returns all property names used for storing the checked-out revision of
 a given project.

recipes/recipe_modules/bot_update/api.py

@@ -74,7 +74,7 @@ class BotUpdateApi(recipe_api.RecipeApi):
                       patchset=None, gerrit_no_reset=False,
                       gerrit_no_rebase_patch_ref=False,
                       disable_syntax_validation=False, manifest_name=None,
-                      **kwargs):
+                      enable_gclient_experiment=False, **kwargs):
     """
     Args:
       gclient_config: The gclient configuration to use when running bot_update.
@@ -205,6 +205,8 @@ 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 enable_gclient_experiment:
+      cmd.append('--enable-gclient-experiment')
 
     # Inject Json output for testing.
     first_sln = cfg.solutions[0].name

recipes/recipe_modules/bot_update/examples/full.py

@@ -51,6 +51,8 @@ def RunSteps(api):
       api.properties.get('gerrit_no_rebase_patch_ref'))
   manifest_name = api.properties.get('manifest_name')
 
+  enable_gclient_experiment = api.properties.get('enable_gclient_experiment')
+
   if api.properties.get('test_apply_gerrit_ref'):
     prop2arg = {
         'gerrit_custom_repo': 'gerrit_repo',
@@ -80,7 +82,8 @@ def RunSteps(api):
         gerrit_no_reset=gerrit_no_reset,
         gerrit_no_rebase_patch_ref=gerrit_no_rebase_patch_ref,
         disable_syntax_validation=True,
-        manifest_name=manifest_name)
+        manifest_name=manifest_name,
+        enable_gclient_experiment=enable_gclient_experiment)
     if patch:
       api.bot_update.deapply_patch(bot_update_step)
 
@@ -193,6 +196,12 @@ def GenTests(api):
       patch_issue=338811,
       patch_set=3,
   )
+  yield api.test('enable_gclient_experiment') + api.properties.tryserver(
+      gerrit_project='angle/angle',
+      patch_issue=338811,
+      patch_set=3,
+      enable_gclient_experiment=True,
+  )
   yield api.test('tryjob_gerrit_v8') + api.properties.tryserver(
       gerrit_project='v8/v8',
       patch_issue=338811,

recipes/recipe_modules/bot_update/resources/bot_update.py

@@ -51,6 +51,9 @@ COMMIT_ORIGINAL_POSITION_FOOTER_KEY = 'Cr-Original-Commit-Position'
 # Regular expression to parse gclient's revinfo entries.
 REVINFO_RE = re.compile(r'^([^:]+):\s+([^@]+)@(.+)$')
 
+# Regular expression to match gclient's patch error message.
+PATCH_ERROR_RE = re.compile('Failed to apply .* @ .* to .* at .*')
+
 # Copied from scripts/recipes/chromium.py.
 GOT_REVISION_MAPPINGS = {
     CHROMIUM_SRC_URL: {
@@ -327,7 +330,8 @@ def gclient_configure(solutions, target_os, target_os_only, target_cpu,
 
 def gclient_sync(
     with_branch_heads, with_tags, shallow, revisions, break_repo_locks,
-    disable_syntax_validation):
+    disable_syntax_validation, gerrit_repo, gerrit_ref, gerrit_reset,
+    gerrit_rebase_patch_ref, enable_gclient_experiment):
   # We just need to allocate a filename.
   fd, gclient_output_file = tempfile.mkstemp(suffix='.json')
   os.close(fd)
@@ -350,9 +354,22 @@ def gclient_sync(
       revision = 'origin/master'
     args.extend(['--revision', '%s@%s' % (name, revision)])
 
+  if enable_gclient_experiment:
+    # TODO(ehmaldonado): Merge gerrit_repo and gerrit_ref into a patch-ref flag
+    # and add support for passing multiple patch refs.
+    args.extend(['--patch-ref', gerrit_repo + '@' + gerrit_ref])
+    if not gerrit_reset:
+      args.append('--no-reset-patch-ref')
+    if not gerrit_rebase_patch_ref:
+      args.append('--no-rebase-patch-ref')
+
   try:
     call_gclient(*args)
   except SubprocessFailed as e:
+    # If gclient sync is handling patching, parse the output for a patch error
+    # message.
+    if enable_gclient_experiment and PATCH_ERROR_RE.search(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)
   else:
@@ -838,7 +855,8 @@ 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, gerrit_repo, gerrit_ref,
                     gerrit_rebase_patch_ref, shallow, refs, git_cache_dir,
-                    cleanup_dir, gerrit_reset, disable_syntax_validation):
+                    cleanup_dir, gerrit_reset, disable_syntax_validation,
+                    enable_gclient_experiment):
   # Get a checkout of each solution, without DEPS or hooks.
   # Calling git directly because there is no way to run Gclient without
   # invoking DEPS.
@@ -846,21 +864,22 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only,
 
   git_checkouts(solutions, revisions, shallow, refs, git_cache_dir, cleanup_dir)
 
-  print '===Processing patch solutions==='
-  patch_root = patch_root or ''
   applied_gerrit_patch = False
-  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)
-      if gerrit_ref:
-        apply_gerrit_ref(gerrit_repo, gerrit_ref, patch_root, gerrit_reset,
-                         gerrit_rebase_patch_ref)
-        applied_gerrit_patch = True
+  if not enable_gclient_experiment:
+    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)
+        if gerrit_ref:
+          apply_gerrit_ref(gerrit_repo, gerrit_ref, patch_root, gerrit_reset,
+                           gerrit_rebase_patch_ref)
+          applied_gerrit_patch = True
 
   # Ensure our build/ directory is set up with the correct .gclient file.
   gclient_configure(solutions, target_os, target_os_only, target_cpu,
@@ -888,7 +907,12 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only,
       shallow,
       gc_revisions,
       break_repo_locks,
-      disable_syntax_validation)
+      disable_syntax_validation,
+      gerrit_repo,
+      gerrit_ref,
+      gerrit_reset,
+      gerrit_rebase_patch_ref,
+      enable_gclient_experiment)
 
   # Now that gclient_sync has finished, we should revert any .DEPS.git so that
   # presubmit doesn't complain about it being modified.
@@ -896,7 +920,7 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only,
     git('checkout', 'HEAD', '--', '.DEPS.git', cwd=first_sln)
 
   # Apply the rest of the patch here (sans DEPS)
-  if gerrit_ref and not applied_gerrit_patch:
+  if gerrit_ref and not applied_gerrit_patch and not enable_gclient_experiment:
     # If gerrit_ref was for solution's main repository, it has already been
     # applied above. This chunk is executed only for patches to DEPS-ed in
     # git repositories.
@@ -1002,6 +1026,8 @@ def parse_args():
   parse.add_option(
       '--disable-syntax-validation', action='store_true',
       help='Disable validation of .gclient and DEPS syntax.')
+  parse.add_option('--enable-gclient-experiment', action='store_true',
+                   help='Patch the gerrit ref in gclient instead of here.')
 
   options, args = parse.parse_args()
 
@@ -1116,7 +1142,8 @@ def checkout(options, git_slns, specs, revisions, step_text, shallow):
           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)
+          disable_syntax_validation=options.disable_syntax_validation,
+          enable_gclient_experiment=options.enable_gclient_experiment)
       gclient_output = ensure_checkout(**checkout_parameters)
     except GclientSyncFailed:
       print 'We failed gclient sync, lets delete the checkout and retry.'

tests/bot_update_coverage_test.py

@@ -158,6 +158,7 @@ class BotUpdateUnittests(unittest.TestCase):
       'cleanup_dir': None,
       'gerrit_reset': None,
       'disable_syntax_validation': False,
+      'enable_gclient_experiment': False,
   }
 
   def setUp(self):
@@ -215,6 +216,22 @@ class BotUpdateUnittests(unittest.TestCase):
     self.assertEquals(args[idx_third_revision+1], 'src/v8@deadbeef')
     return self.call.records
 
+  def testEnableGclientExperiment(self):
+    self.params['gerrit_ref'] = 'refs/changes/12/345/6'
+    self.params['gerrit_repo'] = 'https://chromium.googlesource.com/v8/v8'
+    self.params['enable_gclient_experiment'] = True
+    bot_update.ensure_checkout(**self.params)
+    args = self.gclient.records[0]
+    idx = args.index('--patch-ref')
+    self.assertEqual(
+        args[idx+1],
+        self.params['gerrit_repo'] + '@' + self.params['gerrit_ref'])
+    self.assertNotIn('--patch-ref', args[idx+1:])
+    # Assert we're not patching in bot_update.py
+    for record in self.call.records:
+      self.assertNotIn('git fetch ' + self.params['gerrit_repo'],
+                       ' '.join(record[0]))
+
   def testBreakLocks(self):
     self.overrideSetupForWindows()
     bot_update.ensure_checkout(**self.params)