Move git_repo_tools and process_fakes outside of clang_tidy. (#46017)

Closes flutter/flutter#134988.

Author: matanlurey

testing/run_tests.py

@@ -1045,6 +1045,25 @@ def gather_engine_repo_tools_tests(build_dir):
     )
 
 
+def gather_git_repo_tools_tests(build_dir):
+  test_dir = os.path.join(
+      BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'git_repo_tools'
+  )
+  dart_tests = glob.glob('%s/*_test.dart' % test_dir)
+  for dart_test_file in dart_tests:
+    opts = [
+        '--disable-dart-dev',
+        dart_test_file,
+    ]
+    yield EngineExecutableTask(
+        build_dir,
+        os.path.join('dart-sdk', 'bin', 'dart'),
+        None,
+        flags=opts,
+        cwd=test_dir
+    )
+
+
 def gather_api_consistency_tests(build_dir):
   test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'api_check')
   dart_tests = glob.glob('%s/test/*_test.dart' % test_dir)
@@ -1355,6 +1374,7 @@ def main():
     tasks += list(gather_build_bucket_golden_scraper_tests(build_dir))
     tasks += list(gather_engine_build_configs_tests(build_dir))
     tasks += list(gather_engine_repo_tools_tests(build_dir))
+    tasks += list(gather_git_repo_tools_tests(build_dir))
     tasks += list(gather_api_consistency_tests(build_dir))
     tasks += list(gather_path_ops_tests(build_dir))
     tasks += list(gather_const_finder_tests(build_dir))

tools/clang_tidy/lib/clang_tidy.dart

@@ -6,13 +6,13 @@ import 'dart:convert' show LineSplitter, jsonDecode;
 import 'dart:io' as io show File, stderr, stdout;
 
 import 'package:engine_repo_tools/engine_repo_tools.dart';
+import 'package:git_repo_tools/git_repo_tools.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 import 'package:process/process.dart';
 import 'package:process_runner/process_runner.dart';
 
 import 'src/command.dart';
-import 'src/git_repo.dart';
 import 'src/options.dart';
 
 const String _linterOutputHeader = '''
@@ -211,7 +211,7 @@ class ClangTidy {
         .toList();
     }
 
-    final GitRepo repo = GitRepo(
+    final GitRepo repo = GitRepo.fromRoot(
       options.repoPath,
       processManager: _processManager,
       verbose: options.verbose,

tools/clang_tidy/pubspec.yaml

@@ -18,6 +18,7 @@ environment:
 
 dependencies:
   args: any
+  git_repo_tools: any
   engine_repo_tools: any
   meta: any
   path: any
@@ -28,6 +29,7 @@ dev_dependencies:
   async_helper: any
   expect: any
   litetest: any
+  process_fakes: any
   smith: any
 
 dependency_overrides:
@@ -45,6 +47,8 @@ dependency_overrides:
     path: ../../../third_party/dart/pkg/expect
   file:
     path: ../../../third_party/pkg/file/packages/file
+  git_repo_tools:
+    path: ../pkg/git_repo_tools
   litetest:
     path: ../../testing/litetest
   meta:
@@ -55,6 +59,8 @@ dependency_overrides:
     path: ../../../third_party/pkg/platform
   process:
     path: ../../../third_party/pkg/process
+  process_fakes:
+    path: ../pkg/process_fakes
   process_runner:
     path: ../../../third_party/pkg/process_runner
   smith:

tools/clang_tidy/test/clang_tidy_test.dart

@@ -11,9 +11,9 @@ import 'package:engine_repo_tools/engine_repo_tools.dart';
 import 'package:litetest/litetest.dart';
 import 'package:path/path.dart' as path;
 import 'package:process/process.dart';
+import 'package:process_fakes/process_fakes.dart';
 import 'package:process_runner/process_runner.dart';
 
-import 'process_fakes.dart';
 
 /// A test fixture for the `clang-tidy` tool.
 final class Fixture {

tools/githooks/pubspec.yaml

@@ -45,6 +45,8 @@ dependency_overrides:
     path: ../../../third_party/dart/pkg/expect
   file:
     path: ../../../third_party/pkg/file/packages/file
+  git_repo_tools:
+    path: ../pkg/git_repo_tools
   litetest:
     path: ../../testing/litetest
   meta:

tools/pkg/git_repo_tools/README.md

@@ -0,0 +1,23 @@
+# `git_repo_tools`
+
+This is a repo-internal library for `flutter/engine`, that contains shared code
+for writing tools that want to interact with the `git` repository. For example,
+finding all changed files in the current branch:
+
+```dart
+import 'dart:io' as io show File, Platform;
+
+import 'package:engine_repo_tools/engine_repo_tools.dart';
+import 'package:git_repo_tools/git_repo_tools.dart';
+import 'package:path/path.dart' as path;
+
+void main() async {
+  // Finds the root of the engine repository from the current script.
+  final Engine engine = Engine.findWithin(path.dirname(path.fromUri(io.Platform.script)));
+  final GitRepo gitRepo = GitRepo(engine.flutterDir);
+
+  for (final io.File file in gitRepo.changedFiles) {
+    print('Changed file: ${file.path}');
+  }
+}
+```

tools/clang_tidy/lib/src/git_repo.dart renamed to tools/pkg/git_repo_tools/lib/git_repo_tools.dart

@@ -2,58 +2,66 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:io' as io show Directory, File;
+import 'dart:io' as io show Directory, File, stdout;
 
 import 'package:path/path.dart' as path;
 import 'package:process/process.dart';
 import 'package:process_runner/process_runner.dart';
 
-/// Utility methods for working with a git repo.
-class GitRepo {
+/// Utility methods for working with a git repository.
+final class GitRepo {
   /// The git repository rooted at `root`.
-  GitRepo(this.root, {
+  GitRepo.fromRoot(this.root, {
     this.verbose = false,
+    StringSink? logSink,
     ProcessManager processManager = const LocalProcessManager(),
-  }) : _processManager = processManager;
+  }) :
+      _processManager = processManager,
+      logSink = logSink ?? io.stdout;
 
   /// Whether to produce verbose log output.
+  ///
+  /// If true, output of git commands will be printed to [logSink].
   final bool verbose;
 
+  /// Where to send verbose log output.
+  ///
+  /// Defaults to [io.stdout].
+  final StringSink logSink;
+
   /// The root of the git repo.
   final io.Directory root;
 
   /// The delegate to use for running processes.
   final ProcessManager _processManager;
 
-  List<io.File>? _changedFiles;
-
   /// Returns a list of all non-deleted files which differ from the nearest
   /// merge-base with `main`. If it can't find a fork point, uses the default
   /// merge-base.
   ///
   /// This is only computed once and cached. Subsequent invocations of the
   /// getter will return the same result.
-  Future<List<io.File>> get changedFiles async =>
-    _changedFiles ??= await _getChangedFiles();
-
-  List<io.File>? _changedFilesAtHead;
-
-  /// Returns a list of non-deleted files which differ between the HEAD
-  /// commit and its parent.
-  Future<List<io.File>> get changedFilesAtHead async =>
-    _changedFilesAtHead ??= await _getChangedFilesAtHead();
+  late final Future<List<io.File>> changedFiles = _changedFiles();
 
-  Future<List<io.File>> _getChangedFiles() async {
+  Future<List<io.File>> _changedFiles() async {
     final ProcessRunner processRunner = ProcessRunner(
       defaultWorkingDirectory: root,
       processManager: _processManager,
     );
     await _fetch(processRunner);
+    // Find the merge base between the current branch and the branch that was
+    // checked out at the time of the last fetch. The merge base is the common
+    // ancestor of the two branches, and the output is the hash of the merge
+    // base.
     ProcessRunnerResult mergeBaseResult = await processRunner.runProcess(
       <String>['git', 'merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'],
       failOk: true,
     );
     if (mergeBaseResult.exitCode != 0) {
+      if (verbose) {
+        logSink.writeln('git merge-base --fork-point failed, using default merge-base');
+        logSink.writeln('Output:\n${mergeBaseResult.stdout}');
+      }
       mergeBaseResult = await processRunner.runProcess(<String>[
         'git',
         'merge-base',
@@ -62,8 +70,7 @@ class GitRepo {
       ]);
     }
     final String mergeBase = mergeBaseResult.stdout.trim();
-    final ProcessRunnerResult masterResult = await processRunner
-        .runProcess(<String>[
+    final ProcessRunnerResult masterResult = await processRunner.runProcess(<String>[
       'git',
       'diff',
       '--name-only',
@@ -73,9 +80,17 @@ class GitRepo {
     return _gitOutputToList(masterResult);
   }
 
-  Future<List<io.File>> _getChangedFilesAtHead() async {
+  /// Returns a list of non-deleted files which differ between the HEAD
+  /// commit and its parent.
+  ///
+  /// This is only computed once and cached. Subsequent invocations of the
+  /// getter will return the same result.
+  late final Future<List<io.File>> changedFilesAtHead = _changedFilesAtHead();
+
+  Future<List<io.File>> _changedFilesAtHead() async {
     final ProcessRunner processRunner = ProcessRunner(
       defaultWorkingDirectory: root,
+      processManager: _processManager,
     );
     await _fetch(processRunner);
     final ProcessRunnerResult diffTreeResult = await processRunner.runProcess(
@@ -98,6 +113,10 @@ class GitRepo {
       failOk: true,
     );
     if (fetchResult.exitCode != 0) {
+      if (verbose) {
+        logSink.writeln('git fetch upstream main failed, using origin main');
+        logSink.writeln('Output:\n${fetchResult.stdout}');
+      }
       await processRunner.runProcess(<String>[
         'git',
         'fetch',
@@ -110,7 +129,7 @@ class GitRepo {
   List<io.File> _gitOutputToList(ProcessRunnerResult result) {
     final String diffOutput = result.stdout.trim();
     if (verbose) {
-      print('git diff output:\n$diffOutput');
+      logSink.writeln('git diff output:\n$diffOutput');
     }
     final Set<String> resultMap = <String>{};
     resultMap.addAll(diffOutput.split('\n').where(

tools/pkg/git_repo_tools/pubspec.yaml

@@ -0,0 +1,60 @@
+# 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.
+
+name: git_repo_tools
+publish_to: none
+environment:
+  sdk: '>=3.2.0-0 <4.0.0'
+
+# Do not add any dependencies that require more than what is provided in
+# //third_party/pkg, //third_party/dart/pkg, or
+# //third_party/dart/third_party/pkg. In particular, package:test is not usable
+# here.
+
+# If you do add packages here, make sure you can run `pub get --offline`, and
+# check the .packages and .package_config to make sure all the paths are
+# relative to this directory into //third_party/dart
+
+dependencies:
+  meta: any
+  path: any
+  process: any
+  process_runner: any
+
+dev_dependencies:
+  async_helper: any
+  expect: any
+  litetest: any
+  process_fakes: any
+  smith: any
+
+dependency_overrides:
+  args:
+    path: ../../../../third_party/dart/third_party/pkg/args
+  async:
+    path: ../../../../third_party/dart/third_party/pkg/async
+  async_helper:
+    path: ../../../../third_party/dart/pkg/async_helper
+  collection:
+    path: ../../../../third_party/dart/third_party/pkg/collection
+  expect:
+    path: ../../../../third_party/dart/pkg/expect
+  file:
+    path: ../../../../third_party/pkg/file/packages/file
+  litetest:
+    path: ../../../testing/litetest
+  meta:
+    path: ../../../../third_party/dart/pkg/meta
+  path:
+    path: ../../../../third_party/dart/third_party/pkg/path
+  platform:
+    path: ../../../../third_party/pkg/platform
+  process:
+    path: ../../../../third_party/pkg/process
+  process_fakes:
+    path: ../process_fakes
+  process_runner:
+    path: ../../../../third_party/pkg/process_runner
+  smith:
+    path: ../../../../third_party/dart/pkg/smith