Skip to content

Commit

Permalink
Made compilation error colors come through et. (#57174)
Browse files Browse the repository at this point in the history
fixes flutter/flutter#147931

environment variable documented in github issue:
ninja-build/ninja#2196

## screenshot of results
<img width="712" alt="Screenshot 2024-12-13 at 10 18 15 AM"
src="https://github.com/user-attachments/assets/571da2d8-065d-4b94-8ca2-a5bef5150dc7"
/>

## 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].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/engine/blob/main/docs/testing/Testing-the-engine.md
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md
  • Loading branch information
gaaclarke authored Dec 13, 2024
1 parent 2ddbf28 commit 0c8e1de
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
22 changes: 18 additions & 4 deletions tools/pkg/engine_build_configs/lib/src/build_config_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`.
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -625,10 +633,16 @@ final class BuildRunner extends Runner {
if (dryRun) {
processResult = _dryRunResult;
} else {
final bool shouldEmitAnsi =
(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,
environment: rbeConfig.environment,
environment: {
...rbeConfig.environment,
if (shouldEmitAnsi) 'CLICOLOR_FORCE': '1',
}
);
final List<int> stderrOutput = <int>[];
final List<int> stdoutOutput = <int>[];
Expand Down
29 changes: 29 additions & 0 deletions tools/pkg/engine_build_configs/test/build_config_runner_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> 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];
Expand Down

0 comments on commit 0c8e1de

Please sign in to comment.