Roll chromium-targets Adds the chromium-targets Starlark package to the list of things synced by the Chromium -> Dawn autoroll script. This requires a fair bit of refactoring to share code with the existing logic for the chromium-luci syncing. This also includes a drive-by improvement to the script runtime. It turns out `git ls-remote ... HEAD` is much slower for Chromium than `git ls-remote --branches ... main`, but the two are functionally equivalent for what we need here. Bug: 485816035 Change-Id: I37628f9146716b0f45365f54ec7b3a0a5e12f499 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/292197 Reviewed-by: Jonathan Lee <jonathanjlee@google.com> Commit-Queue: Brian Sheedy <bsheedy@google.com> Auto-Submit: Brian Sheedy <bsheedy@google.com>
diff --git a/scripts/roll_chromium_deps.py b/scripts/roll_chromium_deps.py index 20a626e..e22652d 100755 --- a/scripts/roll_chromium_deps.py +++ b/scripts/roll_chromium_deps.py
@@ -36,6 +36,7 @@ import base64 import dataclasses import datetime +import functools import logging import pathlib import posixpath @@ -49,6 +50,8 @@ DAWN_ROOT = pathlib.Path(__file__).resolve().parents[1] DEPS_FILE = DAWN_ROOT / 'DEPS' +INFRA_PATH = DAWN_ROOT / 'infra' / 'config' / 'global' +PACKAGE_STAR = INFRA_PATH / 'PACKAGE.star' CHROMIUM_GOB_URL = 'https://chromium.googlesource.com' CHROMIUM_SRC_URL = posixpath.join(CHROMIUM_GOB_URL, 'chromium', 'src') @@ -56,19 +59,6 @@ DEFAULT_REVISION_CHARACTERS = 10 -# Used to extract/replace the git revision for the chromium-luci entry in a -# PACKAGE.star file. -CHROMIUM_LUCI_REVISION_PATTERN = re.compile( - # Group 1: Context prefix to ensure that we're matching the chromium-luci - # entry. - r'(name\s*=\s*"@chromium-luci".*?revision\s*=\s*")' - # Group 2: The hexadecimal revision to extract/replace. - r'([a-fA-F0-9]+)' - # Positive lookahead: Asserts that a " follows, but does not match/consume - # it. - r'(?=")', - re.DOTALL) - # GN variables that need to be synced. A map from Dawn variable name to # Chromium variable name. SYNCED_VARIABLES = {} @@ -454,8 +444,11 @@ cmd = [ 'git', 'ls-remote', + '--branches', remote_url, - 'HEAD', + # main and HEAD should be equivalent in this case and main allows usage + # of --branches, which significantly speeds this up. + 'main', ] proc = subprocess.run(cmd, check=True, capture_output=True, text=True) head_revision = proc.stdout.strip().split()[0] @@ -975,82 +968,171 @@ subprocess.run(cmd, check=True) -def _sync_chromium_luci_revision(chromium_revision: str) -> ChangedRepo: - """Syncs the chromium-luci revision used by //infra/config. +def _sync_starlark_packages(chromium_revision: str) -> list[ChangedRepo]: + """Syncs Starlark packages shared with Chromium. + + These are used by //infra/config and stored in + //infra/config/global/PACKAGE.star. + + Note: The returned entries are technically not DEPS entries (ChangedRepo + inherits from ChangedDepsEntry), but they similar enough that they can + treated as such as long as the returned ChangedRepos are not actually + applied to the DEPS file. Args: - chromium_revision: The Chromium revision to read the chromium-luci revision at. + chromium_revision: The Chromium revision to sync Starlark packages to. Returns: - A ChangedRepo specifying the old and new chromium-luci revisions. + A list of ChangedRepo specifying the old and new revisions for Starlark + packages. """ - infra_path = DAWN_ROOT / 'infra' / 'config' / 'global' - dawn_package = infra_path / 'PACKAGE.star' - with open(dawn_package, encoding='utf-8') as infile: - dawn_package_contents = infile.read() - - old_revision = _extract_chromium_luci_revision(dawn_package_contents) - if not old_revision: - raise RuntimeError( - "Unable to extract chromium-luci revision from Dawn's " - '//infra/config/global/PACKAGE.star') - chromium_package_contents = _read_remote_chromium_file( 'infra/config/PACKAGE.star', chromium_revision) - new_revision = _extract_chromium_luci_revision(chromium_package_contents) - if not new_revision: - raise RuntimeError( - "Unable to extract chromium-luci revision from Chromium's " - '//infra/config/PACKAGE.star') + chromium_luci_package = '@chromium-luci' + new_chromium_luci_revision = _extract_starlark_package_revision( + chromium_luci_package, chromium_package_contents) - # Replace the match with Group 1 (matched content before the revision) and - # the new revision itself. - dawn_package_contents = CHROMIUM_LUCI_REVISION_PATTERN.sub( - rf'\g<1>{new_revision}', dawn_package_contents) - with open(dawn_package, 'w', encoding='utf-8') as outfile: + with open(PACKAGE_STAR, encoding='utf-8') as infile: + dawn_package_contents = infile.read() + dawn_package_contents, old_chromium_luci_revision = ( + _exchange_starlark_package_revision(chromium_luci_package, + dawn_package_contents, + new_chromium_luci_revision)) + dawn_package_contents, old_chromium_targets_revision = ( + _exchange_starlark_package_revision('@chromium-targets', + dawn_package_contents, + chromium_revision)) + with open(PACKAGE_STAR, 'w', encoding='utf-8') as outfile: outfile.write(dawn_package_contents) - # We are able to generate files now unlike with + # Automatically stage any changes to be consistent with DEPS modifications. + _run_lucicfg_and_stage_changes() + + changed_packages = [ + ChangedRepo(name='chromium-luci (Starlark)', + url=posixpath.join(CHROMIUM_GOB_URL, 'infra', 'chromium'), + old_revision=old_chromium_luci_revision, + new_revision=new_chromium_luci_revision), + # The chromium-targets package is not reported since the revision is + # identical to Chromium's revision, which is already reported. + ] + + # Only report actual changes so that unchanged packages are omitted from the + # CL description. + return [p for p in changed_packages if p.old_revision != p.new_revision] + + +def _run_lucicfg_and_stage_changes() -> None: + """Runs lucicfg on Dawn's Starlark files and stages any changes.""" + # We are able to generate files as part of this script unlike with # infra/specs/generate_test_spec_json.py because this does not require a # `gclient sync` to pull in updated dependencies. subprocess.check_call( ['lucicfg', 'generate', - str(infra_path / 'main.star')], + str(INFRA_PATH / 'main.star')], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - # Automatically add any changed files to be consistent with DEPS - # modifications. - subprocess.check_call(['git', 'add', str(infra_path)], + subprocess.check_call(['git', 'add', str(INFRA_PATH)], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - # This is technically not a DEPS entry (ChangedRepo inherits from - # ChangedDepsEntry), but it's similar enough that it can treated as such. - return ChangedRepo(name='chromium-luci (Starlark)', - url=posixpath.join(CHROMIUM_GOB_URL, 'infra', - 'chromium'), - old_revision=old_revision, - new_revision=new_revision) - -def _extract_chromium_luci_revision(package_star_contents: str) -> str | None: - """Extracts the revision for chromium-luci from a PACKAGE.star file. +def _exchange_starlark_package_revision(package_name: str, + package_star_contents: str, + new_revision: str) -> tuple[str, str]: + """Exchanges the revision for a Starlark package. Args: - package_star_contents: A string containing the contents of a - PACKAGE.star file. + package_name: The Starlark package name to exchange the revision for, + including the leading @. + package_star_contents: The contents of a PACKAGE.star file to modify. + new_revision: The new revision to put in the file contents. Returns: - The git revision for the chromium-luci entry, or None if it could not - be extracted. + A tuple (exchanged_contents, old_revision). |exchanged_contents| is a + copy of |package_star_contents| with the revision for |package_name| + replaced with |new_revision|. |old_revision| is the revision for + |package_name| that was present before the exchange. """ - match = CHROMIUM_LUCI_REVISION_PATTERN.search(package_star_contents) + old_revision = _extract_starlark_package_revision(package_name, + package_star_contents) + package_star_contents = _replace_starlark_package_revision( + package_name, new_revision, package_star_contents) + return package_star_contents, old_revision + + +@functools.cache +def _get_starlark_package_regex_for(package_name: str) -> re.Pattern: + """Get a Pattern to match the given Starlark package definition. + + This Pattern is suitable for either extracting a revision or replacing it + with a new one. + + Args: + package_name: The Starlark package name to search for, including the + leading @. + + Returns: + A Pattern that matches |package_name| in PACKAGE.star. + """ + revision_pattern = re.compile( + # Group 1: Context prefix to ensure that we're matching the correct + # package entry. + rf'(name\s*=\s*"{package_name}".*?revision\s*=\s*")' + # Group 2: The hexadecimal revision to extract/replace. + r'([a-fA-F0-9]+)' + # Positive lookahead: Asserts that a " follows, but does not + # match/consume it. + r'(?=")', + re.DOTALL) + + return revision_pattern + + +def _extract_starlark_package_revision(package_name: str, + package_star_contents: str) -> str: + """Extract a Starlark package revision from PACKAGE.star content. + + Args: + package_name: The Starlark package name to search for, including the + leading @. + package_star_contents: The contents of a PACKAGE.star file to search. + + Returns: + The git revision of the requested package. + """ + revision_pattern = _get_starlark_package_regex_for(package_name) + match = revision_pattern.search(package_star_contents) if not match: - return None + raise RuntimeError( + f'Unable to extract {package_name} revision from PACKAGE.star ' + f'contents') return match.group(2) +def _replace_starlark_package_revision(package_name: str, new_revision: str, + package_star_contents: str) -> str: + """Replace a Starlark package revision in PACKAGE.star content. + + package_name: The Starlark package name to search for, including the + leading @. + new_revision: The new revision to update the package to. + package_star_contents: The contents of a PACKAGE.star file to + find/replace in. + + Returns: + A copy of |package_star_contents| with the revision for |package_name| + updated to |new_revision|. + """ + revision_pattern = _get_starlark_package_regex_for(package_name) + # Replace the match with Group 1 (matched content before the revision) and + # the new revision itself. + updated_contents = revision_pattern.sub(rf'\g<1>{new_revision}', + package_star_contents) + return updated_contents + + def _parse_args() -> argparse.Namespace: """Parses and returns command line arguments.""" parser = argparse.ArgumentParser('Roll DEPS entries shared with Chromium.') @@ -1098,16 +1180,11 @@ _read_remote_chromium_file('DEPS', revision_range.new_revision)) changed_entries = _get_changed_deps_entries(dawn_deps, chromium_deps) - # We want this entry to be in the commit message, but we do not want it to - # be present for _apply_changed_deps() since it is not actually a DEPS - # entry. We also don't want it to be present if no actual changes occurred. - chromium_luci_entry = _sync_chromium_luci_revision( - revision_range.new_revision) - entries_for_commit_message = changed_entries - if chromium_luci_entry.old_revision != chromium_luci_entry.new_revision: - entries_for_commit_message = entries_for_commit_message + [ - chromium_luci_entry - ] + # We want these entries to be in the commit message, but we do not want them + # to be present for _apply_changed_deps() since they are not actually DEPS + # entries. + changed_packages = _sync_starlark_packages(revision_range.new_revision) + entries_for_commit_message = changed_entries + changed_packages # Create the commit message before adding the entry for the Chromium # revision since Chromium information is explicitly added to the message.