From 52fcefb32a7e07b00a72c4c2904806e73a709912 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:35:36 -0700 Subject: [PATCH] Made clang tidy use arm64 if on an arm64 mac. (flutter/engine#47494) fixes https://github.com/flutter/flutter/issues/137260 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --------- Co-authored-by: Zachary Anderson --- engine/src/flutter/ci/clang_tidy.sh | 6 ++++++ engine/src/flutter/shell/common/shell.cc | 1 + .../tools/clang_tidy/lib/src/command.dart | 3 ++- .../tools/clang_tidy/lib/src/options.dart | 15 +++++++++++++++ .../tools/clang_tidy/test/clang_tidy_test.dart | 17 +++++++++++++++++ 5 files changed, 41 insertions(+), 1 deletion(-) diff --git a/engine/src/flutter/ci/clang_tidy.sh b/engine/src/flutter/ci/clang_tidy.sh index 80cb7030fbd..653256140cf 100755 --- a/engine/src/flutter/ci/clang_tidy.sh +++ b/engine/src/flutter/ci/clang_tidy.sh @@ -46,6 +46,11 @@ else fix_flag="--fix --lint-all" fi +# Determine wether to use x64 or arm64. +if command -v arch &> /dev/null && [[ $(arch) == "arm64" ]]; then + CLANG_TIDY_PATH="buildtools/mac-arm64/clang/bin/clang-tidy" +fi + COMPILE_COMMANDS="$SRC_DIR/out/host_debug/compile_commands.json" if [ ! -f "$COMPILE_COMMANDS" ]; then (cd "$SRC_DIR"; ./flutter/tools/gn) @@ -58,6 +63,7 @@ cd "$SCRIPT_DIR" --disable-dart-dev \ "$SRC_DIR/flutter/tools/clang_tidy/bin/main.dart" \ --src-dir="$SRC_DIR" \ + ${CLANG_TIDY_PATH:+--clang-tidy="$SRC_DIR/$CLANG_TIDY_PATH"} \ $fix_flag \ "$@" && true # errors ignored clang_tidy_return=$? diff --git a/engine/src/flutter/shell/common/shell.cc b/engine/src/flutter/shell/common/shell.cc index 88948eb9d7d..c89e129cc7f 100644 --- a/engine/src/flutter/shell/common/shell.cc +++ b/engine/src/flutter/shell/common/shell.cc @@ -177,6 +177,7 @@ std::unique_ptr Shell::Create( auto resource_cache_limit_calculator = std::make_shared( settings.resource_cache_max_bytes_threshold); + return CreateWithSnapshot(platform_data, // task_runners, // /*parent_thread_merger=*/nullptr, // diff --git a/engine/src/flutter/tools/clang_tidy/lib/src/command.dart b/engine/src/flutter/tools/clang_tidy/lib/src/command.dart index 41dbea59145..38a8c9a455a 100644 --- a/engine/src/flutter/tools/clang_tidy/lib/src/command.dart +++ b/engine/src/flutter/tools/clang_tidy/lib/src/command.dart @@ -146,8 +146,9 @@ class Command { '--', ]; args.addAll(tidyArgs.split(' ')); + final String clangTidyPath = options.clangTidyPath?.path ?? tidyPath; return WorkerJob( - [tidyPath, ...args], + [clangTidyPath, ...args], workingDirectory: directory, name: 'clang-tidy on $filePath', printOutput: options.verbose, diff --git a/engine/src/flutter/tools/clang_tidy/lib/src/options.dart b/engine/src/flutter/tools/clang_tidy/lib/src/options.dart index f9d5320fb18..160210f1ef5 100644 --- a/engine/src/flutter/tools/clang_tidy/lib/src/options.dart +++ b/engine/src/flutter/tools/clang_tidy/lib/src/options.dart @@ -38,6 +38,7 @@ class Options { this.shardCommandsPaths = const [], this.enableCheckProfile = false, StringSink? errSink, + this.clangTidyPath, }) : checks = checksArg.isNotEmpty ? '--checks=$checksArg' : null, _errSink = errSink ?? io.stderr; @@ -69,6 +70,7 @@ class Options { StringSink? errSink, required List shardCommandsPaths, int? shardId, + io.File? clangTidyPath, }) { final LintTarget lintTarget; if (options.wasParsed('lint-all') || io.Platform.environment['FLUTTER_LINT_ALL'] != null) { @@ -92,6 +94,7 @@ class Options { shardCommandsPaths: shardCommandsPaths, shardId: shardId, enableCheckProfile: options['enable-check-profile'] as bool, + clangTidyPath: clangTidyPath, ); } @@ -138,12 +141,16 @@ class Options { if (shardId != null && (shardId > shardCommands.length || shardId < 0)) { return Options._error('Invalid shard-id value: $shardId.', errSink: errSink); } + final io.File? clangTidyPath = ((String? path) => path == null + ? null + : io.File(path))(argResults['clang-tidy'] as String?); return Options._fromArgResults( argResults, buildCommandsPath: buildCommands, errSink: errSink, shardCommandsPaths: shardCommands, shardId: shardId, + clangTidyPath: clangTidyPath, ); } @@ -227,6 +234,10 @@ class Options { 'string, indicating all checks should be performed.', defaultsTo: '', ) + ..addOption('clang-tidy', + help: + 'Path to the clang-tidy executable. Defaults to deriving the path\n' + 'from compile_commands.json.') ..addFlag( 'enable-check-profile', help: 'Enable per-check timing profiles and print a report to stderr.', @@ -276,6 +287,10 @@ class Options { final StringSink _errSink; + /// Override for which clang-tidy to use. If it is null it will be derived + /// instead. + final io.File? clangTidyPath; + /// Print command usage with an additional message. void printUsage({String? message, required Engine? engine}) { if (message != null) { diff --git a/engine/src/flutter/tools/clang_tidy/test/clang_tidy_test.dart b/engine/src/flutter/tools/clang_tidy/test/clang_tidy_test.dart index da3bbf921d1..370c989bfa8 100644 --- a/engine/src/flutter/tools/clang_tidy/test/clang_tidy_test.dart +++ b/engine/src/flutter/tools/clang_tidy/test/clang_tidy_test.dart @@ -214,6 +214,23 @@ Future main(List args) async { }); }); + test('clang-tidy specified', () async { + _withTempFile('shard-id-valid', (String path) { + final Options options = Options.fromCommandLine( [ + '--clang-tidy=foo/bar', + ],); + expect(options.clangTidyPath, isNotNull); + expect(options.clangTidyPath!.path, equals('foo/bar')); + }); + }); + + test('clang-tidy unspecified', () async { + _withTempFile('shard-id-valid', (String path) { + final Options options = Options.fromCommandLine( [],); + expect(options.clangTidyPath, isNull); + }); + }); + test('shard-id invalid', () async { _withTempFile('shard-id-valid', (String path) { final StringBuffer errBuffer = StringBuffer();