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

Fix the cmake's output for vcpkg_get_dep_info() #290

Merged
merged 4 commits into from
Dec 3, 2021

Conversation

tlemo
Copy link
Contributor

@tlemo tlemo commented Dec 1, 2021

Set CLICOLOR=0 to force cmake to output "plain" text (without ANSI escape codes)
(see Terminal.c)

On Linux (and likely on other non-Windows platforms), CMake may "colorize" the output depending on the terminal type. This creates problems when vcpkg attempts to parse the output from CMake (ex)

This PR would fix microsoft/vcpkg#20430

Set CLICOLOR=0 to force cmake to output "plain" text (without ANSI escape codes)
@dg0yt
Copy link
Contributor

dg0yt commented Dec 1, 2021

It looks inconsistent to unset one variable and to set the other to '0'.

I still think that the right fix is to not use console output for the cmake output which is meant to be parsed, but write a regular file instead. This would allow cmake to use color as desired by the user for other output.

@BillyONeal
Copy link
Member

Semi-duplicate of #261 but there dg0yt tried to write a test to repro the problem and could not

@dg0yt
Copy link
Contributor

dg0yt commented Dec 1, 2021

I guess the test for CLICOLOR didn't work because vcpkg CI is not really a terminal. CLICOLOR_FORCE makes cmake ignore the terminal property. So the problem is valid, and this PR is a possible solution.

@tlemo
Copy link
Contributor Author

tlemo commented Dec 1, 2021

Semi-duplicate of #261 but there dg0yt tried to write a test to repro the problem and could not

Constructing a repro should be straightforward. The following conditions can trigger the issue:

  1. A project using the vcpkg manifest
  2. Non-Windows platform + a color supporting terminal
  3. Use make as generator (by default, cmake would attempt to "colorize" the output)
  4. After a cmake configure + build, modify a cmake script than run make

The last step will trigger cmake configure, which would invoke the vcpkg installation, which should hit the issue.

It looks inconsistent to unset one variable and to set the other to '0'.

I agree, it does seem a bit inconsistent, although the source of inconsistency is external - that's the logic that CMake implements:CLICOLOR_FORCE=1 can force colorization on (but not off), and CLICOLOR=0 can force it off (but not on). There may be some history behind this convoluted logic (reference)

Arguably, there are some better fixes: redirect cmake output to a file instead of a pipe (like @dg0yt suggested) or set the CLICOLOR=0 just for the child cmake processes (not vckpg itself). The latter seems to require additional plumbing to add support for exec with env variables on all platforms, which doesn't seem to be there.

Ultimately, the fix proposed here is simple and safe and would eliminate a jarring behavior when vcpkg manifests are used on Linux. It doesn't prevent a more elaborate fix in the future.

tlemo added 2 commits December 1, 2021 15:35
Should explain the logic behind CLICOLOR and CLICOLOR_FORCE
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

LGTM

@ras0219-msft ras0219-msft merged commit 678019b into microsoft:main Dec 3, 2021
@ras0219-msft
Copy link
Contributor

LGTM, thanks!

@tlemo tlemo deleted the fix_launch_and_split branch December 3, 2021 00:44
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.

vcpkg install fails when CLICOLOR_FORCE=1
4 participants