From 6b543367a58ef7c371b43b36be93aaffe02a4531 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 12 Dec 2024 15:51:21 -0800 Subject: [PATCH 1/4] Made compilation error colors come through et. --- .../engine_build_configs/lib/src/build_config_runner.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index f96de99426012..8de5e86020f70 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -628,7 +628,10 @@ final class BuildRunner extends Runner { final io.Process process = await processRunner.processManager.start( command, workingDirectory: engineSrcDir.path, - environment: rbeConfig.environment, + environment: { + ...rbeConfig.environment, + 'CLICOLOR_FORCE': '1', + } ); final List stderrOutput = []; final List stdoutOutput = []; From 49b561c9684638cbd09d30c41e284a2d9c4e854c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 13 Dec 2024 10:01:25 -0800 Subject: [PATCH 2/4] added test and ansi escape support --- .../lib/src/build_config_runner.dart | 12 ++++++-- .../test/build_config_runner_test.dart | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index 8de5e86020f70..61ad4fd0cdf8e 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -565,6 +565,7 @@ final class BuildRunner extends Runner { static final RegExp _gccRegex = RegExp(r'^(.+)(:\d+:\d+:\s+(?:error|note|warning):\s+.*)$'); + static final RegExp _ansiRegex = RegExp(r'\x1b\[[\d;]*m'); /// Converts relative [path], who is relative to [dirPath] to a relative path /// of the `CWD`. @@ -573,15 +574,22 @@ final class BuildRunner extends Runner { return './${p.relative(abs)}'; } + static String _stripAnsi(String input) { + return input.replaceAll(_ansiRegex, ''); + } + /// Takes a [line] from compilation and makes the path relative to `CWD` where /// the paths are relative to [outDir]. @visibleForTesting static String fixGccPaths(String line, String outDir) { - final Match? match = _gccRegex.firstMatch(line); + final sansAnsi = _stripAnsi(line); + final Match? match = _gccRegex.firstMatch(sansAnsi); if (match == null) { return line; } else { - return '${_makeRelative(match.group(1)!, outDir)}${match.group(2)}'; + final String path = match.group(1)!; + final String fixedPath = _makeRelative(match.group(1)!, outDir); + return line.replaceFirst(path, fixedPath); } } diff --git a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart index a090a85fd254e..ef43da8f7dc2d 100644 --- a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart +++ b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart @@ -539,6 +539,35 @@ void main() { expect(fixed, './$error'); }); + test('fixes gcc paths with ansi colors', () { + final String outDir = path.join(io.Directory.current.path, 'foo', 'bar'); + // An error string with ANSI escape codes for colors. + final List bytes = [ + 27, 91, 49, 109, 46, 46, 47, 46, 46, 47, 102, // + 108, 117, 116, 116, 101, 114, 47, 105, 109, 112, 101, 108, 108, 101, // + 114, 47, 100, 105, 115, 112, 108, 97, 121, 95, 108, 105, 115, 116, 47, // + 100, 108, 95, 100, 105, 115, 112, 97, 116, 99, 104, 101, 114, 46, 99, // + 99, 58, 55, 51, 52, 58, 55, 58, 32, 27, 91, 48, 109, 27, 91, 48, 59, // + 49, 59, 51, 49, 109, 101, 114, 114, 111, 114, 58, 32, 27, 91, 48, 109, // + 27, 91, 49, 109, 117, 115, 101, 32, 111, 102, 32, 117, 110, 100, 101, // + 99, 108, 97, 114, 101, 100, 32, 105, 100, 101, 110, 116, 105, 102, 105, // + 101, 114, 32, 39, 114, 111, 99, 107, 101, 116, 39, 27, 91, 48, 109, + ]; + final String error = convert.utf8.decode(bytes); + final String fixed = BuildRunner.fixGccPaths(error, outDir); + expect( + fixed.contains('../../flutter/impeller/display_list/dl_dispatcher.cc'), + isFalse, + reason: 'Fixed string: $fixed', + ); + expect( + fixed.contains('./flutter/impeller/display_list/dl_dispatcher.cc'), + isTrue, + reason: 'Fixed string: $fixed', + ); + expect(fixed[0], '\x1B', reason: 'Fixed string: $fixed'); + }); + test('GlobalBuildRunner skips generators when runGenerators is false', () async { final Build targetBuild = buildConfig.builds[0]; From 90fe1e3401cb3250a326a7deb3ab18c7cb8ee7da Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 13 Dec 2024 10:40:01 -0800 Subject: [PATCH 3/4] only output ansi if we support it --- .../engine_build_configs/lib/src/build_config_runner.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index 61ad4fd0cdf8e..9988890a6ec73 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -5,7 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:ffi' as ffi; -import 'dart:io' as io show Directory, File, Process, ProcessResult; +import 'dart:io' as io show Directory, File, Platform, Process, ProcessResult, stdout; import 'dart:math'; import 'package:meta/meta.dart' show visibleForTesting; @@ -633,12 +633,14 @@ final class BuildRunner extends Runner { if (dryRun) { processResult = _dryRunResult; } else { + final bool shouldEmitAnsi = + io.stdout.supportsAnsiEscapes || io.Platform.environment['CLICOLOR_FORCE'] == '1'; final io.Process process = await processRunner.processManager.start( command, workingDirectory: engineSrcDir.path, environment: { ...rbeConfig.environment, - 'CLICOLOR_FORCE': '1', + if (shouldEmitAnsi) 'CLICOLOR_FORCE': '1', } ); final List stderrOutput = []; From 0e3e4836d91af3548ff4d246afb71c5e82d7d4d2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 13 Dec 2024 13:17:22 -0800 Subject: [PATCH 4/4] made it so you can turn off colors too --- .../pkg/engine_build_configs/lib/src/build_config_runner.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index 9988890a6ec73..a4fed2b435949 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -634,7 +634,8 @@ final class BuildRunner extends Runner { processResult = _dryRunResult; } else { final bool shouldEmitAnsi = - io.stdout.supportsAnsiEscapes || io.Platform.environment['CLICOLOR_FORCE'] == '1'; + (io.stdout.supportsAnsiEscapes && io.Platform.environment['CLICOLOR_FORCE'] != '0') || + io.Platform.environment['CLICOLOR_FORCE'] == '1'; final io.Process process = await processRunner.processManager.start( command, workingDirectory: engineSrcDir.path,