From d0bad673ccdfecfca9828d13187863ed73e6eb16 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Thu, 15 Dec 2022 14:18:33 -0800 Subject: [PATCH 1/2] If clang-format does not run, fall back to other executables in PATH --- script/tool/CHANGELOG.md | 6 ++- .../src/create_all_packages_app_command.dart | 2 +- script/tool/lib/src/format_command.dart | 53 +++++++++++++++---- script/tool/lib/src/main.dart | 2 +- script/tool/pubspec.yaml | 2 +- script/tool/test/format_command_test.dart | 42 +++++++++++++++ 6 files changed, 94 insertions(+), 13 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 021259083af8..072d661f8d99 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,4 +1,8 @@ -## 13.1 +## 0.13.2 + +* Falls back to other executables in PATH when `clang-format` does not run. + +## 0.13.1 * Updates `version-check` to recognize Pigeon's platform test structure. * Pins `package:git` dependency to `2.0.x` until `dart >=2.18.0` becomes our diff --git a/script/tool/lib/src/create_all_packages_app_command.dart b/script/tool/lib/src/create_all_packages_app_command.dart index 142a992972ca..12ec17da139a 100644 --- a/script/tool/lib/src/create_all_packages_app_command.dart +++ b/script/tool/lib/src/create_all_packages_app_command.dart @@ -230,7 +230,7 @@ class CreateAllPackagesAppCommand extends PackageCommand { String _pubspecToString(Pubspec pubspec) { return ''' -### Generated file. Do not edit. Run `pub global run flutter_plugin_tools gen-pubspec` to update. +### Generated file. Do not edit. Run `dart pub global run flutter_plugin_tools gen-pubspec` to update. name: ${pubspec.name} description: ${pubspec.description} publish_to: none diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index cc6936c566e1..5c06fa8d0ebf 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -104,8 +104,8 @@ class FormatCommand extends PackageCommand { print('These files are not formatted correctly (see diff below):'); LineSplitter.split(stdout).map((String line) => ' $line').forEach(print); - print('\nTo fix run "pub global activate flutter_plugin_tools && ' - 'pub global run flutter_plugin_tools format" or copy-paste ' + print('\nTo fix run "dart pub global activate flutter_plugin_tools && ' + 'dart pub global run flutter_plugin_tools format" or copy-paste ' 'this command into your terminal:'); final io.ProcessResult diff = await processRunner.run( @@ -128,16 +128,11 @@ class FormatCommand extends PackageCommand { final Iterable clangFiles = _getPathsWithExtensions( files, {'.h', '.m', '.mm', '.cc', '.cpp'}); if (clangFiles.isNotEmpty) { - final String clangFormat = getStringArg('clang-format'); - if (!await _hasDependency(clangFormat)) { - printError('Unable to run "clang-format". Make sure that it is in your ' - 'path, or provide a full path with --clang-format.'); - throw ToolExit(_exitDependencyMissing); - } + final String clangFormat = await _findValidClangFormat(); print('Formatting .cc, .cpp, .h, .m, and .mm files...'); final int exitCode = await _runBatched( - getStringArg('clang-format'), ['-i', '--style=file'], + clangFormat, ['-i', '--style=file'], files: clangFiles); if (exitCode != 0) { printError( @@ -147,6 +142,26 @@ class FormatCommand extends PackageCommand { } } + Future _findValidClangFormat() async { + final String clangFormatArg = getStringArg('clang-format'); + if (await _hasDependency(clangFormatArg)) { + return clangFormatArg; + } + + // There is a known issue where "chromium/depot_tools/clang-format" + // fails with "Problem while looking for clang-format in Chromium source tree". + // Loop through all "clang-format"s in PATH until a working one is found, + // for example "/usr/local/bin/clang-format" or a "brew" installed version. + for (final String clangFormatPath in await _whichAll('clang-format')) { + if (await _hasDependency(clangFormatPath)) { + return clangFormatPath; + } + } + printError('Unable to run "clang-format". Make sure that it is in your ' + 'path, or provide a full path with --clang-format.'); + throw ToolExit(_exitDependencyMissing); + } + Future _formatJava( Iterable files, String googleFormatterPath) async { final Iterable javaFiles = @@ -279,6 +294,26 @@ class FormatCommand extends PackageCommand { return true; } + /// Returns all instances of [command] executable found on user path. + Future> _whichAll(String command) async { + try { + final io.ProcessResult result = + await processRunner.run('which', ['-a', command]); + + if (result.exitCode != 0) { + return []; + } + + final String stdout = result.stdout as String; + if (stdout.isEmpty) { + return []; + } + return stdout.trim().split('\n').toList(); + } on io.ProcessException { + return []; + } + } + /// Runs [command] on [arguments] on all of the files in [files], batched as /// necessary to avoid OS command-line length limits. /// diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 2d48f079b306..d5ab7f88089e 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -52,7 +52,7 @@ void main(List args) { } final CommandRunner commandRunner = CommandRunner( - 'pub global run flutter_plugin_tools', + 'dart pub global run flutter_plugin_tools', 'Productivity utils for hosting multiple plugins within one repository.') ..addCommand(AnalyzeCommand(packagesDir)) ..addCommand(BuildExamplesCommand(packagesDir)) diff --git a/script/tool/pubspec.yaml b/script/tool/pubspec.yaml index 2879ff125020..ea20364d51d5 100644 --- a/script/tool/pubspec.yaml +++ b/script/tool/pubspec.yaml @@ -1,7 +1,7 @@ name: flutter_plugin_tools description: Productivity utils for flutter/plugins and flutter/packages repository: https://github.com/flutter/plugins/tree/main/script/tool -version: 0.13.1 +version: 0.13.2 dependencies: args: ^2.1.0 diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index 9a865053a2b6..04b08ba46820 100644 --- a/script/tool/test/format_command_test.dart +++ b/script/tool/test/format_command_test.dart @@ -332,6 +332,48 @@ void main() { ])); }); + test('falls back to working clang-format in the path', () async { + const List files = [ + 'linux/foo_plugin.cc', + 'macos/Classes/Foo.h', + ]; + final RepositoryPackage plugin = createFakePlugin( + 'a_plugin', + packagesDir, + extraFiles: files, + ); + + processRunner.mockProcessesForExecutable['clang-format'] = [ + MockProcess(exitCode: 1) + ]; + processRunner.mockProcessesForExecutable['which'] = [ + MockProcess( + stdout: '/usr/local/bin/clang-format\n/path/to/working-clang-format') + ]; + processRunner.mockProcessesForExecutable['/usr/local/bin/clang-format'] = + [MockProcess(exitCode: 1)]; + Error? commandError; + final List output = await runCapturingPrint( + runner, ['format'], errorHandler: (Error e) { + commandError = e; + }); + + expect( + processRunner.recordedCalls, + containsAll([ + const ProcessCall( + '/path/to/working-clang-format', ['--version'], null), + ProcessCall( + '/path/to/working-clang-format', + [ + '-i', + '--style=file', + ...getPackagesDirRelativePaths(plugin, files) + ], + packagesDir.path), + ])); + }); + test('honors --clang-format flag', () async { const List files = [ 'windows/foo_plugin.cpp', From 52cbffcd6206097376ebd026f823253d5068a356 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Fri, 16 Dec 2022 10:08:17 -0800 Subject: [PATCH 2/2] Review edits --- script/tool/lib/src/format_command.dart | 4 ++-- script/tool/test/format_command_test.dart | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index 5c06fa8d0ebf..8198f6d36abd 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -304,11 +304,11 @@ class FormatCommand extends PackageCommand { return []; } - final String stdout = result.stdout as String; + final String stdout = result.stdout.trim() as String; if (stdout.isEmpty) { return []; } - return stdout.trim().split('\n').toList(); + return LineSplitter.split(stdout).toList(); } on io.ProcessException { return []; } diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index 04b08ba46820..1aadafbd3d82 100644 --- a/script/tool/test/format_command_test.dart +++ b/script/tool/test/format_command_test.dart @@ -352,11 +352,7 @@ void main() { ]; processRunner.mockProcessesForExecutable['/usr/local/bin/clang-format'] = [MockProcess(exitCode: 1)]; - Error? commandError; - final List output = await runCapturingPrint( - runner, ['format'], errorHandler: (Error e) { - commandError = e; - }); + await runCapturingPrint(runner, ['format']); expect( processRunner.recordedCalls,