From 16d75edd004eacdaeabe2cf096d5e09af42cfd58 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 16:28:18 -0700 Subject: [PATCH 01/16] Keep tools/clang_tidy/lib/src/git_repo.dart --- ...t-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools/clang_tidy/lib/src/git_repo.dart => tools__clang_tidy__lib__src__git_repo.dart-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart (100%) diff --git a/tools/clang_tidy/lib/src/git_repo.dart b/tools__clang_tidy__lib__src__git_repo.dart-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart similarity index 100% rename from tools/clang_tidy/lib/src/git_repo.dart rename to tools__clang_tidy__lib__src__git_repo.dart-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart From 7820e1b35b43477961e29227a96a538be189acdd Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 16:28:19 -0700 Subject: [PATCH 02/16] Copy tools/clang_tidy/lib/src/git_repo.dart into tools/pkg/git_repo_tools/lib/git_repo_tools.dart --- .../git_repo.dart => pkg/git_repo_tools/lib/git_repo_tools.dart} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools/{clang_tidy/lib/src/git_repo.dart => pkg/git_repo_tools/lib/git_repo_tools.dart} (100%) diff --git a/tools/clang_tidy/lib/src/git_repo.dart b/tools/pkg/git_repo_tools/lib/git_repo_tools.dart similarity index 100% rename from tools/clang_tidy/lib/src/git_repo.dart rename to tools/pkg/git_repo_tools/lib/git_repo_tools.dart From 6e3fbf2d4ce2d26191af7d6c1d6e896614574de1 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 16:28:22 -0700 Subject: [PATCH 03/16] Set back tools/clang_tidy/lib/src/git_repo.dart file --- .../clang_tidy/lib/src/git_repo.dart | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools__clang_tidy__lib__src__git_repo.dart-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart => tools/clang_tidy/lib/src/git_repo.dart (100%) diff --git a/tools__clang_tidy__lib__src__git_repo.dart-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart b/tools/clang_tidy/lib/src/git_repo.dart similarity index 100% rename from tools__clang_tidy__lib__src__git_repo.dart-move-to-tools__pkg__git_repo_tools__lib__git_repo_tools.dart rename to tools/clang_tidy/lib/src/git_repo.dart From 464578acc7a994a660bd2f7739f1018570db92a3 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 16:33:49 -0700 Subject: [PATCH 04/16] Start working on new sub-package. --- tools/pkg/git_repo_tools/README.md | 23 +++++++++++++ tools/pkg/git_repo_tools/pubspec.yaml | 47 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 tools/pkg/git_repo_tools/README.md create mode 100644 tools/pkg/git_repo_tools/pubspec.yaml diff --git a/tools/pkg/git_repo_tools/README.md b/tools/pkg/git_repo_tools/README.md new file mode 100644 index 0000000000000..e308c8104b911 --- /dev/null +++ b/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}'); + } +} +``` diff --git a/tools/pkg/git_repo_tools/pubspec.yaml b/tools/pkg/git_repo_tools/pubspec.yaml new file mode 100644 index 0000000000000..3b952a548eabc --- /dev/null +++ b/tools/pkg/git_repo_tools/pubspec.yaml @@ -0,0 +1,47 @@ +# 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 + smith: any + +dependency_overrides: + async_helper: + path: ../../../../third_party/dart/pkg/async_helper + expect: + path: ../../../../third_party/dart/pkg/expect + litetest: + path: ../../../testing/litetest + meta: + path: ../../../../third_party/dart/pkg/meta + path: + path: ../../../../third_party/dart/third_party/pkg/path + process: + path: ../../../../third_party/pkg/process + process_runner: + path: ../../../../third_party/pkg/process_runner + smith: + path: ../../../../third_party/dart/pkg/smith From ec556e09ff95b08eb3ebbc559a3637ce734c0564 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:09:10 -0700 Subject: [PATCH 05/16] Keep tools/clang_tidy/test/process_fakes.dart --- ...rt-move-to-tools__pkg__process_fakes__lib__process_flakes.dart | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools/clang_tidy/test/process_fakes.dart => tools__clang_tidy__test__process_fakes.dart-move-to-tools__pkg__process_fakes__lib__process_flakes.dart (100%) diff --git a/tools/clang_tidy/test/process_fakes.dart b/tools__clang_tidy__test__process_fakes.dart-move-to-tools__pkg__process_fakes__lib__process_flakes.dart similarity index 100% rename from tools/clang_tidy/test/process_fakes.dart rename to tools__clang_tidy__test__process_fakes.dart-move-to-tools__pkg__process_fakes__lib__process_flakes.dart From 2966a5bb46cd3158260257b63fd00737c64a5924 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:09:12 -0700 Subject: [PATCH 06/16] Copy tools/clang_tidy/test/process_fakes.dart into tools/pkg/process_fakes/lib/process_flakes.dart --- .../process_fakes/lib/process_flakes.dart} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools/{clang_tidy/test/process_fakes.dart => pkg/process_fakes/lib/process_flakes.dart} (100%) diff --git a/tools/clang_tidy/test/process_fakes.dart b/tools/pkg/process_fakes/lib/process_flakes.dart similarity index 100% rename from tools/clang_tidy/test/process_fakes.dart rename to tools/pkg/process_fakes/lib/process_flakes.dart From 0353b3937cd2fbde2ebffbaa12c9072217c94a24 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:09:15 -0700 Subject: [PATCH 07/16] Set back tools/clang_tidy/test/process_fakes.dart file --- .../clang_tidy/test/process_fakes.dart | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools__clang_tidy__test__process_fakes.dart-move-to-tools__pkg__process_fakes__lib__process_flakes.dart => tools/clang_tidy/test/process_fakes.dart (100%) diff --git a/tools__clang_tidy__test__process_fakes.dart-move-to-tools__pkg__process_fakes__lib__process_flakes.dart b/tools/clang_tidy/test/process_fakes.dart similarity index 100% rename from tools__clang_tidy__test__process_fakes.dart-move-to-tools__pkg__process_fakes__lib__process_flakes.dart rename to tools/clang_tidy/test/process_fakes.dart From 275d92061bae59b20c7736b391ad2126cf243d1a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:39:43 -0700 Subject: [PATCH 08/16] Move process_fakes, add some test for git_repo_tools. --- .../git_repo_tools/lib/git_repo_tools.dart | 61 +++-- tools/pkg/git_repo_tools/pubspec.yaml | 3 + .../test/git_repo_tools_test.dart | 215 ++++++++++++++++++ tools/pkg/process_fakes/README.md | 7 + ...process_flakes.dart => process_fakes.dart} | 5 + tools/pkg/process_fakes/pubspec.yaml | 24 ++ 6 files changed, 294 insertions(+), 21 deletions(-) create mode 100644 tools/pkg/git_repo_tools/test/git_repo_tools_test.dart create mode 100644 tools/pkg/process_fakes/README.md rename tools/pkg/process_fakes/lib/{process_flakes.dart => process_fakes.dart} (91%) create mode 100644 tools/pkg/process_fakes/pubspec.yaml diff --git a/tools/pkg/git_repo_tools/lib/git_repo_tools.dart b/tools/pkg/git_repo_tools/lib/git_repo_tools.dart index 746359bb51198..40fd07747f06b 100644 --- a/tools/pkg/git_repo_tools/lib/git_repo_tools.dart +++ b/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? _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> get changedFiles async => - _changedFiles ??= await _getChangedFiles(); - - List? _changedFilesAtHead; - - /// Returns a list of non-deleted files which differ between the HEAD - /// commit and its parent. - Future> get changedFilesAtHead async => - _changedFilesAtHead ??= await _getChangedFilesAtHead(); + late final Future> changedFiles = _changedFiles(); - Future> _getChangedFiles() async { + Future> _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( ['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([ 'git', 'merge-base', @@ -62,8 +70,7 @@ class GitRepo { ]); } final String mergeBase = mergeBaseResult.stdout.trim(); - final ProcessRunnerResult masterResult = await processRunner - .runProcess([ + final ProcessRunnerResult masterResult = await processRunner.runProcess([ 'git', 'diff', '--name-only', @@ -73,9 +80,17 @@ class GitRepo { return _gitOutputToList(masterResult); } - Future> _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> changedFilesAtHead = _changedFilesAtHead(); + + Future> _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([ 'git', 'fetch', @@ -110,7 +129,7 @@ class GitRepo { List _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 resultMap = {}; resultMap.addAll(diffOutput.split('\n').where( diff --git a/tools/pkg/git_repo_tools/pubspec.yaml b/tools/pkg/git_repo_tools/pubspec.yaml index 3b952a548eabc..3f9b5abeb153d 100644 --- a/tools/pkg/git_repo_tools/pubspec.yaml +++ b/tools/pkg/git_repo_tools/pubspec.yaml @@ -26,6 +26,7 @@ dev_dependencies: async_helper: any expect: any litetest: any + process_fakes: any smith: any dependency_overrides: @@ -41,6 +42,8 @@ dependency_overrides: path: ../../../../third_party/dart/third_party/pkg/path process: path: ../../../../third_party/pkg/process + process_fakes: + path: ../process_fakes process_runner: path: ../../../../third_party/pkg/process_runner smith: diff --git a/tools/pkg/git_repo_tools/test/git_repo_tools_test.dart b/tools/pkg/git_repo_tools/test/git_repo_tools_test.dart new file mode 100644 index 0000000000000..bca022f353a5d --- /dev/null +++ b/tools/pkg/git_repo_tools/test/git_repo_tools_test.dart @@ -0,0 +1,215 @@ +// 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. + +import 'dart:io' as io; + +import 'package:git_repo_tools/git_repo_tools.dart'; +import 'package:litetest/litetest.dart'; +import 'package:process_fakes/process_fakes.dart'; + +void main() { + const String fakeShaHash = 'fake-sha-hash'; + + + test('returns non-deleted files which differ from merge-base with main', () async { + final Fixture fixture = Fixture( + processManager: FakeProcessManager( + onStart: (List command) { + // Succeed calling "git merge-base --fork-point FETCH_HEAD HEAD". + if (command.join(' ').startsWith('git merge-base --fork-point')) { + return FakeProcess(stdout: fakeShaHash); + } + + // Succeed calling "git fetch upstream main". + if (command.join(' ') == 'git fetch upstream main') { + return FakeProcess(); + } + + // Succeed calling "git diff --name-only --diff-filter=ACMRT fake-sha-hash". + if (command.join(' ') == 'git diff --name-only --diff-filter=ACMRT $fakeShaHash') { + return FakeProcess(stdout: 'file1\nfile2'); + } + + // Otherwise, fail. + return FakeProcessManager.unhandledStart(command); + }, + ), + ); + + try { + final List changedFiles = await fixture.gitRepo.changedFiles; + expect(changedFiles, hasLength(2)); + expect(changedFiles[0].path, endsWith('file1')); + expect(changedFiles[1].path, endsWith('file2')); + } finally { + fixture.gitRepo.root.deleteSync(recursive: true); + } + }); + + test('returns non-deleted files which differ from default merge-base', () async { + final Fixture fixture = Fixture( + processManager: FakeProcessManager( + onStart: (List command) { + if (command.join(' ').startsWith('git merge-base --fork-point')) { + return FakeProcess(exitCode: 1); + } + + if (command.join(' ').startsWith('git merge-base')) { + return FakeProcess(stdout: fakeShaHash); + } + + if (command.join(' ') == 'git fetch upstream main') { + return FakeProcess(); + } + + if (command.join(' ') == 'git diff --name-only --diff-filter=ACMRT $fakeShaHash') { + return FakeProcess(stdout: 'file1\nfile2'); + } + + // Otherwise, fail. + return FakeProcessManager.unhandledStart(command); + }, + ), + ); + + try { + final List changedFiles = await fixture.gitRepo.changedFiles; + expect(changedFiles, hasLength(2)); + expect(changedFiles[0].path, endsWith('file1')); + expect(changedFiles[1].path, endsWith('file2')); + } finally { + fixture.gitRepo.root.deleteSync(recursive: true); + } + }); + + test('returns non-deleted files which differ from HEAD', () async { + final Fixture fixture = Fixture( + processManager: FakeProcessManager( + onStart: (List command) { + if (command.join(' ') == 'git fetch upstream main') { + return FakeProcess(); + } + + if (command.join(' ') == 'git diff-tree --no-commit-id --name-only --diff-filter=ACMRT -r HEAD') { + return FakeProcess(stdout: 'file1\nfile2'); + } + + // Otherwise, fail. + return FakeProcessManager.unhandledStart(command); + }, + ), + ); + + try { + final List changedFiles = await fixture.gitRepo.changedFilesAtHead; + expect(changedFiles, hasLength(2)); + expect(changedFiles[0].path, endsWith('file1')); + expect(changedFiles[1].path, endsWith('file2')); + } finally { + fixture.gitRepo.root.deleteSync(recursive: true); + } + }); + + test('returns non-deleted files which differ from HEAD when merge-base fails', () async { + final Fixture fixture = Fixture( + processManager: FakeProcessManager( + onStart: (List command) { + if (command.join(' ') == 'git fetch upstream main') { + return FakeProcess(); + } + + if (command.join(' ') == 'git diff-tree --no-commit-id --name-only --diff-filter=ACMRT -r HEAD') { + return FakeProcess(stdout: 'file1\nfile2'); + } + + if (command.join(' ').startsWith('git merge-base --fork-point')) { + return FakeProcess(exitCode: 1); + } + + if (command.join(' ').startsWith('git merge-base')) { + return FakeProcess(stdout: fakeShaHash); + } + + // Otherwise, fail. + return FakeProcessManager.unhandledStart(command); + }, + ), + ); + + try { + final List changedFiles = await fixture.gitRepo.changedFilesAtHead; + expect(changedFiles, hasLength(2)); + expect(changedFiles[0].path, endsWith('file1')); + expect(changedFiles[1].path, endsWith('file2')); + } finally { + fixture.gitRepo.root.deleteSync(recursive: true); + } + }); + + test('verbose output is captured', () async { + final Fixture fixture = Fixture( + processManager: FakeProcessManager( + onStart: (List command) { + if (command.join(' ').startsWith('git merge-base --fork-point')) { + return FakeProcess(exitCode: 1); + } + + if (command.join(' ').startsWith('git merge-base')) { + return FakeProcess(stdout: fakeShaHash); + } + + if (command.join(' ') == 'git fetch upstream main') { + return FakeProcess(); + } + + if (command.join(' ') == 'git diff --name-only --diff-filter=ACMRT $fakeShaHash') { + return FakeProcess(stdout: 'file1\nfile2'); + } + + // Otherwise, fail. + return FakeProcessManager.unhandledStart(command); + }, + ), + verbose: true, + ); + + try { + await fixture.gitRepo.changedFiles; + expect(fixture.logSink.toString(), contains('git merge-base --fork-point failed, using default merge-base')); + expect(fixture.logSink.toString(), contains('git diff output:\nfile1\nfile2')); + } finally { + fixture.gitRepo.root.deleteSync(recursive: true); + } + }); +} + +final class Fixture { + factory Fixture({ + FakeProcessManager? processManager, + bool verbose = false, + }) { + final io.Directory root = io.Directory.systemTemp.createTempSync('git_repo_tools.test'); + final StringBuffer logSink = StringBuffer(); + processManager ??= FakeProcessManager(); + return Fixture._( + gitRepo: GitRepo.fromRoot(root, + logSink: logSink, + processManager: processManager, + verbose: verbose, + ), + logSink: logSink, + processManager: processManager, + ); + } + + const Fixture._({ + required this.gitRepo, + required this.logSink, + required this.processManager, + }); + + final GitRepo gitRepo; + final StringBuffer logSink; + final FakeProcessManager processManager; +} diff --git a/tools/pkg/process_fakes/README.md b/tools/pkg/process_fakes/README.md new file mode 100644 index 0000000000000..33da210aea5b0 --- /dev/null +++ b/tools/pkg/process_fakes/README.md @@ -0,0 +1,7 @@ +# `process_fakes` + +Fake implementations of `Process` and `ProcessManager` for testing. + +This is not a great package, and is the bare minimum needed for fairly basic +tooling that uses `ProcessManager`. If we ever need a more complete solution +we should look at upstreaming [`flutter_tools/.../fake_proecss_manager.dart`](https://github.com/flutter/flutter/blob/a9183f696c8e12617d05a26b0b5e80035e515f2a/packages/flutter_tools/test/src/fake_process_manager.dart#L223) diff --git a/tools/pkg/process_fakes/lib/process_flakes.dart b/tools/pkg/process_fakes/lib/process_fakes.dart similarity index 91% rename from tools/pkg/process_fakes/lib/process_flakes.dart rename to tools/pkg/process_fakes/lib/process_fakes.dart index 7bc5fe5778e1f..356f1d391f33d 100644 --- a/tools/pkg/process_fakes/lib/process_flakes.dart +++ b/tools/pkg/process_fakes/lib/process_fakes.dart @@ -9,15 +9,20 @@ import 'package:process/process.dart'; /// A fake implementation of [ProcessManager] that allows control for testing. final class FakeProcessManager implements ProcessManager { + /// Creates a fake process manager delegates to [onRun] and [onStart]. + /// + /// If either is not provided, it throws an [UnsupportedError] when called. FakeProcessManager({ io.ProcessResult Function(List command) onRun = unhandledRun, io.Process Function(List command) onStart = unhandledStart, }) : _onRun = onRun, _onStart = onStart; + /// A default implementation of [onRun] that throws an [UnsupportedError]. static io.ProcessResult unhandledRun(List command) { throw UnsupportedError('Unhandled run: ${command.join(' ')}'); } + /// A default implementation of [onStart] that throws an [UnsupportedError]. static io.Process unhandledStart(List command) { throw UnsupportedError('Unhandled start: ${command.join(' ')}'); } diff --git a/tools/pkg/process_fakes/pubspec.yaml b/tools/pkg/process_fakes/pubspec.yaml new file mode 100644 index 0000000000000..3d45d8ef0fa44 --- /dev/null +++ b/tools/pkg/process_fakes/pubspec.yaml @@ -0,0 +1,24 @@ +# 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: process_fakes +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: + process: any + +dependency_overrides: + process: + path: ../../../../third_party/pkg/process From b53b561bfef15a1a54ee911250bdbd4fb1ed8c36 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:42:30 -0700 Subject: [PATCH 09/16] Fix pubspecs and imports. --- tools/clang_tidy/lib/clang_tidy.dart | 4 +- tools/clang_tidy/lib/src/git_repo.dart | 123 --------------------- tools/clang_tidy/pubspec.yaml | 6 + tools/clang_tidy/test/clang_tidy_test.dart | 2 +- tools/clang_tidy/test/process_fakes.dart | 117 -------------------- 5 files changed, 9 insertions(+), 243 deletions(-) delete mode 100644 tools/clang_tidy/lib/src/git_repo.dart delete mode 100644 tools/clang_tidy/test/process_fakes.dart diff --git a/tools/clang_tidy/lib/clang_tidy.dart b/tools/clang_tidy/lib/clang_tidy.dart index f54d3c78a15e2..52fbded58489f 100644 --- a/tools/clang_tidy/lib/clang_tidy.dart +++ b/tools/clang_tidy/lib/clang_tidy.dart @@ -5,13 +5,13 @@ import 'dart:convert' show LineSplitter, jsonDecode; import 'dart:io' as io show File, stderr, stdout; +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 = ''' @@ -206,7 +206,7 @@ class ClangTidy { .toList(); } - final GitRepo repo = GitRepo( + final GitRepo repo = GitRepo.fromRoot( options.repoPath, processManager: _processManager, verbose: options.verbose, diff --git a/tools/clang_tidy/lib/src/git_repo.dart b/tools/clang_tidy/lib/src/git_repo.dart deleted file mode 100644 index 746359bb51198..0000000000000 --- a/tools/clang_tidy/lib/src/git_repo.dart +++ /dev/null @@ -1,123 +0,0 @@ -// 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. - -import 'dart:io' as io show Directory, File; - -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 { - /// The git repository rooted at `root`. - GitRepo(this.root, { - this.verbose = false, - ProcessManager processManager = const LocalProcessManager(), - }) : _processManager = processManager; - - /// Whether to produce verbose log output. - final bool verbose; - - /// The root of the git repo. - final io.Directory root; - - /// The delegate to use for running processes. - final ProcessManager _processManager; - - List? _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> get changedFiles async => - _changedFiles ??= await _getChangedFiles(); - - List? _changedFilesAtHead; - - /// Returns a list of non-deleted files which differ between the HEAD - /// commit and its parent. - Future> get changedFilesAtHead async => - _changedFilesAtHead ??= await _getChangedFilesAtHead(); - - Future> _getChangedFiles() async { - final ProcessRunner processRunner = ProcessRunner( - defaultWorkingDirectory: root, - processManager: _processManager, - ); - await _fetch(processRunner); - ProcessRunnerResult mergeBaseResult = await processRunner.runProcess( - ['git', 'merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'], - failOk: true, - ); - if (mergeBaseResult.exitCode != 0) { - mergeBaseResult = await processRunner.runProcess([ - 'git', - 'merge-base', - 'FETCH_HEAD', - 'HEAD', - ]); - } - final String mergeBase = mergeBaseResult.stdout.trim(); - final ProcessRunnerResult masterResult = await processRunner - .runProcess([ - 'git', - 'diff', - '--name-only', - '--diff-filter=ACMRT', - mergeBase, - ]); - return _gitOutputToList(masterResult); - } - - Future> _getChangedFilesAtHead() async { - final ProcessRunner processRunner = ProcessRunner( - defaultWorkingDirectory: root, - ); - await _fetch(processRunner); - final ProcessRunnerResult diffTreeResult = await processRunner.runProcess( - [ - 'git', - 'diff-tree', - '--no-commit-id', - '--name-only', - '--diff-filter=ACMRT', // Added, copied, modified, renamed, or type-changed. - '-r', - 'HEAD', - ], - ); - return _gitOutputToList(diffTreeResult); - } - - Future _fetch(ProcessRunner processRunner) async { - final ProcessRunnerResult fetchResult = await processRunner.runProcess( - ['git', 'fetch', 'upstream', 'main'], - failOk: true, - ); - if (fetchResult.exitCode != 0) { - await processRunner.runProcess([ - 'git', - 'fetch', - 'origin', - 'main', - ]); - } - } - - List _gitOutputToList(ProcessRunnerResult result) { - final String diffOutput = result.stdout.trim(); - if (verbose) { - print('git diff output:\n$diffOutput'); - } - final Set resultMap = {}; - resultMap.addAll(diffOutput.split('\n').where( - (String str) => str.isNotEmpty, - )); - return resultMap.map( - (String filePath) => io.File(path.join(root.path, filePath)), - ).toList(); - } -} diff --git a/tools/clang_tidy/pubspec.yaml b/tools/clang_tidy/pubspec.yaml index e869ef5f41684..d4d96edde076d 100644 --- a/tools/clang_tidy/pubspec.yaml +++ b/tools/clang_tidy/pubspec.yaml @@ -18,6 +18,7 @@ environment: dependencies: args: any + git_repo_tools: any meta: any path: any process: any @@ -27,6 +28,7 @@ dev_dependencies: async_helper: any expect: any litetest: any + process_fakes: any smith: any dependency_overrides: @@ -42,6 +44,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: @@ -52,6 +56,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: diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index 5fcdd984a73e0..84f4d1ca2f425 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -10,9 +10,9 @@ import 'package:clang_tidy/src/options.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 { diff --git a/tools/clang_tidy/test/process_fakes.dart b/tools/clang_tidy/test/process_fakes.dart deleted file mode 100644 index 7bc5fe5778e1f..0000000000000 --- a/tools/clang_tidy/test/process_fakes.dart +++ /dev/null @@ -1,117 +0,0 @@ -// 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. - -import 'dart:convert'; -import 'dart:io' as io; - -import 'package:process/process.dart'; - -/// A fake implementation of [ProcessManager] that allows control for testing. -final class FakeProcessManager implements ProcessManager { - FakeProcessManager({ - io.ProcessResult Function(List command) onRun = unhandledRun, - io.Process Function(List command) onStart = unhandledStart, - }) : _onRun = onRun, _onStart = onStart; - - static io.ProcessResult unhandledRun(List command) { - throw UnsupportedError('Unhandled run: ${command.join(' ')}'); - } - - static io.Process unhandledStart(List command) { - throw UnsupportedError('Unhandled start: ${command.join(' ')}'); - } - - final io.ProcessResult Function(List command) _onRun; - final io.Process Function(List command) _onStart; - - @override - bool canRun(Object? executable, {String? workingDirectory}) => true; - - @override - bool killPid(int pid, [io.ProcessSignal signal = io.ProcessSignal.sigterm]) => true; - - @override - Future run( - List command, { - String? workingDirectory, - Map? environment, - bool includeParentEnvironment = true, - bool runInShell = false, - Encoding stdoutEncoding = io.systemEncoding, - Encoding stderrEncoding = io.systemEncoding, - }) async { - return runSync( - command, - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - stdoutEncoding: stdoutEncoding, - stderrEncoding: stderrEncoding, - ); - } - - @override - io.ProcessResult runSync( - List command, { - String? workingDirectory, - Map? environment, - bool includeParentEnvironment = true, - bool runInShell = false, - Encoding stdoutEncoding = io.systemEncoding, - Encoding stderrEncoding = io.systemEncoding, - }) { - return _onRun(command.map((Object o) => '$o').toList()); - } - - @override - Future start( - List command, { - String? workingDirectory, - Map? environment, - bool includeParentEnvironment = true, - bool runInShell = false, - io.ProcessStartMode mode = io.ProcessStartMode.normal, - }) async { - return _onStart(command.map((Object o) => '$o').toList()); - } -} - -/// An incomplete fake of [io.Process] that allows control for testing. -final class FakeProcess implements io.Process { - /// Creates a fake process that returns the given [exitCode] and out/err. - FakeProcess({ - int exitCode = 0, - String stdout = '', - String stderr = '', - }) : _exitCode = exitCode, - _stdout = stdout, - _stderr = stderr; - - final int _exitCode; - final String _stdout; - final String _stderr; - - @override - Future get exitCode async => _exitCode; - - @override - bool kill([io.ProcessSignal signal = io.ProcessSignal.sigterm]) => true; - - @override - int get pid => 0; - - @override - Stream> get stderr { - return Stream>.fromIterable(>[io.systemEncoding.encoder.convert(_stderr)]); - } - - @override - io.IOSink get stdin => throw UnimplementedError(); - - @override - Stream> get stdout { - return Stream>.fromIterable(>[io.systemEncoding.encoder.convert(_stdout)]); - } -} From 807ca015cd06ca4b2c8dd8653e4369d1184a847f Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:44:57 -0700 Subject: [PATCH 10/16] Update pubspec. --- tools/githooks/pubspec.yaml | 2 ++ tools/pub_get_offline.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tools/githooks/pubspec.yaml b/tools/githooks/pubspec.yaml index a0a6a9789b8cb..1f7a38368d664 100644 --- a/tools/githooks/pubspec.yaml +++ b/tools/githooks/pubspec.yaml @@ -43,6 +43,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: diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index 17f529c61c805..931db7319b8f9 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -44,6 +44,8 @@ os.path.join(ENGINE_DIR, 'tools', 'path_ops', 'dart'), os.path.join(ENGINE_DIR, 'tools', 'pkg', 'engine_build_configs'), os.path.join(ENGINE_DIR, 'tools', 'pkg', 'engine_repo_tools'), + os.path.join(ENGINE_DIR, 'tools', 'pkg', 'git_repo_tools'), + os.path.join(ENGINE_DIR, 'tools', 'pkg', 'process_fakes'), ] From 79576add96480773ffc8874dd0209323453f84ae Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 18 Sep 2023 18:48:26 -0700 Subject: [PATCH 11/16] Update run_tests.py. --- testing/run_tests.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 6a807c65021a6..9c4a6c254dbc3 100755 --- a/testing/run_tests.py +++ b/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)) From a19efec0b717815dcc8f7c7ddd79801578e3fc17 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 20 Sep 2023 09:57:18 -0700 Subject: [PATCH 12/16] Update pubspec overrides. --- tools/pkg/git_repo_tools/pubspec.yaml | 2 ++ tools/pkg/process_fakes/pubspec.yaml | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/tools/pkg/git_repo_tools/pubspec.yaml b/tools/pkg/git_repo_tools/pubspec.yaml index 3f9b5abeb153d..b5b3ed1dab6cd 100644 --- a/tools/pkg/git_repo_tools/pubspec.yaml +++ b/tools/pkg/git_repo_tools/pubspec.yaml @@ -40,6 +40,8 @@ dependency_overrides: 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: diff --git a/tools/pkg/process_fakes/pubspec.yaml b/tools/pkg/process_fakes/pubspec.yaml index 3d45d8ef0fa44..f8a7fbf1b7599 100644 --- a/tools/pkg/process_fakes/pubspec.yaml +++ b/tools/pkg/process_fakes/pubspec.yaml @@ -20,5 +20,13 @@ dependencies: process: any dependency_overrides: + file: + path: ../../../../third_party/pkg/file/packages/file + 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 From 56a43b385d62471ac597d317131d56859ccbdb0e Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 20 Sep 2023 12:33:33 -0700 Subject: [PATCH 13/16] Update. --- tools/pkg/process_fakes/pubspec.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/pkg/process_fakes/pubspec.yaml b/tools/pkg/process_fakes/pubspec.yaml index f8a7fbf1b7599..8ac6e85aa88fe 100644 --- a/tools/pkg/process_fakes/pubspec.yaml +++ b/tools/pkg/process_fakes/pubspec.yaml @@ -17,6 +17,10 @@ environment: # relative to this directory into //third_party/dart dependencies: + file: any + meta: any + path: any + platform: any process: any dependency_overrides: From 69c6ce9bc63a39b44ef78d0b7d7bb12a0e25f597 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 20 Sep 2023 13:35:59 -0700 Subject: [PATCH 14/16] Fix pubspec. --- tools/pkg/git_repo_tools/pubspec.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/pkg/git_repo_tools/pubspec.yaml b/tools/pkg/git_repo_tools/pubspec.yaml index b5b3ed1dab6cd..dcf6f292749c8 100644 --- a/tools/pkg/git_repo_tools/pubspec.yaml +++ b/tools/pkg/git_repo_tools/pubspec.yaml @@ -34,6 +34,8 @@ dependency_overrides: path: ../../../../third_party/dart/pkg/async_helper expect: path: ../../../../third_party/dart/pkg/expect + file: + path: ../../../../third_party/pkg/file/packages/file litetest: path: ../../../testing/litetest meta: From 1b67e56a52f7cca08df6d24cbd791bd3b6adb54c Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 21 Sep 2023 10:04:46 -0700 Subject: [PATCH 15/16] Add pkg/async. --- tools/pkg/git_repo_tools/pubspec.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/pkg/git_repo_tools/pubspec.yaml b/tools/pkg/git_repo_tools/pubspec.yaml index dcf6f292749c8..ff7a3f11b0685 100644 --- a/tools/pkg/git_repo_tools/pubspec.yaml +++ b/tools/pkg/git_repo_tools/pubspec.yaml @@ -30,6 +30,8 @@ dev_dependencies: smith: any dependency_overrides: + async: + path: ../../../../third_party/dart/third_party/pkg/async async_helper: path: ../../../../third_party/dart/pkg/async_helper expect: From 85aefe23e1107870f7bca6803ea2d7cc8968979b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 21 Sep 2023 10:49:52 -0700 Subject: [PATCH 16/16] Tweak. --- tools/pkg/git_repo_tools/pubspec.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/pkg/git_repo_tools/pubspec.yaml b/tools/pkg/git_repo_tools/pubspec.yaml index ff7a3f11b0685..1cf937a42d22d 100644 --- a/tools/pkg/git_repo_tools/pubspec.yaml +++ b/tools/pkg/git_repo_tools/pubspec.yaml @@ -30,10 +30,14 @@ dev_dependencies: 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: