mirror of
https://github.com/flutter/flutter.git
synced 2026-02-20 02:29:02 +08:00
This makes the lint script use multiprocessing to speed it up. (#19987)
I got tired of waiting for it to run, so I added some of the "worker" queue code that I wrote for the assets-for-api-docs generator. I also tried out putting all the files in one call to clang-tidy with the -p argument, but that was still a lot slower because it runs them all on one core. This runs separate jobs for each file, simultaneously, and then reports the results at the end (associated with each file, of course).
This commit is contained in:
Greg Spencer
GitHub
parent
a6cd3ebc61
commit
280bbfc763
256
ci/bin/lint.dart
Normal file
256
ci/bin/lint.dart
Normal file
@ -0,0 +1,256 @@
|
||||
/// Runs clang-tidy on files with changes.
|
||||
///
|
||||
/// usage:
|
||||
/// dart lint.dart <path to compile_commands.json> <path to git repository> [clang-tidy checks]
|
||||
///
|
||||
/// User environment variable FLUTTER_LINT_ALL to run on all files.
|
||||
|
||||
import 'dart:async' show Completer;
|
||||
import 'dart:convert' show jsonDecode, utf8, LineSplitter;
|
||||
import 'dart:io' show File, exit, Directory, FileSystemEntity, Platform, stderr;
|
||||
|
||||
import 'package:args/args.dart';
|
||||
import 'package:path/path.dart' as path;
|
||||
import 'package:process_runner/process_runner.dart';
|
||||
|
||||
String _linterOutputHeader = '''
|
||||
┌──────────────────────────┐
|
||||
│ Engine Clang Tidy Linter │
|
||||
└──────────────────────────┘
|
||||
The following errors have been reported by the Engine Clang Tidy Linter. For
|
||||
more information on addressing these issues please see:
|
||||
https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter
|
||||
''';
|
||||
|
||||
class Command {
|
||||
Directory directory = Directory('');
|
||||
String command = '';
|
||||
File file = File('');
|
||||
}
|
||||
|
||||
Command parseCommand(Map<String, dynamic> map) {
|
||||
final Directory dir = Directory(map['directory'] as String).absolute;
|
||||
return Command()
|
||||
..directory = dir
|
||||
..command = map['command'] as String
|
||||
..file = File(path.normalize(path.join(dir.path, map['file'] as String)));
|
||||
}
|
||||
|
||||
String calcTidyArgs(Command command) {
|
||||
String result = command.command;
|
||||
result = result.replaceAll(RegExp(r'\S*clang/bin/clang'), '');
|
||||
result = result.replaceAll(RegExp(r'-MF \S*'), '');
|
||||
return result;
|
||||
}
|
||||
|
||||
String calcTidyPath(Command command) {
|
||||
final RegExp regex = RegExp(r'\S*clang/bin/clang');
|
||||
return regex
|
||||
.stringMatch(command.command)
|
||||
?.replaceAll('clang/bin/clang', 'clang/bin/clang-tidy') ??
|
||||
'';
|
||||
}
|
||||
|
||||
bool isNonEmptyString(String str) => str.isNotEmpty;
|
||||
|
||||
bool containsAny(File file, Iterable<File> queries) {
|
||||
return queries.where((File query) => path.equals(query.path, file.path)).isNotEmpty;
|
||||
}
|
||||
|
||||
/// Returns a list of all non-deleted files which differ from the nearest
|
||||
/// merge-base with `master`. If it can't find a fork point, uses the default
|
||||
/// merge-base.
|
||||
Future<List<File>> getListOfChangedFiles(Directory repoPath) async {
|
||||
final ProcessRunner processRunner = ProcessRunner(defaultWorkingDirectory: repoPath);
|
||||
final ProcessRunnerResult fetchResult = await processRunner.runProcess(
|
||||
<String>['git', 'fetch', 'upstream', 'master'],
|
||||
failOk: true,
|
||||
);
|
||||
if (fetchResult.exitCode != 0) {
|
||||
await processRunner.runProcess(<String>['git', 'fetch', 'origin', 'master']);
|
||||
}
|
||||
final Set<String> result = <String>{};
|
||||
ProcessRunnerResult mergeBaseResult = await processRunner.runProcess(
|
||||
<String>['git', 'merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'],
|
||||
failOk: true);
|
||||
if (mergeBaseResult.exitCode != 0) {
|
||||
if (verbose) {
|
||||
stderr.writeln("Didn't find a fork point, falling back to default merge base.");
|
||||
}
|
||||
mergeBaseResult = await processRunner
|
||||
.runProcess(<String>['git', 'merge-base', 'FETCH_HEAD', 'HEAD'], failOk: false);
|
||||
}
|
||||
final String mergeBase = mergeBaseResult.stdout.trim();
|
||||
final ProcessRunnerResult masterResult = await processRunner
|
||||
.runProcess(<String>['git', 'diff', '--name-only', '--diff-filter=ACMRT', mergeBase]);
|
||||
result.addAll(masterResult.stdout.split('\n').where(isNonEmptyString));
|
||||
return result.map<File>((String filePath) => File(path.join(repoPath.path, filePath))).toList();
|
||||
}
|
||||
|
||||
Future<List<File>> dirContents(Directory dir) {
|
||||
final List<File> files = <File>[];
|
||||
final Completer<List<File>> completer = Completer<List<File>>();
|
||||
final Stream<FileSystemEntity> lister = dir.list(recursive: true);
|
||||
lister.listen((FileSystemEntity file) => file is File ? files.add(file) : null,
|
||||
onError: (Object e) => completer.completeError(e), onDone: () => completer.complete(files));
|
||||
return completer.future;
|
||||
}
|
||||
|
||||
File buildFileAsRepoFile(String buildFile, Directory repoPath) {
|
||||
// Removes the "../../flutter" from the build files to make it relative to the flutter
|
||||
// dir.
|
||||
final String relativeBuildFile = path.joinAll(path.split(buildFile).sublist(3));
|
||||
final File result = File(path.join(repoPath.absolute.path, relativeBuildFile));
|
||||
print('Build file: $buildFile => ${result.path}');
|
||||
return result;
|
||||
}
|
||||
|
||||
Future<String> shouldIgnoreFile(File file) async {
|
||||
if (path.split(file.path).contains('third_party')) {
|
||||
return 'third_party';
|
||||
} else {
|
||||
final RegExp exp = RegExp(r'//\s*FLUTTER_NOLINT');
|
||||
await for (String line
|
||||
in file.openRead().transform(utf8.decoder).transform(const LineSplitter())) {
|
||||
if (exp.hasMatch(line)) {
|
||||
return 'FLUTTER_NOLINT';
|
||||
} else if (line.isNotEmpty && line[0] != '\n' && line[0] != '/') {
|
||||
// Quick out once we find a line that isn't empty or a comment. The
|
||||
// FLUTTER_NOLINT must show up before the first real code.
|
||||
return '';
|
||||
}
|
||||
}
|
||||
return '';
|
||||
}
|
||||
}
|
||||
|
||||
void _usage(ArgParser parser, {int exitCode = 1}) {
|
||||
stderr.writeln('lint.dart [--help] [--lint-all] [--verbose] [--diff-branch]');
|
||||
stderr.writeln(parser.usage);
|
||||
exit(exitCode);
|
||||
}
|
||||
|
||||
bool verbose = false;
|
||||
|
||||
void main(List<String> arguments) async {
|
||||
final ArgParser parser = ArgParser();
|
||||
parser.addFlag('help', help: 'Print help.');
|
||||
parser.addFlag('lint-all',
|
||||
help: 'lint all of the sources, regardless of FLUTTER_NOLINT.', defaultsTo: false);
|
||||
parser.addFlag('verbose', help: 'Print verbose output.', defaultsTo: verbose);
|
||||
parser.addOption('repo', help: 'Use the given path as the repo path');
|
||||
parser.addOption('compile-commands',
|
||||
help: 'Use the given path as the source of compile_commands.json. This '
|
||||
'file is created by running tools/gn');
|
||||
parser.addOption('checks',
|
||||
help: 'Perform the given checks on the code. Defaults to the empty '
|
||||
'string, indicating all checks should be performed.',
|
||||
defaultsTo: '');
|
||||
final ArgResults options = parser.parse(arguments);
|
||||
|
||||
verbose = options['verbose'] as bool;
|
||||
|
||||
if (options['help'] as bool) {
|
||||
_usage(parser, exitCode: 0);
|
||||
}
|
||||
|
||||
if (!options.wasParsed('compile-commands')) {
|
||||
stderr.writeln('ERROR: The --compile-commands argument is requried.');
|
||||
_usage(parser);
|
||||
}
|
||||
|
||||
if (!options.wasParsed('repo')) {
|
||||
stderr.writeln('ERROR: The --repo argument is requried.');
|
||||
_usage(parser);
|
||||
}
|
||||
|
||||
final File buildCommandsPath = File(options['compile-commands'] as String);
|
||||
if (!buildCommandsPath.existsSync()) {
|
||||
stderr.writeln("ERROR: Build commands path ${buildCommandsPath.absolute.path} doesn't exist.");
|
||||
_usage(parser);
|
||||
}
|
||||
|
||||
final Directory repoPath = Directory(options['repo'] as String);
|
||||
if (!repoPath.existsSync()) {
|
||||
stderr.writeln("ERROR: Repo path ${repoPath.absolute.path} doesn't exist.");
|
||||
_usage(parser);
|
||||
}
|
||||
|
||||
print(_linterOutputHeader);
|
||||
|
||||
final String checksArg = options.wasParsed('checks') ? options['checks'] as String : '';
|
||||
final String checks = checksArg.isNotEmpty ? '--checks=$checksArg' : '--config=';
|
||||
final bool lintAll =
|
||||
Platform.environment['FLUTTER_LINT_ALL'] != null || options['lint-all'] as bool;
|
||||
final List<File> changedFiles =
|
||||
lintAll ? await dirContents(repoPath) : await getListOfChangedFiles(repoPath);
|
||||
|
||||
if (verbose) {
|
||||
print('Checking lint in repo at $repoPath.');
|
||||
if (checksArg.isNotEmpty) {
|
||||
print('Checking for specific checks: $checks.');
|
||||
}
|
||||
if (lintAll) {
|
||||
print('Checking all ${changedFiles.length} files the repo dir.');
|
||||
} else {
|
||||
print('Dectected ${changedFiles.length} files that have changed');
|
||||
}
|
||||
}
|
||||
|
||||
final List<dynamic> buildCommandMaps =
|
||||
jsonDecode(await buildCommandsPath.readAsString()) as List<dynamic>;
|
||||
final List<Command> buildCommands = buildCommandMaps
|
||||
.map<Command>((dynamic x) => parseCommand(x as Map<String, dynamic>))
|
||||
.toList();
|
||||
final Command firstCommand = buildCommands[0];
|
||||
final String tidyPath = calcTidyPath(firstCommand);
|
||||
assert(tidyPath.isNotEmpty);
|
||||
final List<Command> changedFileBuildCommands =
|
||||
buildCommands.where((Command x) => containsAny(x.file, changedFiles)).toList();
|
||||
|
||||
if (changedFileBuildCommands.isEmpty) {
|
||||
print('No changed files that have build commands associated with them '
|
||||
'were found.');
|
||||
exit(0);
|
||||
}
|
||||
|
||||
if (verbose) {
|
||||
print('Found ${changedFileBuildCommands.length} files that have build '
|
||||
'commands associated with them and can be lint checked.');
|
||||
}
|
||||
|
||||
int exitCode = 0;
|
||||
final List<WorkerJob> jobs = <WorkerJob>[];
|
||||
for (Command command in changedFileBuildCommands) {
|
||||
final String relativePath = path.relative(command.file.path, from: repoPath.parent.path);
|
||||
final String ignoreReason = await shouldIgnoreFile(command.file);
|
||||
if (ignoreReason.isEmpty) {
|
||||
final String tidyArgs = calcTidyArgs(command);
|
||||
final List<String> args = <String>[command.file.path, checks, '--'];
|
||||
args.addAll(tidyArgs?.split(' ') ?? <String>[]);
|
||||
print('🔶 linting $relativePath');
|
||||
jobs.add(WorkerJob(
|
||||
<String>[tidyPath, ...args],
|
||||
workingDirectory: command.directory,
|
||||
name: 'clang-tidy on ${command.file.path}',
|
||||
));
|
||||
} else {
|
||||
print('🔷 ignoring $relativePath ($ignoreReason)');
|
||||
}
|
||||
}
|
||||
final ProcessPool pool = ProcessPool();
|
||||
|
||||
await for (final WorkerJob job in pool.startWorkers(jobs)) {
|
||||
if (job.result.stdout.isEmpty) {
|
||||
continue;
|
||||
}
|
||||
print('❌ Failures for ${job.name}:');
|
||||
print(job.result.stdout);
|
||||
exitCode = 1;
|
||||
}
|
||||
print('\n');
|
||||
if (exitCode == 0) {
|
||||
print('No lint problems found.');
|
||||
}
|
||||
exit(exitCode);
|
||||
}
|
||||
168
ci/lint.dart
168
ci/lint.dart
@ -1,168 +0,0 @@
|
||||
/// Runs clang-tidy on files with changes.
|
||||
///
|
||||
/// usage:
|
||||
/// dart lint.dart <path to compile_commands.json> <path to git repository> [clang-tidy checks]
|
||||
///
|
||||
/// User environment variable FLUTTER_LINT_ALL to run on all files.
|
||||
|
||||
import 'dart:io'
|
||||
show
|
||||
File,
|
||||
Process,
|
||||
ProcessResult,
|
||||
exit,
|
||||
Directory,
|
||||
FileSystemEntity,
|
||||
Platform;
|
||||
import 'dart:convert' show jsonDecode, utf8, LineSplitter;
|
||||
import 'dart:async' show Completer;
|
||||
|
||||
String _linterOutputHeader = '''┌──────────────────────────┐
|
||||
│ Engine Clang Tidy Linter │
|
||||
└──────────────────────────┘
|
||||
The following errors have been reported by the Engine Clang Tidy Linter. For
|
||||
more information on addressing these issues please see:
|
||||
https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter
|
||||
''';
|
||||
|
||||
class Command {
|
||||
String directory;
|
||||
String command;
|
||||
String file;
|
||||
}
|
||||
|
||||
Command parseCommand(Map<String, dynamic> map) {
|
||||
return Command()
|
||||
..directory = map['directory']
|
||||
..command = map['command']
|
||||
..file = map['file'];
|
||||
}
|
||||
|
||||
String calcTidyArgs(Command command) {
|
||||
String result = command.command;
|
||||
result = result.replaceAll(RegExp(r'\S*clang/bin/clang'), '');
|
||||
result = result.replaceAll(RegExp(r'-MF \S*'), '');
|
||||
return result;
|
||||
}
|
||||
|
||||
String calcTidyPath(Command command) {
|
||||
final RegExp regex = RegExp(r'\S*clang/bin/clang');
|
||||
return regex
|
||||
.stringMatch(command.command)
|
||||
.replaceAll('clang/bin/clang', 'clang/bin/clang-tidy');
|
||||
}
|
||||
|
||||
bool isNonEmptyString(String str) => str.length > 0;
|
||||
|
||||
bool containsAny(String str, List<String> queries) {
|
||||
for (String query in queries) {
|
||||
if (str.contains(query)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/// Returns a list of all files with current changes or differ from `master`.
|
||||
List<String> getListOfChangedFiles(String repoPath) {
|
||||
final Set<String> result = Set<String>();
|
||||
final ProcessResult diffResult = Process.runSync(
|
||||
'git', ['diff', '--name-only'],
|
||||
workingDirectory: repoPath);
|
||||
final ProcessResult diffCachedResult = Process.runSync(
|
||||
'git', ['diff', '--cached', '--name-only'],
|
||||
workingDirectory: repoPath);
|
||||
|
||||
final ProcessResult fetchResult =
|
||||
Process.runSync('git', ['fetch', 'upstream', 'master']);
|
||||
if (fetchResult.exitCode != 0) {
|
||||
Process.runSync('git', ['fetch', 'origin', 'master']);
|
||||
}
|
||||
final ProcessResult mergeBaseResult = Process.runSync(
|
||||
'git', ['merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'],
|
||||
workingDirectory: repoPath);
|
||||
final String mergeBase = mergeBaseResult.stdout.trim();
|
||||
final ProcessResult masterResult = Process.runSync(
|
||||
'git', ['diff', '--name-only', mergeBase],
|
||||
workingDirectory: repoPath);
|
||||
result.addAll(diffResult.stdout.split('\n').where(isNonEmptyString));
|
||||
result.addAll(diffCachedResult.stdout.split('\n').where(isNonEmptyString));
|
||||
result.addAll(masterResult.stdout.split('\n').where(isNonEmptyString));
|
||||
return result.toList();
|
||||
}
|
||||
|
||||
Future<List<String>> dirContents(String repoPath) {
|
||||
Directory dir = Directory(repoPath);
|
||||
var files = <String>[];
|
||||
var completer = new Completer<List<String>>();
|
||||
var lister = dir.list(recursive: true);
|
||||
lister.listen((FileSystemEntity file) => files.add(file.path),
|
||||
// should also register onError
|
||||
onDone: () => completer.complete(files));
|
||||
return completer.future;
|
||||
}
|
||||
|
||||
Future<bool> shouldIgnoreFile(String path) async {
|
||||
if (path.contains('/third_party/')) {
|
||||
return true;
|
||||
} else {
|
||||
final RegExp exp = RegExp(r'//.*FLUTTER_NOLINT');
|
||||
await for (String line in File(path.substring(6))
|
||||
.openRead()
|
||||
.transform(utf8.decoder)
|
||||
.transform(const LineSplitter())) {
|
||||
if (exp.hasMatch(line)) {
|
||||
return true;
|
||||
} else if (line.length > 0 && line[0] != '\n' && line[0] != '/') {
|
||||
// Quick out once we find a line that isn't empty or a comment. The
|
||||
// FLUTTER_NOLINT must show up before the first real code.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
void main(List<String> arguments) async {
|
||||
final String buildCommandsPath = arguments[0];
|
||||
final String repoPath = arguments[1];
|
||||
final String checks =
|
||||
arguments.length >= 3 ? '--checks=${arguments[2]}' : '--config=';
|
||||
final List<String> changedFiles =
|
||||
Platform.environment['FLUTTER_LINT_ALL'] != null
|
||||
? await dirContents(repoPath)
|
||||
: getListOfChangedFiles(repoPath);
|
||||
/// TODO(gaaclarke): Convert FLUTTER_LINT_ALL to a command-line flag and add
|
||||
/// `--verbose` flag.
|
||||
|
||||
final List<dynamic> buildCommandMaps =
|
||||
jsonDecode(await new File(buildCommandsPath).readAsString());
|
||||
final List<Command> buildCommands =
|
||||
buildCommandMaps.map((x) => parseCommand(x)).toList();
|
||||
final Command firstCommand = buildCommands[0];
|
||||
final String tidyPath = calcTidyPath(firstCommand);
|
||||
final List<Command> changedFileBuildCommands =
|
||||
buildCommands.where((x) => containsAny(x.file, changedFiles)).toList();
|
||||
|
||||
print(_linterOutputHeader);
|
||||
int exitCode = 0;
|
||||
//TODO(aaclarke): Coalesce this into one call using the `-p` arguement.
|
||||
for (Command command in changedFileBuildCommands) {
|
||||
if (!(await shouldIgnoreFile(command.file))) {
|
||||
final String tidyArgs = calcTidyArgs(command);
|
||||
final List<String> args = [command.file, checks, '--'];
|
||||
args.addAll(tidyArgs.split(' '));
|
||||
print('🔶 linting ${command.file}');
|
||||
final Process process = await Process.start(tidyPath, args,
|
||||
workingDirectory: command.directory, runInShell: false);
|
||||
process.stdout.transform(utf8.decoder).listen((data) {
|
||||
print(data);
|
||||
exitCode = 1;
|
||||
});
|
||||
await process.exitCode;
|
||||
} else {
|
||||
print('🔷 ignoring ${command.file}');
|
||||
}
|
||||
}
|
||||
exit(exitCode);
|
||||
}
|
||||
46
ci/lint.sh
46
ci/lint.sh
@ -2,9 +2,47 @@
|
||||
|
||||
set -e
|
||||
|
||||
COMPILE_COMMANDS="out/compile_commands.json"
|
||||
if [ ! -f $COMPILE_COMMANDS ]; then
|
||||
./flutter/tools/gn
|
||||
# Needed because if it is set, cd may print the path it changed to.
|
||||
unset CDPATH
|
||||
|
||||
# On Mac OS, readlink -f doesn't work, so follow_links traverses the path one
|
||||
# link at a time, and then cds into the link destination and find out where it
|
||||
# ends up.
|
||||
#
|
||||
# The returned filesystem path must be a format usable by Dart's URI parser,
|
||||
# since the Dart command line tool treats its argument as a file URI, not a
|
||||
# filename. For instance, multiple consecutive slashes should be reduced to a
|
||||
# single slash, since double-slashes indicate a URI "authority", and these are
|
||||
# supposed to be filenames. There is an edge case where this will return
|
||||
# multiple slashes: when the input resolves to the root directory. However, if
|
||||
# that were the case, we wouldn't be running this shell, so we don't do anything
|
||||
# about it.
|
||||
#
|
||||
# The function is enclosed in a subshell to avoid changing the working directory
|
||||
# of the caller.
|
||||
function follow_links() (
|
||||
cd -P "$(dirname -- "$1")"
|
||||
file="$PWD/$(basename -- "$1")"
|
||||
while [[ -h "$file" ]]; do
|
||||
cd -P "$(dirname -- "$file")"
|
||||
file="$(readlink -- "$file")"
|
||||
cd -P "$(dirname -- "$file")"
|
||||
file="$PWD/$(basename -- "$file")"
|
||||
done
|
||||
echo "$file"
|
||||
)
|
||||
PROG_NAME="$(follow_links "${BASH_SOURCE[0]}")"
|
||||
CI_DIR="$(cd "${PROG_NAME%/*}" ; pwd -P)"
|
||||
SRC_DIR="$(cd "$CI_DIR/../.."; pwd -P)"
|
||||
|
||||
COMPILE_COMMANDS="$SRC_DIR/out/compile_commands.json"
|
||||
if [ ! -f "$COMPILE_COMMANDS" ]; then
|
||||
(cd $SRC_DIR; ./flutter/tools/gn)
|
||||
fi
|
||||
|
||||
dart flutter/ci/lint.dart $COMPILE_COMMANDS flutter/
|
||||
cd "$CI_DIR"
|
||||
pub get && dart \
|
||||
bin/lint.dart \
|
||||
--compile-commands="$COMPILE_COMMANDS" \
|
||||
--repo="$SRC_DIR/flutter" \
|
||||
"$@"
|
||||
|
||||
9
ci/pubspec.yaml
Normal file
9
ci/pubspec.yaml
Normal file
@ -0,0 +1,9 @@
|
||||
name: ci_scripts
|
||||
|
||||
dependencies:
|
||||
args: ^1.6.0
|
||||
path: ^1.7.0
|
||||
process_runner: ^2.0.3
|
||||
|
||||
environment:
|
||||
sdk: '>=2.8.0 <3.0.0'
|
||||
Loading…
x
Reference in New Issue
Block a user