Closed memsharded closed 11 months ago
Hi @ytimenkov
If this is a regression, lets have a look and fix it. We merged this because we thought it had very low risk of breaking, all the changes seemed opt-in, and at the end of the day it is just changing the UX, it was unlikely that it would break a build.
We explicitly set CONAN_COLOR_DISPLAY=0 when running conan --version and now it no longer works.
Could you please clarify? I have tried:
$ conan --version
Conan version 1.30.0-dev # in green color
$ CONAN_COLOR_DISPLAY=0
$ conan --version
Conan version 1.30.0-dev # no color
$ conan --version > version.txt # file without escape sequence
Please let us know the steps to reproduce the regression and we will try to fix it asap.
Hint: if you are trying to automate in your CI or somewhere and force a specific version, you don't need to parse this output, you can use the required_conan_version
in conan.conf to enforce it.
@memsharded Thanks for taking my rant seriously 🙇
It's easy to reproduce:
env CLICOLOR_FORCE=1 CONAN_COLOR_DISPLAY=0 conan --version | cat
Before it was raw text, now it's ANSI-colored. Thing is those variables were set for different purposes: CLICOLOR_FORCE
was set to get color output from the build tool in general while CONAN_FORCE_COLOR=0
was added only when querying the version to get machine-readable text.
As you can see the problem is when there are multiple conflicting variables and the order in which Conan processes them. There were like 2 before and PR introduces 3 more with
Well, they didn't conflict before, and there is always risk when you use something non-namespaced (that's why somewhere I suggested introducing a completely new one, like CONAN_FORCE_ANSI_COLORS
which should behave exactly as CLICOLOR_FORCE=1
now).
Hint: if you are trying to automate in your CI or somewhere and force a specific version, you don't need to parse this output, you can use the required_conan_version in conan.conf to enforce it.
Yes, I automate this on CI, but I do version detection to use new features, so same script works with multiple versions, so just minimum doesn't solve all the problems.
Probably it would not break if new logic was applied after the original one (well, maybe except for NO_COLOR
), but I've fixed this on our side and I like new behavior more, so I suggest keeping things as they are now.
Can't close the issue...
Another thought is that documentation could be be improved. Something like adding a section "controlling output colorst" describing all variables and their interaction in one place. Also links in the commit message could be referred, they're useful.
This is the list of variables that influence Conan, in order: NO_COLOR CLICOLOR_FORCE CLICOLOR CONAN_COLOR_DISPLAY PYCHARM_HOSTED
NO_COLOR
is set to whatever value, colors are disabled.CLICOLOR_FORCE
is nonblank/nonzero, then colors are enabled regardless of whether stdout/err is a TTY, and also it disables any potential VT100=>Pre-MSWin10 conversion.CLICOLOR
is 0, then colors are disabled.CLICOLOR
is nonblank/nonzero and stdout is not a TTY, then colors are disabled.CLICOLOR
is nonblank/nonzero and stdout is a TTY, then colors are enabled.CONAN_COLOR_DISPLAY
is 0, then colors are disabled.CONAN_COLOR_DISPLAY
is nonblank/nonzero, then colors are enabled regardless of whether stdout/err is a TTY (and if PYCHARM_HOSTED
is set to whatever value, it disables any potential VT100=>Pre-MSWin10 conversion).PYCHARM_HOSTED
is set to whatever value, it disables any potential VT100=>Pre-MSWin10 conversion).Note that Conan doesn't color stdout and stderr separately and only tests whether stdout is a TTY, so, stderr will be colored iif stdout is colored.
Problems:
CLICOLOR_FORCE=1
or CLICOLOR=1
takes precedence over CONAN_COLOR_DISPLAY=0
.CLICOLOR_FORCE=1
breaks Pre-MSWin10 terminal.CLICOLOR=1
breaks JetBrain's PyCharm terminal.I think the rules should be changed so a veto wins (i.e. a No wins over a Yes).
First the Noes:
NO_COLOR
is nonblank, colors are disabled.CLICOLOR
is 0, then colors are disabled.CONAN_COLOR_DISPLAY
is 0, then colors are disabled.Then the Ayes:
CLICOLOR
is nonblank/nonzero and stdout is a TTY, then colors are enabled.CLICOLOR_FORCE
is nonblank/nonzero, then colors are enabled regardless of whether stdout/err is a TTY.CONAN_COLOR_DISPLAY
is nonblank/nonzero, then colors are enabled regardless of whether stdout/err is a TTY.Notice the following are blank votes, they have no effect whatsoever:
CLICOLOR=1
means "I'm not opposed to colors, no daltonism."CLICOLOR_FORCE=0
means "I'm not forcing anybody."With the rules above, NO_COLOR=1
, CLICOLOR=0
, or CONAN_COLOR_DISPLAY=0
would all take precedence over CLICOLOR_FORCE=1
.
When enabling colors, we should always test whether the environment asks for the VT100=>Pre-MSWin10 conversion to be disabled. Currently, the test is with PYCHARM_HOSTED
which JetBrain invented has a dirty kluge, rather than TERM
, why ? Doesn't PyCharm set TERM
? It seems to me colorama.init(strip=False)
should force colors without TTY, while still retaining minimal TERM
detection to decide when doing convert=False
. If not, that's a colorama bug IMHO. I can't see a situation where you have say TERM=xterm
and you'd want to use Win32 console API!
When launching Conan through winpty
on Windows within a mintty terminal, the background becomes pink. It's atrocious on the eyes.
So, when I don't need an interactive prompt, I don't run it through winpty
, but, then Conan can't detect any TTY, so it doesn't do color.
So, I force it to use colors with CONAN_COLOR_DISPLAY=1
, but then it does the VT100=>Pre-MSWin10 conversion which doesn't make sense on the mintty VT100 terminal.
So, to force it to NOT do the VT100=>Pre-MSWin10 conversion, I have to set PYCHARM_HOSTED
to whatever value.
Now comes a bug: you can't really disable color anymore with CONAN_COLOR_DISPLAY=0
or CLICOLOR=0
or NO_COLOR
.
$ PYCHARM_HOSTED=1 CONAN_COLOR_DISPLAY=0 conan config home | cat -A
C:\Users\jboule\.conan^M$
^[[0m
The VT100 escape sequence SGR reset ESC + [0m
should not be present. In fact, the same command on a tmux terminal in linux does not print it.
This badly breaks the parsing of the output for commands like conan --version
, conan config home
, conan config get
, etc.
The current workaround for mintty is to not set PYCHARM_HOSTED
and CONAN_COLOR_DISPLAY=1
, and instead set CLICOLOR_FORCE=1
, this will bypass the VT100=>Pre-MSWin10 conversion.
But this means to make parsing like home=$(conan config home)
work, only NO_COLOR=1
will correctly disable colors, and neither CLICOLOR=0
nor CONAN_COLOR_DISPLAY=0
will work anymore (due to bad precedence rules described above).
In the end, if you have a script that does home=$(conan config home)
,
for robustness, it has to be replaced with home=$(env -u PYCHARM_HOSTED -u CLICOLOR_FORCE NO_COLOR=1 conan config home)
.
This badly breaks the parsing of the output for commands like conan --version, conan config home, conan config get, etc.
conan config home
already prints to stdout without colors in Conan 2.0conan config get
has been removed in Conan 2.0conan version --format=json
(and getting the version
item)So there are 2 different issues in this thread: output that shouldn't be using colors at all, like conan --version
(lets change that), and then using different env-vars to activate/deactivate/force colors.
It is very important to highlight that in Conan 2.0, except very limited cases, all the terminal regular text output is not intended to be parsed, and it can change at any time. Most commands are providing now structured --format=json
output. So while we are also willing to improve the definition of activation of colors, it should be noted that some output printing strange color characters should never be a reason for breaking user CI or automated scripts (and if it is, like the exception of conan --version
, we will change it and remove colors).
Regarding the variables, this should be reviewed for Conan 2.0, that has greatly simplified the logic:
def color_enabled(stream):
"""
NO_COLOR: No colors
https://no-color.org/
Command-line software which adds ANSI color to its output by default should check for the
presence of a NO_COLOR environment variable that, when present (**regardless of its value**),
prevents the addition of ANSI color.
CLICOLOR_FORCE: Force color
https://bixense.com/clicolors/
"""
if os.getenv("CLICOLOR_FORCE", "0") != "0":
# CLICOLOR_FORCE != 0, ANSI colors should be enabled no matter what.
return True
if os.getenv("NO_COLOR") is not None:
return False
return is_terminal(stream)
Only 2 variables are used now:
CLICOLOR_FORCE
is defined to anything else than "0", colors will be activated and forced (highest priority)NO_COLOR
is defined, that colors will be disabledI think this should be simpler and easier to understand logic.
https://github.com/conan-io/conan/pull/15002 has been merged.
I think that Conan 2.0 CLICOLOR_FORCE
and NO_COLOR
should be enough to define the desired behavior, so this issue could be closed.
I will add these variables description to the docs and close this ticket as solved in 2.0. Thanks for the feedback!
The Conan 2.0 env-vars meaning and precedence is already documented in https://docs.conan.io/2/reference/environment.html#terminal-color-variables, closing the ticket
Funny thing: you were so afraid to break customers with my PRs so you finally broke us :( By introducing a lot of new variables with extremely complicated interacton and overriding...
As PR mentiones those variables are used by other tools used by Conan so we set
CLICOLOR_FORCE
to 1 and this breaks parsing conan output, its--version
as it becomes populated with ANSI-sequences.We explicitly set
CONAN_COLOR_DISPLAY=0
when runningconan --version
and now it no longer works.Originally posted by @ytimenkov in https://github.com/conan-io/conan/pull/7154#issuecomment-686961779