Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move git_repo_tools and process_fakes outside of clang_tidy. #46017

Merged
merged 26 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
16d75ed
Keep tools/clang_tidy/lib/src/git_repo.dart
matanlurey Sep 18, 2023
7820e1b
Copy tools/clang_tidy/lib/src/git_repo.dart into tools/pkg/git_repo_t…
matanlurey Sep 18, 2023
bfceab1
Duplicate tools/clang_tidy/lib/src/git_repo.dart history.
matanlurey Sep 18, 2023
6e3fbf2
Set back tools/clang_tidy/lib/src/git_repo.dart file
matanlurey Sep 18, 2023
464578a
Start working on new sub-package.
matanlurey Sep 18, 2023
ec556e0
Keep tools/clang_tidy/test/process_fakes.dart
matanlurey Sep 19, 2023
2966a5b
Copy tools/clang_tidy/test/process_fakes.dart into tools/pkg/process_…
matanlurey Sep 19, 2023
2f8adff
Duplicate tools/clang_tidy/test/process_fakes.dart history.
matanlurey Sep 19, 2023
0353b39
Set back tools/clang_tidy/test/process_fakes.dart file
matanlurey Sep 19, 2023
275d920
Move process_fakes, add some test for git_repo_tools.
matanlurey Sep 19, 2023
b53b561
Fix pubspecs and imports.
matanlurey Sep 19, 2023
807ca01
Update pubspec.
matanlurey Sep 19, 2023
79576ad
Update run_tests.py.
matanlurey Sep 19, 2023
aac8841
Merge.
matanlurey Sep 19, 2023
65b692a
Merge branch 'main' into engine-tools-git-repo
matanlurey Sep 19, 2023
4706b1a
Merge remote-tracking branch 'upstream/main' into engine-tools-git-repo
matanlurey Sep 20, 2023
a19efec
Update pubspec overrides.
matanlurey Sep 20, 2023
c0c4afd
Merge branch 'main' into engine-tools-git-repo
matanlurey Sep 20, 2023
eccf732
Merge branch 'main' into engine-tools-git-repo
matanlurey Sep 20, 2023
56a43b3
Update.
matanlurey Sep 20, 2023
849b20d
Merge branch 'main' into engine-tools-git-repo
matanlurey Sep 20, 2023
69c6ce9
Fix pubspec.
matanlurey Sep 20, 2023
a525bfb
Merge branch 'main' into engine-tools-git-repo
matanlurey Sep 21, 2023
1b67e56
Add pkg/async.
matanlurey Sep 21, 2023
85aefe2
Tweak.
matanlurey Sep 21, 2023
c7022dc
Merge remote-tracking branch 'upstream/main' into engine-tools-git-repo
matanlurey Sep 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions testing/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions tools/clang_tidy/lib/clang_tidy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '''
Expand Down Expand Up @@ -211,7 +211,7 @@ class ClangTidy {
.toList();
}

final GitRepo repo = GitRepo(
final GitRepo repo = GitRepo.fromRoot(
options.repoPath,
processManager: _processManager,
verbose: options.verbose,
Expand Down
6 changes: 6 additions & 0 deletions tools/clang_tidy/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ environment:

dependencies:
args: any
git_repo_tools: any
engine_repo_tools: any
meta: any
path: any
Expand All @@ -28,6 +29,7 @@ dev_dependencies:
async_helper: any
expect: any
litetest: any
process_fakes: any
smith: any

dependency_overrides:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tools/clang_tidy/test/clang_tidy_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions tools/githooks/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
23 changes: 23 additions & 0 deletions tools/pkg/git_repo_tools/README.md
Original file line number Diff line number Diff line change
@@ -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}');
}
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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(
Expand All @@ -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',
Expand All @@ -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(
Expand Down
60 changes: 60 additions & 0 deletions tools/pkg/git_repo_tools/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
matanlurey marked this conversation as resolved.
Show resolved Hide resolved
process_fakes:
path: ../process_fakes
process_runner:
path: ../../../../third_party/pkg/process_runner
smith:
path: ../../../../third_party/dart/pkg/smith
Loading