Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: Default color diagnostics to force-on with ccache/ninja #417

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

mattkretz
Copy link
Collaborator

Reading diagnostics with ANSI color codes in between is unnecessarily hard. We now make use of the cmake documented switch to control color output. And we only force color on if the tooling suggest it might be necessary.

Copy link
Member

@wirew0rm wirew0rm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff looks good, in general I'm all for using the the facilities that cmake is providing.

Unfortunately it does not seem to detect the CI correctly and switches off color there.
E.g. compare main and this PR.

It should imo either default more aggressively to colored output or the -DCMAKE_COLOR_DIAGNOSTICS=ON flag would at least have to be added to the CI.

CMakeLists.txt Show resolved Hide resolved
Reading diagnostics with ANSI color codes in between is unnecessarily
hard. We now make use of the cmake documented switch to control color
output. And we only force color on if the tooling suggest it might be
necessary.

Since GitHub Actions run without a terminal, auto-detection would always
determine to not use colored output. However, color works fine on GitHub
and therefore we force it on in CI builds.

Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
Copy link
Member

@wirew0rm wirew0rm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for also changing the CI to keep the output colored there.

I also tested in my IDE (clion, with CCACHE and with -DUSE_CCACHE=OFF and gnumake to not run in the forced mode also in both building via the Build->BuildProjct menu and the builtin terminal) and it correctly used colored output everywhere.

I don't see negative side effects of this and form my PoV this could be merged.

@wirew0rm wirew0rm merged commit a0e01a8 into main Sep 30, 2024
9 of 11 checks passed
@wirew0rm wirew0rm deleted the cmake_fix_compiler_color_usage branch September 30, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants