| Message ID | 20180612224659.spp22g33ym4pib5r@angband.pl |
|---|---|
| State | New |
| Headers | show |
| Series | Let env var CLICOLOR_FORCE set -fdiagnostics-color=always. | expand |
On 06/12/2018 04:46 PM, Adam Borowski wrote: > Hi! > Here's a patch for Bugzilla 80271, to add support for env CLICOLOR_FORCE. > > Automated build environments -- and even some which you run by hand -- > redirect the output somewhere to save a log, even if they tee it to the > terminal anyway. This kills all colored highlights for diagnostics. > Those highlights are really nice for a human -- unlike the old days when > a single-threaded build always had the failure at the end, modern setups > tend to spew pages of stuff after the real reason your build failed. As > human vision evolved to be extremely good at noticing a fleck of color, > it's easy to spot that bit of red even when scrolling very fast. > > Besides terminals, you can read colored logs via "less -R", ansi2html, etc. > > For such piped setups, gcc has -fdiagnostics-color=always. Alas, while this > flag is easy to set by someone writing a given piece of software, there's > no reasonable way to set it globally in the build environment. > > Thus, in #80271 Jan Niklas Hasse requested, and I concur, to be able to > set the same via an env var. This flag doesn't affect code produced by gcc > in any way, thus it's reasonable to set it via env. There's already > GCC_COLORS letting you customize colors used, you just can't tell it to > ignore stderr not being a terminal. > > > Please forgive my lack of knowledge wrt proper ways of submitting patches to > gcc (ie, when biting please point to piece of TFM I failed to R). In > particular, I'm not sure how to run the testsuite: it produces a lot of > fails even without my patch, and I don't know what is your way to spot new > failures. The testsuite could be interesting as gcc in a lot of places sets > and unsets CLICOLOR_FORCE despite not supporting it itself (yet a number of > other tools do). Weirdly, in every case you unset it in the very next line > after enabling it. > > I've submitted an equivalent of this patch to llvm/clang as well. We _strongly_ discourage the use of environment variables to control the compiler. Really the way to go here is to use the -fdiagnostics-color=always and deal with the difficulties of flag injection. Jeff
On Fri, Jun 22, 2018 at 02:08:27PM -0600, Jeff Law wrote: > On 06/12/2018 04:46 PM, Adam Borowski wrote: > > Here's a patch for Bugzilla 80271, to add support for env CLICOLOR_FORCE. > > > > Automated build environments -- and even some which you run by hand -- > > redirect the output somewhere to save a log, even if they tee it to the > > terminal anyway. This kills all colored highlights for diagnostics. > > Those highlights are really nice for a human -- unlike the old days when > > a single-threaded build always had the failure at the end, modern setups > > tend to spew pages of stuff after the real reason your build failed. As > > human vision evolved to be extremely good at noticing a fleck of color, > > it's easy to spot that bit of red even when scrolling very fast. > > > > Besides terminals, you can read colored logs via "less -R", ansi2html, etc. > > > > For such piped setups, gcc has -fdiagnostics-color=always. Alas, while this > > flag is easy to set by someone writing a given piece of software, there's > > no reasonable way to set it globally in the build environment. > > > > Thus, in #80271 Jan Niklas Hasse requested, and I concur, to be able to > > set the same via an env var. This flag doesn't affect code produced by gcc > > in any way, thus it's reasonable to set it via env. There's already > > GCC_COLORS letting you customize colors used, you just can't tell it to > > ignore stderr not being a terminal. > We _strongly_ discourage the use of environment variables to control the > compiler. Really the way to go here is to use the > -fdiagnostics-color=always and deal with the difficulties of flag injection. As discussed in #80271, color vs b&w diagnostics don't change any produced files in any way. The presentation of these diagnostics is already affected by things not included in -f args (namely, isatty(2)). And, this very feature is already controlled by another env variable, ie, GCC_COLORS. Another reason is that, certain build systems store -f flags and consider a build to be different if the flags differ. This can cause spurious recompiles just because one invocation was redirected but another was not, something likely to happen in a debugging session. Meow!
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index e22c17bc02c..d9e352932c6 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -190,12 +190,18 @@ diagnostic_color_init (diagnostic_context *context, int value /*= -1 */) /* value == -1 is the default value. */ if (value < 0) { + /* Explicit command-line args override env, but env prevails over + defaults, whatever they are. + The spec defines an existing but empty var to mean "yes". */ + const char* clic = getenv ("CLICOLOR_FORCE"); + if (clic && strcmp (clic, "0")) + value = DIAGNOSTICS_COLOR_YES; /* If DIAGNOSTICS_COLOR_DEFAULT is -1, default to -fdiagnostics-color=auto if GCC_COLORS is in the environment, otherwise default to -fdiagnostics-color=never, for other values default to that -fdiagnostics-color={never,auto,always}. */ - if (DIAGNOSTICS_COLOR_DEFAULT == -1) + else if (DIAGNOSTICS_COLOR_DEFAULT == -1) { if (!getenv ("GCC_COLORS")) return;
Hi! Here's a patch for Bugzilla 80271, to add support for env CLICOLOR_FORCE. Automated build environments -- and even some which you run by hand -- redirect the output somewhere to save a log, even if they tee it to the terminal anyway. This kills all colored highlights for diagnostics. Those highlights are really nice for a human -- unlike the old days when a single-threaded build always had the failure at the end, modern setups tend to spew pages of stuff after the real reason your build failed. As human vision evolved to be extremely good at noticing a fleck of color, it's easy to spot that bit of red even when scrolling very fast. Besides terminals, you can read colored logs via "less -R", ansi2html, etc. For such piped setups, gcc has -fdiagnostics-color=always. Alas, while this flag is easy to set by someone writing a given piece of software, there's no reasonable way to set it globally in the build environment. Thus, in #80271 Jan Niklas Hasse requested, and I concur, to be able to set the same via an env var. This flag doesn't affect code produced by gcc in any way, thus it's reasonable to set it via env. There's already GCC_COLORS letting you customize colors used, you just can't tell it to ignore stderr not being a terminal. Please forgive my lack of knowledge wrt proper ways of submitting patches to gcc (ie, when biting please point to piece of TFM I failed to R). In particular, I'm not sure how to run the testsuite: it produces a lot of fails even without my patch, and I don't know what is your way to spot new failures. The testsuite could be interesting as gcc in a lot of places sets and unsets CLICOLOR_FORCE despite not supporting it itself (yet a number of other tools do). Weirdly, in every case you unset it in the very next line after enabling it. I've submitted an equivalent of this patch to llvm/clang as well. -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- >8 -- From: Adam Borowski <kilobyte@angband.pl> Date: Mon, 11 Jun 2018 01:01:46 +0200 Subject: [PATCH] Let env var CLICOLOR_FORCE set -fdiagnostics-color=always. --- gcc/diagnostic.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)