From ef612343d90a79a49b9116e4cf7afb07eaa08ab4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 20 Nov 2024 08:50:55 +0100 Subject: [PATCH] scripts: make clang tools obey b_colorout Right now, the clang-tidy and clang-format targets use the program default and do not let b_colorout decide whether to colorize output. However, the wrappers that run the tool are going to be changed to buffer output, and that would disable colorization unconditionally. So pass a --color option to the tools and use it when building the command line. clang-format's -fcolor-diagnostics option simply does not work, and the "right" (or at least working) option is --color which is undocumented. --color is present all the way back to clang 10, but I digged into clang-format's source code to figure out what's happening. The problem is that -fcolor-diagnostics is a complete no-operation; in fact it is a bool that is initialized to true. gdb shows: (gdb) p ShowColors $2 = { = { ... > = {Value = true, ... }, ...} on entry to clang-format's main, meaning that specifying the option on the command line does nothing at all. To see how clang-format determines whether to use colors you need to look at enters SMDiagnostic::print, which simply does ColorMode Mode = ShowColors ? ColorMode::Auto : ColorMode::Disable; showing once more that in fact the option cannot force-on the colors ( -fno-color-diagnostics instead works). Continuing in SMDiagnostic::print, this RAII constructor would write the escape sequence to the terminal: WithColor S(OS, raw_ostream::SAVEDCOLOR, true, false, Mode); It ends up in WithColor::changeColor, which does if (colorsEnabled()) OS.changeColor(Color, Bold, BG); Digging further down, colorsEnabled() is where the Mode member is consulted: bool WithColor::colorsEnabled() { switch (Mode) { case ColorMode::Enable: return true; case ColorMode::Disable: return false; case ColorMode::Auto: return AutoDetectFunction(OS); } llvm_unreachable("All cases handled above."); } and the "AutoDetectFunction" is static bool DefaultAutoDetectFunction(const raw_ostream &OS) { return *UseColor == cl::BOU_UNSET ? OS.has_colors() : *UseColor == cl::BOU_TRUE; } UseColor is controlled by the "--color" option, so if that option was unset you go to OS.has_colors() even in the presence of -fcolor-diagnostics. This has been around for over 5 years in clang-format, and it was present even earlier, so use it in meson as well. Signed-off-by: Paolo Bonzini --- mesonbuild/backend/ninjabackend.py | 3 +++ mesonbuild/scripts/clangformat.py | 13 +++++++++---- mesonbuild/scripts/clangtidy.py | 4 ++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py index 64921ffa65a8..81b8bb51ab81 100644 --- a/mesonbuild/backend/ninjabackend.py +++ b/mesonbuild/backend/ninjabackend.py @@ -3659,6 +3659,9 @@ def generate_clangtool(self, name: str, extra_arg: T.Optional[str] = None) -> No if extra_arg: target_name += f'-{extra_arg}' extra_args.append(f'--{extra_arg}') + colorout = self.environment.coredata.optstore.get_value('b_colorout') \ + if OptionKey('b_colorout') in self.environment.coredata.optstore else 'always' + extra_args.extend(['--color', colorout]) if not os.path.exists(os.path.join(self.environment.source_dir, '.clang-' + name)) and \ not os.path.exists(os.path.join(self.environment.source_dir, '_clang-' + name)): return diff --git a/mesonbuild/scripts/clangformat.py b/mesonbuild/scripts/clangformat.py index 9ce050458986..88cc89071d7c 100644 --- a/mesonbuild/scripts/clangformat.py +++ b/mesonbuild/scripts/clangformat.py @@ -6,6 +6,7 @@ import argparse import subprocess from pathlib import Path +import sys from .run_tool import run_tool from ..environment import detect_clangformat @@ -13,12 +14,15 @@ from ..programs import ExternalProgram import typing as T -def run_clang_format(fname: Path, exelist: T.List[str], check: bool, cformat_ver: T.Optional[str]) -> subprocess.CompletedProcess: +def run_clang_format(fname: Path, exelist: T.List[str], options: argparse.Namespace, cformat_ver: T.Optional[str]) -> subprocess.CompletedProcess: clangformat_10 = False - if check and cformat_ver: + if options.check and cformat_ver: if version_compare(cformat_ver, '>=10'): clangformat_10 = True exelist = exelist + ['--dry-run', '--Werror'] + # The option is not documented but it exists in version 10 + if options.color == 'always' or options.color == 'auto' and sys.stdout.isatty(): + exelist += ['--color=1'] else: original = fname.read_bytes() before = fname.stat().st_mtime @@ -26,7 +30,7 @@ def run_clang_format(fname: Path, exelist: T.List[str], check: bool, cformat_ver after = fname.stat().st_mtime if before != after: print('File reformatted: ', fname) - if check and not clangformat_10: + if options.check and not clangformat_10: # Restore the original if only checking. fname.write_bytes(original) ret.returncode = 1 @@ -35,6 +39,7 @@ def run_clang_format(fname: Path, exelist: T.List[str], check: bool, cformat_ver def run(args: T.List[str]) -> int: parser = argparse.ArgumentParser() parser.add_argument('--check', action='store_true') + parser.add_argument('--color', default='always') parser.add_argument('sourcedir') parser.add_argument('builddir') options = parser.parse_args(args) @@ -52,4 +57,4 @@ def run(args: T.List[str]) -> int: else: cformat_ver = None - return run_tool('clang-format', srcdir, builddir, run_clang_format, exelist, options.check, cformat_ver) + return run_tool('clang-format', srcdir, builddir, run_clang_format, exelist, options, cformat_ver) diff --git a/mesonbuild/scripts/clangtidy.py b/mesonbuild/scripts/clangtidy.py index a922f8514062..fe34801e0e74 100644 --- a/mesonbuild/scripts/clangtidy.py +++ b/mesonbuild/scripts/clangtidy.py @@ -26,6 +26,7 @@ def run_clang_tidy(fname: Path, tidyexe: list, builddir: Path, fixesdir: T.Optio def run(args: T.List[str]) -> int: parser = argparse.ArgumentParser() parser.add_argument('--fix', action='store_true') + parser.add_argument('--color', default='always') parser.add_argument('sourcedir') parser.add_argument('builddir') options = parser.parse_args(args) @@ -38,6 +39,9 @@ def run(args: T.List[str]) -> int: print(f'Could not execute clang-tidy "{" ".join(tidyexe)}"') return 1 + if options.color == 'always' or options.color == 'auto' and sys.stdout.isatty(): + tidyexe += ['--use-color'] + fixesdir: T.Optional[Path] = None if options.fix: applyexe = detect_clangapply()