Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tools] If clang-format does not run, fall back to other executables in PATH #6853

Merged
merged 2 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. (Since this isn't in packages/ it doesn't get the metadata checks.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does script/tool still need to be versioned like this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I publish it periodically so that flutter/packages can use the latest updates, so it does need versioning. Once the repo merge is done it won't.

## 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 '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I thought I'd fixed this everywhere, but apparently only in CI scripts.

'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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the trim be before this check?

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