Skip to content

Commit

Permalink
Fix colored cmake output when using Ninja (#7444)
Browse files Browse the repository at this point in the history
Summary:
Compilers usually color output by default if they detect the output is a terminal, but this breaks if you are using Ninja. Adding an option to let users explicitly control it, and setting it automatically when Ninja is used.

Pull Request resolved: #7444

Reviewed By: kgpai

Differential Revision: D51056403

Pulled By: pedroerp

fbshipit-source-id: cf40e58b23823ac7e9682ae3b412c43ccbd71aee
  • Loading branch information
pedroerp authored and facebook-github-bot committed Nov 7, 2023
1 parent 5ff9cee commit 9e8e6ae
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
20 changes: 18 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.
cmake_minimum_required(VERSION 3.14)

# the policy allows us to change options without caching
# The policy allows us to change options without caching.
cmake_policy(SET CMP0077 NEW)
set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)

Expand All @@ -25,7 +25,7 @@ if(POLICY CMP0135)
set(CMAKE_POLICY_DEFAULT_CMP0135 NEW)
endif()

# set the project name
# Set the project name.
project(velox)

# If we are in an active conda env disable search in system paths and add env to
Expand Down Expand Up @@ -62,6 +62,7 @@ option(
VELOX_BUILD_MINIMAL
"Build a minimal set of components only. This will override other build options."
OFF)

# option() always creates a BOOL variable so we have to use a normal cache
# variable with STRING type for this option.
#
Expand Down Expand Up @@ -106,6 +107,12 @@ option(
"make buildPartitionBounds_ a vector int64 instead of int32 to avoid integer overflow when the hashtable has billions of records"
OFF)

# Explicitly force compilers to generate colored output. Compilers usually do
# this by default if they detect the output is a terminal, but this assumption
# is broken if you use ninja.
option(VELOX_FORCE_COLORED_OUTPUT
"Always produce ANSI-colored output (GNU/Clang only)." OFF)

if(${VELOX_BUILD_MINIMAL})
# Enable and disable components for velox base build
set(VELOX_BUILD_TESTING OFF)
Expand Down Expand Up @@ -194,6 +201,15 @@ if(VELOX_ENABLE_CCACHE
endif()
endif()

if(${VELOX_FORCE_COLORED_OUTPUT})
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
add_compile_options(-fdiagnostics-color=always)
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang"
OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
add_compile_options(-fcolor-diagnostics)
endif()
endif()

# At the moment we prefer static linking but by default cmake looks for shared
# libs first. This will still fallback to shared libs when static ones are not
# found
Expand Down
8 changes: 6 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ endif
USE_NINJA ?= 1
ifeq ($(USE_NINJA), 1)
ifneq ($(shell which ninja), )
GENERATOR=-GNinja -DMAX_LINK_JOBS=$(MAX_LINK_JOBS) -DMAX_HIGH_MEM_JOBS=$(MAX_HIGH_MEM_JOBS)
GENERATOR := -GNinja
GENERATOR += -DMAX_LINK_JOBS=$(MAX_LINK_JOBS)
GENERATOR += -DMAX_HIGH_MEM_JOBS=$(MAX_HIGH_MEM_JOBS)

# Ninja makes compilers disable colored output by default.
GENERATOR += -DVELOX_FORCE_COLORED_OUTPUT=ON
endif
endif

Expand Down Expand Up @@ -84,7 +89,6 @@ cmake: #: Use CMake to create a Makefile build system
${CMAKE_FLAGS} \
$(GENERATOR) \
$(USE_CCACHE) \
$(FORCE_COLOR) \
${EXTRA_CMAKE_FLAGS}

cmake-gpu:
Expand Down

0 comments on commit 9e8e6ae

Please sign in to comment.