From d567ad2923dd431eba8e57c50668b6e0da736f38 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 30 Oct 2023 14:04:04 -0700 Subject: [PATCH 1/9] Made clang tidy use arm64 if on an arm64 mac. --- ci/clang_tidy.sh | 10 +++++++++- shell/common/shell.cc | 1 + tools/clang_tidy/lib/src/command.dart | 3 ++- tools/clang_tidy/lib/src/options.dart | 13 +++++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/ci/clang_tidy.sh b/ci/clang_tidy.sh index 80cb7030fbd55..444b37a20839e 100755 --- a/ci/clang_tidy.sh +++ b/ci/clang_tidy.sh @@ -46,7 +46,14 @@ else fix_flag="--fix --lint-all" fi -COMPILE_COMMANDS="$SRC_DIR/out/host_debug/compile_commands.json" +# 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" +else + CLANG_TIDY_PATH="buildtools/mac-x64/clang/bin/clang-tidy" +fi + +COMPILE_COMMANDS="$SRC_DIR/out/$BUILD_DIR/compile_commands.json" if [ ! -f "$COMPILE_COMMANDS" ]; then (cd "$SRC_DIR"; ./flutter/tools/gn) fi @@ -58,6 +65,7 @@ cd "$SCRIPT_DIR" --disable-dart-dev \ "$SRC_DIR/flutter/tools/clang_tidy/bin/main.dart" \ --src-dir="$SRC_DIR" \ + --clang-tidy="$SRC_DIR/$CLANG_TIDY_PATH" \ $fix_flag \ "$@" && true # errors ignored clang_tidy_return=$? diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 88948eb9d7d62..c89e129cc7f68 100644 --- a/shell/common/shell.cc +++ b/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/tools/clang_tidy/lib/src/command.dart b/tools/clang_tidy/lib/src/command.dart index 41dbea5914592..38a8c9a455a7c 100644 --- a/tools/clang_tidy/lib/src/command.dart +++ b/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/tools/clang_tidy/lib/src/options.dart b/tools/clang_tidy/lib/src/options.dart index f9d5320fb18a4..427ed907a925e 100644 --- a/tools/clang_tidy/lib/src/options.dart +++ b/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,8 @@ class Options { final StringSink _errSink; + final io.File? clangTidyPath; + /// Print command usage with an additional message. void printUsage({String? message, required Engine? engine}) { if (message != null) { From b67f6c62648405c14fb5604b90f86e41fe14e5cd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 30 Oct 2023 16:08:36 -0700 Subject: [PATCH 2/9] added tests --- tools/clang_tidy/test/clang_tidy_test.dart | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index da3bbf921d171..d7cecb543181d 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/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, 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(); From a356a2505a6b5a336c261df106b05dae1c4ffdd7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 30 Oct 2023 16:27:24 -0700 Subject: [PATCH 3/9] fixed linux --- ci/clang_tidy.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ci/clang_tidy.sh b/ci/clang_tidy.sh index 444b37a20839e..e66accb825688 100755 --- a/ci/clang_tidy.sh +++ b/ci/clang_tidy.sh @@ -49,8 +49,6 @@ 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" -else - CLANG_TIDY_PATH="buildtools/mac-x64/clang/bin/clang-tidy" fi COMPILE_COMMANDS="$SRC_DIR/out/$BUILD_DIR/compile_commands.json" @@ -65,7 +63,7 @@ cd "$SCRIPT_DIR" --disable-dart-dev \ "$SRC_DIR/flutter/tools/clang_tidy/bin/main.dart" \ --src-dir="$SRC_DIR" \ - --clang-tidy="$SRC_DIR/$CLANG_TIDY_PATH" \ + ${CLANG_TIDY_PATH:+--clang-tidy="$SRC_DIR/$CLANG_TIDY_PATH"} \ $fix_flag \ "$@" && true # errors ignored clang_tidy_return=$? From e7d1d4b6029b0cbbee82a49b84ba9f47d11baa60 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Tue, 31 Oct 2023 09:38:13 -0700 Subject: [PATCH 4/9] Update tools/clang_tidy/test/clang_tidy_test.dart Co-authored-by: Zachary Anderson --- tools/clang_tidy/test/clang_tidy_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index d7cecb543181d..ba4af5eaf7a5d 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -220,7 +220,7 @@ Future main(List args) async { '--clang-tidy="foo/bar"', ],); expect(options.clangTidyPath, isNotNull); - expect(options.clangTidyPath, equals('foo/bar')); + expect(options.clangTidyPath.path, equals('foo/bar')); }); }); From 6e55ce1a882235303696647d21bb677600677b5e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 31 Oct 2023 09:57:55 -0700 Subject: [PATCH 5/9] null check --- tools/clang_tidy/test/clang_tidy_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index ba4af5eaf7a5d..8649d201b9bd5 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -220,7 +220,7 @@ Future main(List args) async { '--clang-tidy="foo/bar"', ],); expect(options.clangTidyPath, isNotNull); - expect(options.clangTidyPath.path, equals('foo/bar')); + expect(options.clangTidyPath!.path, equals('foo/bar')); }); }); From 2208d34b2fe86a7cf6da21113740e9e5e41b36bf Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 31 Oct 2023 10:21:52 -0700 Subject: [PATCH 6/9] sorry, wackamole to avoid relearning how to run these tests since they are nonstandard, we should update the readme. --- tools/clang_tidy/test/clang_tidy_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index 8649d201b9bd5..370c989bfa8a7 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -217,7 +217,7 @@ 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"', + '--clang-tidy=foo/bar', ],); expect(options.clangTidyPath, isNotNull); expect(options.clangTidyPath!.path, equals('foo/bar')); From abd5e2998c2494644aba7721ea1f34c59ca71d2c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 31 Oct 2023 10:40:34 -0700 Subject: [PATCH 7/9] started executing gn indiscriminately --- ci/clang_tidy.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ci/clang_tidy.sh b/ci/clang_tidy.sh index e66accb825688..cc1bcceda5cba 100755 --- a/ci/clang_tidy.sh +++ b/ci/clang_tidy.sh @@ -51,10 +51,7 @@ 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/$BUILD_DIR/compile_commands.json" -if [ ! -f "$COMPILE_COMMANDS" ]; then - (cd "$SRC_DIR"; ./flutter/tools/gn) -fi +(cd "$SRC_DIR"; ./flutter/tools/gn) echo "$(date +%T) Running clang_tidy" From 8e94cbd751e1a50fc7da4a08b3b2f45ef6ff9a8a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 31 Oct 2023 10:57:07 -0700 Subject: [PATCH 8/9] brought back the if check --- ci/clang_tidy.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/clang_tidy.sh b/ci/clang_tidy.sh index cc1bcceda5cba..653256140cfa6 100755 --- a/ci/clang_tidy.sh +++ b/ci/clang_tidy.sh @@ -51,7 +51,10 @@ if command -v arch &> /dev/null && [[ $(arch) == "arm64" ]]; then CLANG_TIDY_PATH="buildtools/mac-arm64/clang/bin/clang-tidy" fi -(cd "$SRC_DIR"; ./flutter/tools/gn) +COMPILE_COMMANDS="$SRC_DIR/out/host_debug/compile_commands.json" +if [ ! -f "$COMPILE_COMMANDS" ]; then + (cd "$SRC_DIR"; ./flutter/tools/gn) +fi echo "$(date +%T) Running clang_tidy" From 170d2440ed22e1d1f8f3ed37b148f31e4687f7c1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 31 Oct 2023 12:34:50 -0700 Subject: [PATCH 9/9] added doc --- tools/clang_tidy/lib/src/options.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/clang_tidy/lib/src/options.dart b/tools/clang_tidy/lib/src/options.dart index 427ed907a925e..160210f1ef5cc 100644 --- a/tools/clang_tidy/lib/src/options.dart +++ b/tools/clang_tidy/lib/src/options.dart @@ -287,6 +287,8 @@ 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.