From 2d3416713fe83bf1c31290bafc05919cd6cbd99d Mon Sep 17 00:00:00 2001 From: Slava Egorov Date: Mon, 20 Oct 2025 19:34:11 +0200 Subject: [PATCH] Cleanup create_updated_flutter_deps.py a bit (#177162) Make the code for updating Dart dependencies easier to follow by splitting the logic into helper functions, adding comments and tweaking logic slightly. Make it handle all *_git variables uniformly instead of hardcoding specific ones. Make it handle the case where *_git variable does not exist without emitting incorrect result. Add a unit test to make it simpler to check that script does reasonable things for all different inputs. --- .../tools/dart/create_updated_flutter_deps.py | 149 +++++++++++++----- .../dart/create_updated_flutter_deps_tests.py | 117 ++++++++++++++ 2 files changed, 229 insertions(+), 37 deletions(-) create mode 100644 engine/src/tools/dart/create_updated_flutter_deps_tests.py diff --git a/engine/src/tools/dart/create_updated_flutter_deps.py b/engine/src/tools/dart/create_updated_flutter_deps.py index 818b770ab21..4b0d41a8c25 100755 --- a/engine/src/tools/dart/create_updated_flutter_deps.py +++ b/engine/src/tools/dart/create_updated_flutter_deps.py @@ -19,6 +19,9 @@ OLD_DART_DEPS = os.path.realpath(os.path.join(DART_SCRIPT_DIR, '../../third_part DART_DEPS = os.path.realpath(os.path.join(DART_SCRIPT_DIR, '../../flutter/third_party/dart/DEPS')) FLUTTER_DEPS = os.path.realpath(os.path.join(DART_SCRIPT_DIR, '../../../../DEPS')) +# Path to Dart SDK checkout within Flutter repo. +DART_SDK_ROOT = 'engine/src/flutter/third_party/dart' + class VarImpl(object): def __init__(self, local_scope): self._local_scope = local_scope @@ -64,22 +67,122 @@ def ParseArgs(args): default=FLUTTER_DEPS) return parser.parse_args(args) +def PrettifySourcePathForDEPS(flutter_vars, dep_path, source): + """Prepare source for writing into Flutter DEPS file. + + If source is not a string then it is expected to be a dictionary defining + a CIPD dependency. In this case it is written as is - but sorted to + guarantee stability of the output. + + Otherwise source is a path to a repo plus version (hash or tag): + + {repo_host}/{repo_path}@{version} + + We want to convert this into one of the following: + + Var(repo_host_var) + 'repo_path' + '@' + Var(version_var) + Var(repo_host_var) + 'repo_path@version' + 'source' + + Where repo_host_var is one of '*_git' variables and version_var is one + of 'dart_{dep_name}_tag' or 'dart_{dep_name}_rev' variables. + """ + + # If this a CIPD dependency then keep it as-is but sort its contents + # to ensure stable ordering. + if not isinstance(source, str): + return dict(sorted(source.items())) + + # Decompose source into {repo_host}/{repo_path}@{version} + repo_host_var = None + version_var = None + repo_path_with_version = source + for var_name, var_value in flutter_vars.items(): + if var_name.endswith("_git") and source.startswith(var_value): + repo_host_var = var_name + repo_path_with_version = source[len(var_value):] + break + + if repo_path_with_version.find('@') == -1: + raise ValueError(f'{dep_path} source is unversioned') + + repo_path, version = repo_path_with_version.split('@', 1) + + # Figure out the name of the dependency from its path to compute + # corresponding version_var. + # + # Normally, the last component of the dep_path is the name of the dependency. + # However some dependencies are placed in a subdirectory named "src" + # within a directory named after the dependency. + dep_name = os.path.basename(dep_path) + if dep_name == 'src': + dep_name = os.path.basename(os.path.dirname(dep_path)) + for var_name in [f'dart_{dep_name}_tag', f'dart_{dep_name}_rev']: + if var_name in flutter_vars: + version_var = var_name + break + + # Format result from available individual pieces. + result = [] + if repo_host_var is not None: + result += [f"Var('{repo_host_var}')"] + if version_var is not None: + result += [f"'{repo_path}'", "'@'", f"Var('{version_var}')"] + else: + result += [f"'{repo_path_with_version}'"] + return " + ".join(result) + +def ComputeDartDeps(flutter_vars, flutter_deps, dart_deps): + """Compute sources for deps nested under DART_SDK_ROOT in Flutter DEPS. + + These dependencies originate from Dart SDK, so their version are + computed by looking them up in Dart DEPS using appropriately + relocated paths, e.g. '{DART_SDK_ROOT}/third_party/foo' is located at + 'sdk/third_party/foo' in Dart DEPS. + + Source paths are expressed in terms of 'xyz_git' and 'dart_xyz_tag' or + 'dart_xyz_rev' variables if possible. + + If corresponding path is not found in Dart DEPS the dependency is considered + no longer needed and is removed from Flutter DEPS. + + Returns: dictionary of dependencies + """ + new_dart_deps = {} + + # Trailing / to avoid matching Dart SDK dependency itself. + dart_sdk_root_dir = DART_SDK_ROOT + '/' + + # Find all dependencies which are nested inside Dart SDK and check + # if Dart SDK still needs them. If Dart DEPS still mentions them + # take updated version from Dart DEPS. + for (dep_path, dep_source) in sorted(flutter_deps.items()): + if dep_path.startswith(dart_sdk_root_dir): + # Dart dependencies are given relative to root directory called `sdk/`. + dart_dep_path = f'sdk/{dep_path[len(dart_sdk_root_dir):]}' + if dart_dep_path in dart_deps: + # Still used, add it to the result. + new_dart_deps[dep_path] = PrettifySourcePathForDEPS(flutter_vars, dep_path, dart_deps[dart_dep_path]) + + return new_dart_deps + def Main(argv): args = ParseArgs(argv) if args.dart_deps == DART_DEPS and not os.path.isfile(DART_DEPS): args.dart_deps = OLD_DART_DEPS - (new_vars, new_deps) = ParseDepsFile(args.dart_deps) - (old_vars, old_deps) = ParseDepsFile(args.flutter_deps) + (dart_vars, dart_deps) = ParseDepsFile(args.dart_deps) + (flutter_vars, flutter_deps) = ParseDepsFile(args.flutter_deps) updated_vars = {} # Collect updated dependencies - for (k,v) in sorted(old_vars.items()): + for (k,v) in sorted(flutter_vars.items()): if k not in ('dart_revision', 'dart_git') and k.startswith('dart_'): dart_key = k[len('dart_'):] - if dart_key in new_vars: - updated_revision = new_vars[dart_key].lstrip('@') if dart_key in new_vars else v - updated_vars[k] = updated_revision + if dart_key in dart_vars: + updated_vars[k] = dart_vars[dart_key].lstrip('@') + + new_dart_deps = ComputeDartDeps(flutter_vars, flutter_deps, dart_deps) # Write updated DEPS file to a side updatedfilename = args.flutter_deps + ".new" @@ -104,40 +207,12 @@ def Main(argv): elif lines[i].startswith(" # WARNING: Unused Dart dependencies"): updatedfile.write('\n') i = i + 1 - while i < len(lines) and (lines[i].startswith(" # WARNING: end of dart dependencies") == 0): + while i < len(lines) and not lines[i].startswith(" # WARNING: end of dart dependencies"): i = i + 1 - for (k, v) in sorted(old_deps.items()): - if (k.startswith('engine/src/flutter/third_party/dart/')): - for (dart_k, dart_v) in (list(new_deps.items())): - dart_k_suffix = dart_k[len('sdk/') if dart_k.startswith('sdk/') else 0:] - if (k.endswith(dart_k_suffix)): - if (isinstance(dart_v, str)): - updated_value = dart_v.replace(new_vars["dart_git"], "Var('dart_git') + '/") - updated_value = updated_value.replace(new_vars["chromium_git"], "Var('chromium_git') + '") - updated_value = updated_value.replace(new_vars["android_git"], "Var('android_git') + '") - plain_v = os.path.basename(dart_k) - # Some dependencies are placed in a subdirectory named "src" - # within a directory named for the package. - if plain_v == 'src': - plain_v = os.path.basename(os.path.dirname(dart_k)) - # This dependency has to be special-cased here because the - # repository name is not the same as the directory name. - if plain_v == "quiver": - plain_v = "quiver-dart" - if ('dart_' + plain_v + '_tag' in updated_vars): - updated_value = updated_value[:updated_value.rfind('@')] + "' + '@' + Var('dart_" + plain_v + "_tag')" - elif ('dart_' + plain_v + '_rev' in updated_vars): - updated_value = updated_value[:updated_value.rfind('@')] + "' + '@' + Var('dart_" + plain_v + "_rev')" - else: - updated_value = updated_value + "'" - else: - # Non-string values(dicts) copy verbatim, keeping them sorted - # to ensure stable ordering of items. - updated_value = dict(sorted(dart_v.items())) + for dep_path, dep_source in new_dart_deps.items(): + updatedfile.write(f" '{dep_path}':\n {dep_source},\n\n") - updatedfile.write(" '%s':\n %s,\n\n" % (k, updated_value)) - break updatedfile.write(lines[i]) i = i + 1 diff --git a/engine/src/tools/dart/create_updated_flutter_deps_tests.py b/engine/src/tools/dart/create_updated_flutter_deps_tests.py new file mode 100644 index 00000000000..68918d1b37b --- /dev/null +++ b/engine/src/tools/dart/create_updated_flutter_deps_tests.py @@ -0,0 +1,117 @@ +#!/usr/bin/env python3 +# +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. +# +# Usage: python3 create_updated_flutter_deps_tests.py +# +# Unit tests for create_updated_flutter_deps.py script. + +import unittest + +from create_updated_flutter_deps import ( + DART_SDK_ROOT, + ComputeDartDeps, + PrettifySourcePathForDEPS, +) + + +class TestPrettifySourcePathForDEPS(unittest.TestCase): + def test_PrettifySourcePathForDEPS_unversioned(self): + with self.assertRaises(ValueError): + PrettifySourcePathForDEPS(flutter_vars={}, dep_path="a", source="b") + + def test_PrettifySourcePathForDEPS_all_cases(self): + a_git = "https://a.googlesource.com" + b_git = "https://b.googlesource.com" + flutter_vars = { + "a_git": a_git, + "dart_dep2_tag": "xyz", + "dart_dep3_rev": "def", + } + + deps = { + "/no_repo_var/dep1": f"{b_git}/repos/dep1@whatever", + "/no_repo_var/dep2": f"{b_git}/repos/dep2@whatever", + "/no_repo_var/dep2/src": f"{b_git}/repos/dep2@whatever", + "/no_repo_var/dep3": f"{b_git}/repos/dep3@whatever", + "/no_repo_var/dep3/src": f"{b_git}/repos/dep3@whatever", + "/a_git_repo/dep1": f"{a_git}/repos/dep1@whatever", + "/a_git_repo/dep2": f"{a_git}/repos/dep2@whatever", + "/a_git_repo/dep2/src": f"{a_git}/repos/dep2@whatever", + "/a_git_repo/dep3": f"{a_git}/repos/dep3@whatever", + "/a_git_repo/dep3/src": f"{a_git}/repos/dep3@whatever", + } + + expected = { + "/no_repo_var/dep1": f"'{b_git}/repos/dep1@whatever'", + "/no_repo_var/dep2": f"'{b_git}/repos/dep2' + '@' + Var('dart_dep2_tag')", + "/no_repo_var/dep2/src": f"'{b_git}/repos/dep2' + '@' + Var('dart_dep2_tag')", + "/no_repo_var/dep3": f"'{b_git}/repos/dep3' + '@' + Var('dart_dep3_rev')", + "/no_repo_var/dep3/src": f"'{b_git}/repos/dep3' + '@' + Var('dart_dep3_rev')", + "/a_git_repo/dep1": "Var('a_git') + '/repos/dep1@whatever'", + "/a_git_repo/dep2": "Var('a_git') + '/repos/dep2' + '@' + Var('dart_dep2_tag')", + "/a_git_repo/dep2/src": "Var('a_git') + '/repos/dep2' + '@' + Var('dart_dep2_tag')", + "/a_git_repo/dep3": "Var('a_git') + '/repos/dep3' + '@' + Var('dart_dep3_rev')", + "/a_git_repo/dep3/src": "Var('a_git') + '/repos/dep3' + '@' + Var('dart_dep3_rev')", + } + + for dep_path, source_path in deps.items(): + self.assertEqual( + PrettifySourcePathForDEPS(flutter_vars, dep_path, source_path), + expected[dep_path], + ) + + +class TestComputeDartDeps(unittest.TestCase): + def test_ComputeDartDeps_nothing_to_do(self): + # Note: DART_SDK_ROOT dependency itself should be simply ignored. + self.assertEqual( + ComputeDartDeps( + flutter_vars={}, + flutter_deps={ + DART_SDK_ROOT: "whatever", + }, + dart_deps={ + "sdk": "xyz", + }, + ), + {}, + ) + + def test_ComputeDartDeps_unused_dep(self): + a_git = "https://a.googlesource.com" + self.assertEqual( + ComputeDartDeps( + flutter_vars={ + "a_git": a_git, + }, + flutter_deps={ + f"{DART_SDK_ROOT}/third_party/dep": f"{a_git}/repos/dep@version", + }, + dart_deps={}, + ), + {}, + ) + + def test_ComputeDartDeps_used_dep(self): + a_git = "https://a.googlesource.com" + self.assertEqual( + ComputeDartDeps( + flutter_vars={ + "a_git": a_git, + }, + flutter_deps={ + f"{DART_SDK_ROOT}/third_party/dep": "whatever", + }, + dart_deps={"sdk/third_party/dep": f"{a_git}/repos/dep@version"}, + ), + { + f"{DART_SDK_ROOT}/third_party/dep": "Var('a_git') + '/repos/dep@version'", + }, + ) + + +if __name__ == "__main__": + unittest.main()