diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 2aad6883c9..7031b5f480 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -58,18 +58,18 @@ Recipe module to ensure a checkout is consistent on a bot. -#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#74)([RecipeApi][recipe_engine/wkt/RecipeApi]):** +#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#73)([RecipeApi][recipe_engine/wkt/RecipeApi]):** -— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#83)(self, name, cmd, \*\*kwargs):** +— **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#82)(self, name, cmd, \*\*kwargs):** Wrapper for easy calling of bot_update. -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#683)(self, bot_update_result):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#682)(self, bot_update_result):** Deapplies a patch, taking care of DEPS and solution revisions properly. -— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#194)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, step_tags=None, clean_ignored=False, \*\*kwargs):** +— **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#193)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, step_tags=None, clean_ignored=False, \*\*kwargs):** Args: * gclient_config: The gclient configuration to use when running bot_update. @@ -107,7 +107,7 @@ Args: * step_tags: a dict {tag name: tag value} of tags to add to the step * clean_ignored: If True, also clean ignored files from the checkout. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#660)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#659)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -121,14 +121,14 @@ 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. -  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#102)(self):** +  **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#101)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#611)(self, bot_update_result, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#610)(self, bot_update_result, name):** Sets a fixed revision for a single dependency using project revision properties. -— **def [step\_name](/recipes/recipe_modules/bot_update/api.py#700)(self, patch, suffix):** +— **def [step\_name](/recipes/recipe_modules/bot_update/api.py#699)(self, patch, suffix):** ### *recipe_modules* / [depot\_tools](/recipes/recipe_modules/depot_tools) [DEPS](/recipes/recipe_modules/depot_tools/__init__.py#6): [recipe\_engine/cipd][recipe_engine/recipe_modules/cipd], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime] diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index 381862b0c7..05231c037c 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -13,7 +13,6 @@ from recipe_engine.engine_types import StepPresentation from PB.go.chromium.org.luci.buildbucket.proto import common as common_pb2 - @dataclasses.dataclass(kw_only=True, frozen=True) class RelativeRoot: """A root that is relative to the checkout root. @@ -338,8 +337,8 @@ class BotUpdateApi(recipe_api.RecipeApi): # Guarantee that first solution has a revision. # TODO(machenbach): We should explicitly pass HEAD for ALL solutions # that don't specify anything else. - first_sol = cfg.solutions[0].name - revisions[first_sol] = revisions.get(first_sol) or 'HEAD' + first_sln = cfg.solutions[0].name + revisions[first_sln] = revisions.get(first_sln) or 'HEAD' if cfg.revisions: # Only update with non-empty values. Some recipe might otherwise @@ -347,7 +346,7 @@ class BotUpdateApi(recipe_api.RecipeApi): revisions.update( (k, v) for k, v in cfg.revisions.items() if v) if cfg.solutions and root_solution_revision: - revisions[first_sol] = root_solution_revision + revisions[first_sln] = root_solution_revision # Allow for overrides required to bisect into rolls. revisions.update(self._deps_revision_overrides) @@ -419,12 +418,12 @@ class BotUpdateApi(recipe_api.RecipeApi): cmd.append('--clean-ignored') # Inject Json output for testing. - first_sln = cfg.solutions[0].name step_test_data = step_test_data or (lambda: self.test_api.output_json( - first_sln, - reverse_rev_map, - patch_root=patch_root, + first_sln=first_sln, + revisions=self._test_data.get('revisions', {}), fixed_revisions=fixed_revisions, + got_revision_mapping=reverse_rev_map, + patch_root=patch_root, fail_checkout=self._test_data.get('fail_checkout', False), fail_patch=self._test_data.get('fail_patch', False), commit_positions=self._test_data.get('commit_positions', True), diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json b/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json index bcefced0d4..c742cd9140 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json +++ b/recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json @@ -135,8 +135,7 @@ "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\",@@@", "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\",@@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\",@@@", - "@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@", "@@@STEP_LOG_LINE@json.output@ },@@@", "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\",@@@", "@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@", @@ -160,8 +159,7 @@ "@@@STEP_LOG_LINE@json.output@}@@@", "@@@STEP_LOG_END@json.output@@@", "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@" + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@" ] }, { @@ -235,8 +233,7 @@ "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\",@@@", "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\",@@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\",@@@", - "@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@", "@@@STEP_LOG_LINE@json.output@ },@@@", "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\",@@@", "@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@", diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index 5e2fe7a992..3db82ae42f 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -100,6 +100,7 @@ def GenTests(api): def ci_build(**kwargs): kwargs.setdefault( 'git_repo', 'https://chromium.googlesource.com/chromium/src') + kwargs.setdefault('revision', '2d72510e447ab60a9728aeea2362d8be2cbd7789') return ( api.buildbucket.ci_build('chromium/src', 'ci', 'linux', **kwargs) + api.properties(patch=False) @@ -262,10 +263,11 @@ def GenTests(api): # attempts to do comparisons on the revision value api.step_data( 'bot_update (without patch)', - api.bot_update.output_json(patch_root='src', - first_sln='src', - revision_mapping={'got_revision': 'src'}, - commit_positions=False))) + api.bot_update.output_json( + patch_root='src', + first_sln='src', + got_revision_mapping={'got_revision': 'src'}, + commit_positions=False))) yield api.test( 'upload_traces', @@ -342,3 +344,65 @@ def GenTests(api): {'$depot_tools/bot_update': { 'stale_process_duration_override': 3000, }}) + ci_build()) + + def check_revisions(check, output_json): + preconditions_valid = ( + check(output_json['fixed_revisions']['infra'] == 'HEAD') + and check(output_json['fixed_revisions']['src'] == + '2d72510e447ab60a9728aeea2362d8be2cbd7789')) + if not preconditions_valid: + return # pragma: no cover + + manifest = output_json['manifest'] + # An empty revision specified for a project where there is a fixed revision + # uses the fixed revision + check(manifest['src']['revision'] == + '2d72510e447ab60a9728aeea2362d8be2cbd7789') + # An empty revision specified for a project where there is not a fixed + # revision will generate a revision for 'HEAD' + check(manifest['src/foo']['revision'] == api.bot_update.gen_revision( + 'src/foo')) + # A non-empty revision specified for a project where there is not a fixed + # revision will use the revision (generating an actual revision for a ref) + check(manifest['src/v8']['revision'] == api.bot_update.gen_revision( + 'src/v8@refs/bar')) + # A non-empty revision specified for a project where there is a fixed + # revision with the same value will use the revision (generating an actual + # revision for a ref) + check(manifest['infra']['revision'] == api.bot_update.gen_revision('infra')) + + yield api.test( + 'revisions', + ci_build(), + api.bot_update.revisions({ + 'src': '', + 'src/foo': '', + 'src/v8': 'refs/bar', + 'infra': 'HEAD', + }), + api.bot_update.post_check_output_json('bot_update (without patch)', + check_revisions), + api.post_process(post_process.DropExpectation), + ) + + def check_revisions_overriding_generation(check, output_json): + # Precondition check + if not check(output_json['fixed_revisions']['infra'] == 'HEAD'): + return # pragma: no cover + + manifest = output_json['manifest'] + # A non-empty revision for a project where there is a fixed revision that is + # different is okay if the fixed revision would be replaced with a generated + # value: the specified revision will be used + check(manifest['infra']['revision'] == 'a' * 40) + + yield api.test( + 'revisions-overriding-revision-generation', + ci_build(), + api.bot_update.revisions({ + 'infra': 'a' * 40, + }), + api.bot_update.post_check_output_json( + 'bot_update (without patch)', check_revisions_overriding_generation), + api.post_process(post_process.DropExpectation), + ) diff --git a/recipes/recipe_modules/bot_update/test_api.py b/recipes/recipe_modules/bot_update/test_api.py index 5152d075ae..1eda162c10 100644 --- a/recipes/recipe_modules/bot_update/test_api.py +++ b/recipes/recipe_modules/bot_update/test_api.py @@ -4,6 +4,7 @@ import collections.abc import hashlib +import json import struct import typing @@ -12,6 +13,11 @@ from recipe_engine import recipe_test_api class BotUpdateTestApi(recipe_test_api.RecipeTestApi): + @recipe_test_api.mod_test_data + @staticmethod + def revisions(val: dict[str, str]): + return val + @recipe_test_api.mod_test_data @staticmethod def fail_checkout(val: bool): @@ -29,30 +35,59 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): def output_json( self, - first_sln: str, - revision_mapping: collections.abc.Mapping[str, str], *, - patch_root: str | None = None, + first_sln: str | None = None, + revisions: collections.abc.Mapping[str, str] | None = None, fixed_revisions: collections.abc.Mapping[str, str] | None = None, + got_revision_mapping: collections.abc.Mapping[str, str] | None = None, + patch_root: str | None = None, fail_checkout: bool = False, fail_patch: bool | typing.Literal['download'] = False, commit_positions: bool = True, ): """Synthesized json output for bot_update.py. + Revision values will be replaced with generated values if they are + equal to 'HEAD' or if they are a ref (starts with refs/ or origin/). + Args: first_sln: The name of the first solution in the gclient config. - revision_mapping: A mapping from property name to project - name/checkout-relative repo path. The resultant json will have - the given property set to the revision of that project. - patch_root: The relative path within the checkout to where the - patch should be applied. + This will apear in the resultant json's root value. If not + provided, then the first project that appears in revisions will + be used. An entry for this will appear in the resultant json's + manifest even if it doesn't appear in revisions, fixed_revisions + or got_revision_mapping. + revisions: A mapping from project name/checkout-relative repo path + to the revision that will appear in the manifests in the + resultant json. Any projects present in fixed_revisions or + got_revision_mapping will also result in entries in the + manifests. If non-empty, the resultant json's root value will be + the first project. An empty value for a project indicates to + generate a revision. If first_sln isn't specified, then the + first project will be used as first_sln. Dicts have a guaranteed + iteration order based on declaration/insertion order, but other + mappings may not, so they may produce inconsistent results if + first_sln isn't specified. fixed_revisions: A mapping from project name/checkout-relative repo path to the revisions to be used for the project. The - revisions can be either refs or commit hashes. The manifests in - the resultant json will have refs replaced with generated - revision values. The provided revisions will be reported as-is - in the fixed_revisions of the resultant json. + provided revisions will be reported as-is in the fixed_revisions + of the resultant json. The resultant json's manifests will + include entries for any project's not present in revisions. If + non-empty and revisions is empty or None, the resultant json's + root value will be the first project. It is an error if an entry + exists for a project in revisions unless the values are equal, + the value in revisions is empty or the value in fixed_revisions + would be replaced with a generated value and the value in + revisions would not be. + got_revision_mapping: A mapping from property name to project + name/checkout-relative repo path. The resultant json will have + the given property set to the revision of that project. The + resultant json's manifests will include entries for any projects + not present in revisions. If non-empty and revisions and + fixed_revisions are both empty or None, the resultant json's + root value will be the first project. + patch_root: The relative path within the checkout to where the + patch should be applied. fail_checkout: Whether or not the simulated checkout should fail. fail_patch: Whether or not the checkout should fail to apply the patch. A value of 'download' can be provided to indicate a @@ -69,44 +104,85 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): 'did_run': True, } - fixed_revisions = fixed_revisions or {} - if fail_checkout: t.retcode = 1 else: - revisions = { - project_name: self.gen_revision(project_name) - for project_name in set(revision_mapping.values()) - } - if fixed_revisions: - for project_name, revision in fixed_revisions.items(): - if revision == 'HEAD': - revision = self.gen_revision(project_name) - elif revision.startswith('refs/') or revision.startswith('origin/'): - revision = self.gen_revision('{}@{}'.format(project_name, revision)) - revisions[project_name] = revision + assert revisions or fixed_revisions or got_revision_mapping, ( + 'a non-empty value must be provided for at least one of' + ' revisions, fixed_revisions or got_revision_mapping') + assert first_sln or revisions, ( + 'a non-empty value must be provided for first_sln or revisions') - properties = { - property_name: revisions[project_name] - for property_name, project_name in revision_mapping.items() - } - properties.setdefault('got_revision', self.gen_revision(first_sln)) - if commit_positions: + revisions = revisions or {} + first_sln = first_sln or next(iter(revisions)) + fixed_revisions = fixed_revisions or {} + # If no got_revision_mapping is specified, then bot_update.py will set the + # got_revision property for the first solution. It also has some default + # properties mapped in GOT_REVISION_MAPPINGS that isn't handled here for + # test data + got_revision_mapping = got_revision_mapping or {'got_revision': first_sln} - def get_ref(project_name): - revision = fixed_revisions.get(project_name, 'HEAD') - if revision.startswith('origin/'): - return revision.replace('origin/', 'refs/heads/', 1) - if revision.startswith('refs/'): + def resolve_revision(project_name, revision): + if revision == 'HEAD': + return self.gen_revision(project_name) + if revision.startswith('refs/') or revision.startswith('origin/'): + return self.gen_revision('{}@{}'.format(project_name, revision)) + return revision + + def choose_revision(project_name): + fixed_revision = (fixed_revisions or {}).get(project_name) + assert fixed_revision is None or fixed_revision, ( + f'empty fixed_revision provided for {project_name}') + revision = revisions.get(project_name) + match (revision, fixed_revision): + case ('', _): + return resolve_revision(project_name, fixed_revision or 'HEAD') + case (_, None): + return resolve_revision(project_name, revision) + case (_, _) if revision == fixed_revision: + return resolve_revision(project_name, revision) + case _, _: + + def will_generate(rev): + return resolve_revision(project_name, rev) != rev + + # If the revision and fixed_revision are different, then the fixed + # revision must be HEAD or a ref and revision must not be and the + # revision will be treated as the revision of the commit that HEAD + # or the ref point at + assert ( + will_generate(fixed_revision) and not will_generate(revision) + ), f'{project_name} revision and fixed_revision are different' return revision - return 'refs/heads/main' - properties.update({ - '%s_cp' % property_name: - ('%s@{#%s}' % - (get_ref(project_name), self.gen_commit_position(project_name))) - for property_name, project_name in revision_mapping.items() - }) + resolved_revisions = { + project_name: choose_revision(project_name) + for project_name in revisions + } + + for project_name, fixed_revision in fixed_revisions.items(): + if project_name not in resolved_revisions: + resolved_revisions[project_name] = resolve_revision( + project_name, fixed_revision) + + resolved_revisions.setdefault(first_sln, self.gen_revision(first_sln)) + + properties = {} + for property_name, project_name in got_revision_mapping.items(): + if project_name not in resolved_revisions: + resolved_revisions[project_name] = self.gen_revision(project_name) + properties[property_name] = resolved_revisions[project_name] + if commit_positions: + fixed_revision = (fixed_revisions.get(project_name) + or revisions.get(project_name) or 'HEAD') + if fixed_revision.startswith('origin/'): + ref = fixed_revision.replace('origin/', 'refs/heads/', 1) + elif fixed_revision.startswith('refs/'): + ref = fixed_revision + else: + ref = 'refs/heads/main' + properties[f'{property_name}_cp'] = '%s@{#%s}' % ( + ref, self.gen_commit_position(project_name)) output.update({ 'patch_root': patch_root, @@ -121,7 +197,7 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): 'repository': 'https://fake.org/%s.git' % project_name, 'revision': revision, } - for project_name, revision in revisions.items() + for project_name, revision in sorted(resolved_revisions.items()) } }) @@ -135,7 +211,8 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): 'revision': revision } } - for project_name, revision in revisions.items() + for project_name, revision in sorted( + resolved_revisions.items()) } } }) @@ -179,3 +256,20 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi): """Hash project to bogus deterministic Cr-Commit-Position values.""" h = hashlib.sha1(project.encode('utf-8')) return struct.unpack('!I', h.digest()[:4])[0] % 300000 + + def post_check_output_json(self, step_name: str, custom_check_fn): + """Perform a post check on the output json of a bot_update step. + + Args: + step_name: The name of the bot_update step to check. + custom_check_fn: A function taking 2 positional arguments: the + "magic check" function provided by the recipe engine and the + deserialized bot_update output json. + """ + + def perform_post_check(check, steps): + bot_update_step = steps[step_name] + output_json = json.loads(bot_update_step.logs['json.output']) + custom_check_fn(check, output_json) + + return self.post_check(perform_post_check)