From 0c3bd49069d44dd6c217f3db103d27bfc1272424 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Fri, 5 Oct 2018 17:05:25 +0000 Subject: [PATCH] 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/1260057 Reviewed-by: Andrii Shyshkalov Commit-Queue: Edward Lesmes --- recipes/README.recipes.md | 12 +++---- recipes/recipe_modules/bot_update/api.py | 4 --- .../no_apply_patch_on_gclient.json | 6 ++-- .../bot_update/examples/full.py | 3 +- .../bot_update/resources/bot_update.py | 36 ++++--------------- recipes/recipe_modules/bot_update/test_api.py | 19 +++------- tests/bot_update_coverage_test.py | 5 +-- 7 files changed, 21 insertions(+), 64 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 78aa65a04d..8c91bdb5bf 100644 --- a/recipes/README.recipes.md +++ b/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):** -  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#36)(self):** +  **@property**
— **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] diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 5754ef2231..a19d8ece8f 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/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 diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/no_apply_patch_on_gclient.json b/recipes/recipe_modules/bot_update/examples/full.expected/no_apply_patch_on_gclient.json index 93b6060b85..d1f059c670 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/no_apply_patch_on_gclient.json +++ b/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": [ diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index 01a2c1581e..0b3e90161c 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/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') + diff --git a/recipes/recipe_modules/bot_update/resources/bot_update.py b/recipes/recipe_modules/bot_update/resources/bot_update.py index 2775e84c3d..fdc777e041 100755 --- a/recipes/recipe_modules/bot_update/resources/bot_update.py +++ b/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.' diff --git a/recipes/recipe_modules/bot_update/test_api.py b/recipes/recipe_modules/bot_update/test_api.py index 54213d7498..7851d17460 100644 --- a/recipes/recipe_modules/bot_update/test_api.py +++ b/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': { diff --git a/tests/bot_update_coverage_test.py b/tests/bot_update_coverage_test.py index d0778344da..c83dee49ac 100755 --- a/tests/bot_update_coverage_test.py +++ b/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(