Re: [cmake-developers] [CMake] Forcing colorization of output from cmake
On 10/09/2014 01:38 PM, Paul Smith wrote: What I really want is that if MAKE_TERMOUT is set to a non-empty value, cmake should pretend that it's in a terminal regardless of what isatty() says. We can do this easily enough by adding a simple test to Terminal.c:kwsysTerminalStreamIsVT100() _without_ adding any code anywhere else: One problem with this is that non-make processes launched by make may run their own children to capture the output through pipes and not unset MAKE_TERMOUT. We need it to be 'make' that evaluates that env variable. Your original command-line approach did this. In a way this is gross, certainly, but this function already checks for environment variable such as TERM, EMACS, etc. which are set by calling utilities and handles them specially. So why not MAKE_TERMOUT as well? The TERM and EMACS variables are checked to determine whether the terminal supports color when isatty() returns true. That is not the same as forcing tty behavior. I would expect FORCE to be stronger than that, and return true regardless of TERM settings. Okay, but the internal API in KWSys Terminal needs to be more granular than that. Please revise your patch to use my KWSys part but also add a ForceVT100 setting that causes TERM and EMACS to be ignored. Then the --switch=FORCE can set both the ForceTTY and ForceVT100 setting internally. I'm not at all familiar with Windows so I have no strong opinions on whether FORCE should also force color output on a Windows console... although I know a number of people who do use GNU make on Windows and the Windows port of GNU make does set MAKE_TERMOUT properly. Native Windows terminals do not support the color escape sequences at all. We need to get a handle to the underlying console buffer and set parameters on it directly. If the output pipe is not connected to a real console then we cannot get the console handle and print color at all. We already have a CMAKE_COLOR_MAKEFILE option. Perhaps instead of just ON or OFF values it could have a GNU value that enables Well, it's easy enough to detect if CMAKE_MAKE_PROGRAM is GNU make, if we can run it to test the output of ${CMAKE_MAKE_PROGRAM} --version. The question is, what do you do in the generated makefile if you see that it's GNU make? The CMake generator would recognize CMAKE_COLOR_MAKEFILE==GNU and enable the GNU-specific content. CMAKE_COLOR_MAKEFILE is a cache setting the user could set if they don't like the default. We can set the default based on whether the CMAKE_MAKE_PROGRAM is GNU. It would need to be the equivalent of: --switch=$(or $(COLOR),$(if $(MAKE_TERMOUT),FORCE)) to preserve the current behavior, where the COLOR variable setting can be inherited from the environment. Yes. What if someone runs cmake and it detects GNU make, but then they run /my/other/make which is not GNU make? That would work fine now but fail if we generated GNU-specific content in 'Unix Makefiles' generators. The user could always set CMAKE_COLOR_MAKEFILE to ON instead of GNU to work with other make tools. If we had a different generator, like 'GNU Makefiles' for example, then people who chose that would clearly expect the results would only work with GNU make, but we don't have that. One day we may create a 'GNU Makefiles' generator to enable use of GNU-specific features, but that would need to be a more comprehensive effort than just changing the color option. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [CMake] Forcing colorization of output from cmake
On 10/08/2014 02:36 PM, Paul Smith wrote: Hi all. Attached please find a proposed patch for the above. I still think there's some slightly awkward redundancy between AssumeTTY and the new ForceTTY, but I'm not sure these can actually be combined into one flag... certainly not without reworking more of how AssumeTTY is interpreted and used than I felt confident with. In this hunk: @@ -68,14 +68,16 @@ void kwsysTerminal_cfprintf(int color, FILE* stream, const char* format, ...) #if defined(KWSYS_TERMINAL_SUPPORT_CONSOLE) CONSOLE_SCREEN_BUFFER_INFO hOutInfo; HANDLE hOut = kwsysTerminalGetStreamHandle(stream); - if(GetConsoleScreenBufferInfo(hOut, hOutInfo)) + if(color kwsysTerminal_Color_ForceTTY || + GetConsoleScreenBufferInfo(hOut, hOutInfo)) { pipeIsConsole = 1; kwsysTerminalSetConsoleColor(hOut, hOutInfo, stream, color); } we cannot enter the if() block unless GetConsoleScreenBufferInfo returns true because otherwise the hOutInfo will not be initialized correctly. In the Windows console code path we really need a handle to the console from the stream or it cannot work. Also, once the pipeIsConsole value is true, the kwsysTerminalSetVT100Color code path should not be used, so we do not want to force the second code path either. Instead ForceTTY should just influence the return value of kwsysTerminalStreamIsVT100. The Source/kwsys part of the change actually belongs in a separate KWSys upstream first. I've added a modified version of your change for upstream review and testing here: http://review.source.kitware.com/17578 Please try out that version with the rest of your changes. it's darn handy to have this just work without having to export COLOR='$(if $(MAKE_TERMOUT),FORCE)' in your ~/.bashrc or whatever. There's no GNU Makefiles generator, and I couldn't come up with a good way to implement this in the generic Unix Makefiles generator. We already have a CMAKE_COLOR_MAKEFILE option. Perhaps instead of just ON or OFF values it could have a GNU value that enables this behavior. When initializing it in CMakeGenericSystem.cmake perhaps it is possible to detect if CMAKE_MAKE_PROGRAM is a GNU make tool and provide a good default. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [CMake] Forcing colorization of output from cmake
Thanks Brad. I wrote a bunch more below and left it for posterity. However thinking more about this I wonder if we couldn't make this simpler. What I really want is that if MAKE_TERMOUT is set to a non-empty value, cmake should pretend that it's in a terminal regardless of what isatty() says. We can do this easily enough by adding a simple test to Terminal.c:kwsysTerminalStreamIsVT100() _without_ adding any code anywhere else: /* Check for a valid terminal. */ if(!default_vt100) { ... } + + /* If we're being run from GNU make 4.1+ see if IT thinks we have a TTY. */ + const char* termout = getenv(MAKE_TERMOUT); + if(termout termout[0] != '\0') +{ +return 1; +} In a way this is gross, certainly, but this function already checks for environment variable such as TERM, EMACS, etc. which are set by calling utilities and handles them specially. So why not MAKE_TERMOUT as well? That one change is enough for my particular use-case, without any other changes (don't need a FORCE setting for color). I think it would be useful to allow FORCE (I think this is a good capability to provide; as I mentioned other tools that support colorization such as GCC etc. to provide an always-type flag) but it would be decoupled from this GNU make capability. Thoughts? On Thu, 2014-10-09 at 09:55 -0400, Brad King wrote: The Source/kwsys part of the change actually belongs in a separate KWSys upstream first. I've added a modified version of your change for upstream review and testing here: http://review.source.kitware.com/17578 Please try out that version with the rest of your changes. OK. I see that in your version FORCE only comes into effect if we have a known good terminal (the new code comes after the check for valid terminal). Personally I would expect FORCE to be stronger than that, and return true regardless of TERM settings. Whether or not it should override EMACS env.var. I don't know... I'm on the fence. In my solution I had it really FORCE; that is, kwsysTerminalStreamIsVT100() effectively always was true if --switch=FORCE, regardless of EMACS or TERM. I'm not at all familiar with Windows so I have no strong opinions on whether FORCE should also force color output on a Windows console... although I know a number of people who do use GNU make on Windows and the Windows port of GNU make does set MAKE_TERMOUT properly. it's darn handy to have this just work without having to export COLOR='$(if $(MAKE_TERMOUT),FORCE)' in your ~/.bashrc or whatever. There's no GNU Makefiles generator, and I couldn't come up with a good way to implement this in the generic Unix Makefiles generator. We already have a CMAKE_COLOR_MAKEFILE option. Perhaps instead of just ON or OFF values it could have a GNU value that enables this behavior. When initializing it in CMakeGenericSystem.cmake perhaps it is possible to detect if CMAKE_MAKE_PROGRAM is a GNU make tool and provide a good default. Well, it's easy enough to detect if CMAKE_MAKE_PROGRAM is GNU make, if we can run it to test the output of ${CMAKE_MAKE_PROGRAM} --version. The question is, what do you do in the generated makefile if you see that it's GNU make? Anything you'd generate into the makefile that could choose to set --switch=FORCE would have to be GNU-specific (I can't think of a way to write it using POSIX standard makefile syntax). It would need to be the equivalent of: --switch=$(or $(COLOR),$(if $(MAKE_TERMOUT),FORCE)) to preserve the current behavior, where the COLOR variable setting can be inherited from the environment. What if someone runs cmake and it detects GNU make, but then they run /my/other/make which is not GNU make? That would work fine now but fail if we generated GNU-specific content in 'Unix Makefiles' generators. If we had a different generator, like 'GNU Makefiles' for example, then people who chose that would clearly expect the results would only work with GNU make, but we don't have that. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [CMake] Forcing colorization of output from cmake
On Wed, 2014-10-08 at 12:55 -0400, Brad King wrote: On 10/08/2014 12:15 PM, Paul Smith wrote: Maybe we could add another valid value to --switch, like --switch=FORCE which would always colorize. [snip] The best option seems to be to add another flag to the color bitflag variable that forces colorization always. Yes, I think both of the above make sense. If you want to work on it please read CONTRIBUTING.rst from the top of our source tree in Git ( http://cmake.org/cmake.git ) and come to the developers list with a proposal: Hi all. Attached please find a proposed patch for the above. I still think there's some slightly awkward redundancy between AssumeTTY and the new ForceTTY, but I'm not sure these can actually be combined into one flag... certainly not without reworking more of how AssumeTTY is interpreted and used than I felt confident with. I didn't try to allow FORCE to be case-insensitive. It wouldn't be hard, but I wasn't sure if it was worth it. You must capitalize it as the code is written today. I found precedent for both options. Also, the check for MAKE_TERMOUT might not be something you want to keep... there's not a lot of precedent in the code, that I found, for looking at other utilities' environment variables. On the other hand it's darn handy to have this just work without having to export COLOR='$(if $(MAKE_TERMOUT),FORCE)' in your ~/.bashrc or whatever. There's no GNU Makefiles generator, and I couldn't come up with a good way to implement this in the generic Unix Makefiles generator. From 82d2c63750c706e5f4f749a123934c725432 Mon Sep 17 00:00:00 2001 From: Paul Smith p...@mad-scientist.net Date: Wed, 8 Oct 2014 14:18:14 -0400 Subject: [PATCH] cmake: Allow forced color output Provide a new Terminal.h flag, Color_ForceTTY, which causes color output always regardless of TTY/Console settings. If --switch=FORCE is given on the cmake command line, enable this flag. Also check for the MAKE_TERMOUT variable, set by GNU make 4.1+ when make is capturing stdout which will eventually go to a terminal, and set Color_ForceTTY. --- Source/cmSystemTools.cxx | 36 +--- Source/cmcmd.cxx | 8 +++- Source/kwsys/Terminal.c| 8 +--- Source/kwsys/Terminal.h.in | 17 +++-- 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index fbb4416..7c983c6 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -2290,31 +2290,45 @@ std::string const cmSystemTools::GetCMakeRoot() void cmSystemTools::MakefileColorEcho(int color, const char* message, bool newline, bool enabled) { + if(!enabled) +{ +// Color is disabled. Print without color. +fprintf(stdout, %s%s, message, newline? \n : ); +return; +} + // On some platforms (an MSYS prompt) cmsysTerminal may not be able // to determine whether the stream is displayed on a tty. In this // case it assumes no unless we tell it otherwise. Since we want // color messages to be displayed for users we will assume yes. // However, we can test for some situations when the answer is most - // likely no. - int assumeTTY = cmsysTerminal_Color_AssumeTTY; + // likely no, and other cases where it should be forced to yes. + int flags = cmsysTerminal_Color_AssumeTTY; + if(cmSystemTools::GetEnv(DART_TEST_FROM_DART) || cmSystemTools::GetEnv(DASHBOARD_TEST_FROM_CTEST) || cmSystemTools::GetEnv(CTEST_INTERACTIVE_DEBUG_MODE)) { // Avoid printing color escapes during dashboard builds. -assumeTTY = 0; -} - - if(enabled) -{ -cmsysTerminal_cfprintf(color | assumeTTY, stdout, %s%s, - message, newline? \n : ); +flags = 0; } else { -// Color is disabled. Print without color. -fprintf(stdout, %s%s, message, newline? \n : ); +// Newer versions of GNU make (4.0+) have the ability to separate the +// output of parallel builds so they don't interfere with each other. +// However this means commands invoked by make don't appear to have a TTY. +// GNU make 4.1+ exports an environment variable MAKE_TERMOUT which is +// non-empty if make thinks that it is printing _it's_ output to stdout. +// If this variable exists and is non-empty, then force color output. +const char* var = cmSystemTools::GetEnv(MAKE_TERMOUT); +if(var var[0] != '\0') + { + flags |= cmsysTerminal_Color_ForceTTY; + } } + + cmsysTerminal_cfprintf(color | flags, stdout, %s%s, + message, newline? \n : ); } #endif diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index a0c67e0..af9b139 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -907,6 +907,7 @@ int cmcmd::ExecuteEchoColor(std::vectorstd::string args) bool enabled = true; int color = cmsysTerminal_Color_Normal; + int force = 0; bool