[flutter_plugin_tools] Add a new 'make-deps-path-based' command (#4575)

Adds a new command that adds `dependency_overrides` to any packages in the repository that depend on a list of target packages, including an option to target packages that will publish a non-breaking change in a given diff.

Adds a new CI step that uses the above in conjunction with a new `--run-on-dirty-packages` to adjust the dependencies of anything in the repository that uses a to-be-published package and then re-run analysis on just those packages. This will allow us to catch in presubmit any changes that are not breaking from a semver standpoint, but will break us due to our strict analysis in CI.

Fixes flutter/flutter#89862

Author: stuartmorgan-g

.cirrus.yml

@@ -109,13 +109,25 @@ task:
         matrix:
           CHANNEL: "master"
           CHANNEL: "stable"
-      tool_script:
+      analyze_tool_script:
         - cd script/tool
         - dart analyze --fatal-infos
-      script:
+      analyze_script:
         # DO NOT change the custom-analysis argument here without changing the Dart repo.
         # See the comment in script/configs/custom_analysis.yaml for details.
         - ./script/tool_runner.sh analyze --custom-analysis=script/configs/custom_analysis.yaml
+      pathified_analyze_script:
+        # Re-run analysis with path-based dependencies to ensure that publishing
+        # the changes won't break analysis of other packages in the respository
+        # that depend on it.
+        - ./script/tool_runner.sh make-deps-path-based --target-dependencies-with-non-breaking-updates
+        # This uses --run-on-dirty-packages rather than --packages-for-branch
+        # since only the packages changed by 'make-deps-path-based' need to be
+        # checked.
+        - dart $PLUGIN_TOOL analyze --run-on-dirty-packages --log-timing --custom-analysis=script/configs/custom_analysis.yaml
+        # Restore the tree to a clean state, to avoid accidental issues if
+        # other script steps are added to this task.
+        - git checkout .
     ### Android tasks ###
     - name: android-build_all_plugins
       env:

script/tool/CHANGELOG.md

@@ -1,6 +1,10 @@
 ## NEXT
 
 - Ensures that `firebase-test-lab` runs include an `integration_test` runner.
+- Adds a `make-deps-path-based` command to convert inter-repo package
+  dependencies to path-based dependencies.
+- Adds a (hidden) `--run-on-dirty-packages` flag for use with
+  `make-deps-path-based` in CI.
 
 ## 0.7.3
 

script/tool/lib/src/common/git_version_finder.dart

@@ -31,10 +31,16 @@ class GitVersionFinder {
   }
 
   /// Get a list of all the changed files.
-  Future<List<String>> getChangedFiles() async {
+  Future<List<String>> getChangedFiles(
+      {bool includeUncommitted = false}) async {
     final String baseSha = await getBaseSha();
     final io.ProcessResult changedFilesCommand = await baseGitDir
-        .runCommand(<String>['diff', '--name-only', baseSha, 'HEAD']);
+        .runCommand(<String>[
+      'diff',
+      '--name-only',
+      baseSha,
+      if (!includeUncommitted) 'HEAD'
+    ]);
     final String changedFilesStdout = changedFilesCommand.stdout.toString();
     if (changedFilesStdout.isEmpty) {
       return <String>[];

script/tool/lib/src/common/plugin_command.dart

@@ -75,9 +75,15 @@ abstract class PluginCommand extends Command<void> {
         help: 'Run the command on changed packages/plugins.\n'
             'If no packages have changed, or if there have been changes that may\n'
             'affect all packages, the command runs on all packages.\n'
-            'The packages excluded with $_excludeArg is also excluded even if changed.\n'
+            'Packages excluded with $_excludeArg are excluded even if changed.\n'
             'See $_baseShaArg if a custom base is needed to determine the diff.\n\n'
             'Cannot be combined with $_packagesArg.\n');
+    argParser.addFlag(_runOnDirtyPackagesArg,
+        help:
+            'Run the command on packages with changes that have not been committed.\n'
+            'Packages excluded with $_excludeArg are excluded even if changed.\n'
+            'Cannot be combined with $_packagesArg.\n',
+        hide: true);
     argParser.addFlag(_packagesForBranchArg,
         help:
             'This runs on all packages (equivalent to no package selection flag)\n'
@@ -103,6 +109,7 @@ abstract class PluginCommand extends Command<void> {
   static const String _packagesForBranchArg = 'packages-for-branch';
   static const String _pluginsArg = 'plugins';
   static const String _runOnChangedPackagesArg = 'run-on-changed-packages';
+  static const String _runOnDirtyPackagesArg = 'run-on-dirty-packages';
   static const String _shardCountArg = 'shardCount';
   static const String _shardIndexArg = 'shardIndex';
 
@@ -289,6 +296,7 @@ abstract class PluginCommand extends Command<void> {
     final Set<String> packageSelectionFlags = <String>{
       _packagesArg,
       _runOnChangedPackagesArg,
+      _runOnDirtyPackagesArg,
       _packagesForBranchArg,
     };
     if (packageSelectionFlags
@@ -300,7 +308,7 @@ abstract class PluginCommand extends Command<void> {
       throw ToolExit(exitInvalidArguments);
     }
 
-    Set<String> plugins = Set<String>.from(getStringListArg(_packagesArg));
+    Set<String> packages = Set<String>.from(getStringListArg(_packagesArg));
 
     final bool runOnChangedPackages;
     if (getBoolArg(_runOnChangedPackagesArg)) {
@@ -331,7 +339,21 @@ abstract class PluginCommand extends Command<void> {
       final List<String> changedFiles =
           await gitVersionFinder.getChangedFiles();
       if (!_changesRequireFullTest(changedFiles)) {
-        plugins = _getChangedPackages(changedFiles);
+        packages = _getChangedPackages(changedFiles);
+      }
+    } else if (getBoolArg(_runOnDirtyPackagesArg)) {
+      final GitVersionFinder gitVersionFinder =
+          GitVersionFinder(await gitDir, 'HEAD');
+      print('Running for all packages that have uncommitted changes\n');
+      // _changesRequireFullTest is deliberately not used here, as this flag is
+      // intended for use in CI to re-test packages changed by
+      // 'make-deps-path-based'.
+      packages = _getChangedPackages(
+          await gitVersionFinder.getChangedFiles(includeUncommitted: true));
+      // For the same reason, empty is not treated as "all packages" as it is
+      // for other flags.
+      if (packages.isEmpty) {
+        return;
       }
     }
 
@@ -347,7 +369,7 @@ abstract class PluginCommand extends Command<void> {
           in dir.list(followLinks: false)) {
         // A top-level Dart package is a plugin package.
         if (_isDartPackage(entity)) {
-          if (plugins.isEmpty || plugins.contains(p.basename(entity.path))) {
+          if (packages.isEmpty || packages.contains(p.basename(entity.path))) {
             yield PackageEnumerationEntry(
                 RepositoryPackage(entity as Directory),
                 excluded: excludedPluginNames.contains(entity.basename));
@@ -364,9 +386,9 @@ abstract class PluginCommand extends Command<void> {
                   path.relative(subdir.path, from: dir.path);
               final String packageName = path.basename(subdir.path);
               final String basenamePath = path.basename(entity.path);
-              if (plugins.isEmpty ||
-                  plugins.contains(relativePath) ||
-                  plugins.contains(basenamePath)) {
+              if (packages.isEmpty ||
+                  packages.contains(relativePath) ||
+                  packages.contains(basenamePath)) {
                 yield PackageEnumerationEntry(
                     RepositoryPackage(subdir as Directory),
                     excluded: excludedPluginNames.contains(basenamePath) ||

script/tool/lib/src/common/repository_package.dart

@@ -4,6 +4,7 @@
 
 import 'package:file/file.dart';
 import 'package:path/path.dart' as p;
+import 'package:pubspec_parse/pubspec_parse.dart';
 
 import 'core.dart';
 
@@ -47,6 +48,14 @@ class RepositoryPackage {
   /// The package's top-level pubspec.yaml.
   File get pubspecFile => directory.childFile('pubspec.yaml');
 
+  late final Pubspec _parsedPubspec =
+      Pubspec.parse(pubspecFile.readAsStringSync());
+
+  /// Returns the parsed [pubspecFile].
+  ///
+  /// Caches for future use.
+  Pubspec parsePubspec() => _parsedPubspec;
+
   /// True if this appears to be a federated plugin package, according to
   /// repository conventions.
   bool get isFederated =>

script/tool/lib/src/create_all_plugins_app_command.dart

@@ -178,8 +178,7 @@ class CreateAllPluginsAppCommand extends PluginCommand {
       final RepositoryPackage package = entry.package;
       final Directory pluginDirectory = package.directory;
       final String pluginName = pluginDirectory.basename;
-      final File pubspecFile = package.pubspecFile;
-      final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
+      final Pubspec pubspec = package.parsePubspec();
 
       if (pubspec.publishTo != 'none') {
         pathDependencies[pluginName] = PathDependency(pluginDirectory.path);

script/tool/lib/src/main.dart

@@ -20,6 +20,7 @@ import 'license_check_command.dart';
 import 'lint_android_command.dart';
 import 'lint_podspecs_command.dart';
 import 'list_command.dart';
+import 'make_deps_path_based_command.dart';
 import 'native_test_command.dart';
 import 'publish_check_command.dart';
 import 'publish_plugin_command.dart';
@@ -58,6 +59,7 @@ void main(List<String> args) {
     ..addCommand(LintPodspecsCommand(packagesDir))
     ..addCommand(ListCommand(packagesDir))
     ..addCommand(NativeTestCommand(packagesDir))
+    ..addCommand(MakeDepsPathBasedCommand(packagesDir))
     ..addCommand(PublishCheckCommand(packagesDir))
     ..addCommand(PublishPluginCommand(packagesDir))
     ..addCommand(PubspecCheckCommand(packagesDir))