Skip to content

Commit

Permalink
[flutter_plugin_tools] If clang-format does not run, fall back to o…
Browse files Browse the repository at this point in the history
…ther executables in PATH (flutter#6853)

* If clang-format does not run, fall back to other executables in PATH

* Review edits
  • Loading branch information
jmagman authored Dec 16, 2022
1 parent fd2841f commit abc9f9a
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 13 deletions.
6 changes: 5 additions & 1 deletion script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion script/tool/lib/src/create_all_packages_app_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 44 additions & 9 deletions script/tool/lib/src/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -128,16 +128,11 @@ class FormatCommand extends PackageCommand {
final Iterable<String> clangFiles = _getPathsWithExtensions(
files, <String>{'.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'), <String>['-i', '--style=file'],
clangFormat, <String>['-i', '--style=file'],
files: clangFiles);
if (exitCode != 0) {
printError(
Expand All @@ -147,6 +142,26 @@ class FormatCommand extends PackageCommand {
}
}

Future<String> _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<void> _formatJava(
Iterable<String> files, String googleFormatterPath) async {
final Iterable<String> javaFiles =
Expand Down Expand Up @@ -279,6 +294,26 @@ class FormatCommand extends PackageCommand {
return true;
}

/// Returns all instances of [command] executable found on user path.
Future<List<String>> _whichAll(String command) async {
try {
final io.ProcessResult result =
await processRunner.run('which', <String>['-a', command]);

if (result.exitCode != 0) {
return <String>[];
}

final String stdout = result.stdout.trim() as String;
if (stdout.isEmpty) {
return <String>[];
}
return LineSplitter.split(stdout).toList();
} on io.ProcessException {
return <String>[];
}
}

/// Runs [command] on [arguments] on all of the files in [files], batched as
/// necessary to avoid OS command-line length limits.
///
Expand Down
2 changes: 1 addition & 1 deletion script/tool/lib/src/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void main(List<String> args) {
}

final CommandRunner<void> commandRunner = CommandRunner<void>(
'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))
Expand Down
2 changes: 1 addition & 1 deletion script/tool/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
38 changes: 38 additions & 0 deletions script/tool/test/format_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,44 @@ void main() {
]));
});

test('falls back to working clang-format in the path', () async {
const List<String> files = <String>[
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
final RepositoryPackage plugin = createFakePlugin(
'a_plugin',
packagesDir,
extraFiles: files,
);

processRunner.mockProcessesForExecutable['clang-format'] = <io.Process>[
MockProcess(exitCode: 1)
];
processRunner.mockProcessesForExecutable['which'] = <io.Process>[
MockProcess(
stdout: '/usr/local/bin/clang-format\n/path/to/working-clang-format')
];
processRunner.mockProcessesForExecutable['/usr/local/bin/clang-format'] =
<io.Process>[MockProcess(exitCode: 1)];
await runCapturingPrint(runner, <String>['format']);

expect(
processRunner.recordedCalls,
containsAll(<ProcessCall>[
const ProcessCall(
'/path/to/working-clang-format', <String>['--version'], null),
ProcessCall(
'/path/to/working-clang-format',
<String>[
'-i',
'--style=file',
...getPackagesDirRelativePaths(plugin, files)
],
packagesDir.path),
]));
});

test('honors --clang-format flag', () async {
const List<String> files = <String>[
'windows/foo_plugin.cpp',
Expand Down

0 comments on commit abc9f9a

Please sign in to comment.