Cleanup create_updated_flutter_deps.py a bit (#177162)

mraleph · web-flow · commit 2d3416713fe8 · 2025-10-20T17:34:11.000Z
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.
diff --git a/engine/src/tools/dart/create_updated_flutter_deps.py b/engine/src/tools/dart/create_updated_flutter_deps.py
@@ -19,6 +19,9 @@
 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()))
-
-              updatedfile.write("  '%s':\n   %s,\n\n" % (k, updated_value))
-              break
+
+      for dep_path, dep_source in new_dart_deps.items():
+        updatedfile.write(f"  '{dep_path}':\n   {dep_source},\n\n")
+
       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
@@ -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()
