Skip to content

Commit

Permalink
build: stop relying on CMAKE_BUILD_TYPE to determine the build type
Browse files Browse the repository at this point in the history
Any logic involving CMAKE_BUILD_TYPE is automatically broken as it won't
work with multi-config generators. The only exception is if we
explicitly check whether the current generator is single-config as well.
Instead, use generator expressions or cmake variables that allows to set
options for certain build types only such as
INTERPROCEDURAL_OPTIMIZATION_<CONFIG>.
  • Loading branch information
dundargoc committed Feb 1, 2023
1 parent 9ce44a7 commit f266e24
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 141 deletions.
37 changes: 0 additions & 37 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,28 +130,6 @@ set(NVIM_API_LEVEL 11) # Bump this after any API change.
set(NVIM_API_LEVEL_COMPAT 0) # Adjust this after a _breaking_ API change.
set(NVIM_API_PRERELEASE true)

set(NVIM_VERSION_BUILD_TYPE "${CMAKE_BUILD_TYPE}")
# NVIM_VERSION_CFLAGS set further below.

# Log level (MIN_LOG_LEVEL in log.h)
if("${MIN_LOG_LEVEL}" MATCHES "^$")
# Minimize logging for release-type builds.
if(CMAKE_BUILD_TYPE STREQUAL "Release"
OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo"
OR CMAKE_BUILD_TYPE STREQUAL "MinSizeRel")
message(STATUS "MIN_LOG_LEVEL not specified, default is 3 (ERROR) for release builds")
set(MIN_LOG_LEVEL 3)
else()
message(STATUS "MIN_LOG_LEVEL not specified, default is 1 (INFO)")
set(MIN_LOG_LEVEL 1)
endif()
else()
if(NOT MIN_LOG_LEVEL MATCHES "^[0-3]$")
message(FATAL_ERROR "invalid MIN_LOG_LEVEL: " ${MIN_LOG_LEVEL})
endif()
message(STATUS "MIN_LOG_LEVEL=${MIN_LOG_LEVEL}")
endif()

# Default to -O2 on release builds.
if(CMAKE_C_FLAGS_RELEASE MATCHES "-O3")
message(STATUS "Replacing -O3 in CMAKE_C_FLAGS_RELEASE with -O2")
Expand All @@ -175,20 +153,6 @@ if(CMAKE_C_FLAGS_RELWITHDEBINFO MATCHES DNDEBUG)
string(REPLACE "-DNDEBUG" "" CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO}")
endif()

# gcc 4.0+ sets _FORTIFY_SOURCE=2 automatically. This currently
# does not work with Neovim due to some uses of dynamically-sized structures.
# https://github.com/neovim/neovim/issues/223

# Include the build type's default flags in the check for _FORTIFY_SOURCE,
# otherwise we may incorrectly identify the level as acceptable and find out
# later that it was not when optimizations were enabled. CFLAGS is applied
# even though you don't see it in CMAKE_REQUIRED_FLAGS.
set(INIT_FLAGS_NAME CMAKE_C_FLAGS_${CMAKE_BUILD_TYPE})
string(TOUPPER ${INIT_FLAGS_NAME} INIT_FLAGS_NAME)
if(${INIT_FLAGS_NAME})
set(CMAKE_REQUIRED_FLAGS "${${INIT_FLAGS_NAME}}")
endif()

option(LOG_LIST_ACTIONS "Add list actions logging" OFF)

option(CLANG_ASAN_UBSAN "Enable Clang address & undefined behavior sanitizer for nvim binary." OFF)
Expand Down Expand Up @@ -347,7 +311,6 @@ install_helper(
#

add_subdirectory(src/nvim)
get_directory_property(NVIM_VERSION_CFLAGS DIRECTORY src/nvim DEFINITION NVIM_VERSION_CFLAGS)
add_subdirectory(cmake.config)
add_subdirectory(test/functional/fixtures) # compile test programs
add_subdirectory(runtime)
Expand Down
18 changes: 13 additions & 5 deletions cmake.config/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,19 @@ configure_file (
"${PROJECT_BINARY_DIR}/cmake.config/auto/config.h"
)

# generate version definitions
configure_file (
"${PROJECT_SOURCE_DIR}/cmake.config/versiondef.h.in"
"${PROJECT_BINARY_DIR}/cmake.config/auto/versiondef.h"
)
# Improved :version output on newer cmake versions
if(${CMAKE_VERSION} VERSION_LESS 3.14)
configure_file(
"${PROJECT_SOURCE_DIR}/cmake.config/versiondef-old.h.in"
"${PROJECT_BINARY_DIR}/cmake.config/auto/versiondef.h.gen")
else()
configure_file(
"${PROJECT_SOURCE_DIR}/cmake.config/versiondef.h.in"
"${PROJECT_BINARY_DIR}/cmake.config/auto/versiondef.h.gen")
endif()
file(GENERATE
OUTPUT "${PROJECT_BINARY_DIR}/cmake.config/auto/versiondef.h"
INPUT "${PROJECT_BINARY_DIR}/cmake.config/auto/versiondef.h.gen")

# generate pathdef.c
find_program(WHOAMI_PROG whoami)
Expand Down
21 changes: 21 additions & 0 deletions cmake.config/versiondef-old.h.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#ifndef AUTO_VERSIONDEF_H
#define AUTO_VERSIONDEF_H

#define NVIM_VERSION_MAJOR @NVIM_VERSION_MAJOR@
#define NVIM_VERSION_MINOR @NVIM_VERSION_MINOR@
#define NVIM_VERSION_PATCH @NVIM_VERSION_PATCH@
#define NVIM_VERSION_PRERELEASE "@NVIM_VERSION_PRERELEASE@"

#cmakedefine NVIM_VERSION_MEDIUM "@NVIM_VERSION_MEDIUM@"
#ifndef NVIM_VERSION_MEDIUM
# include "auto/versiondef_git.h"
#endif

#define NVIM_API_LEVEL @NVIM_API_LEVEL@
#define NVIM_API_LEVEL_COMPAT @NVIM_API_LEVEL_COMPAT@
#define NVIM_API_PRERELEASE @NVIM_API_PRERELEASE@

#define NVIM_VERSION_CFLAGS "${CMAKE_C_COMPILER} $<JOIN:$<TARGET_PROPERTY:nvim,COMPILE_OPTIONS>, > -D$<JOIN:$<TARGET_PROPERTY:nvim,COMPILE_DEFINITIONS>, -D> -I$<JOIN:$<TARGET_PROPERTY:nvim,INCLUDE_DIRECTORIES>, -I>"
#define NVIM_VERSION_BUILD_TYPE "$<CONFIG>"

#endif // AUTO_VERSIONDEF_H
4 changes: 2 additions & 2 deletions cmake.config/versiondef.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#define NVIM_API_LEVEL_COMPAT @NVIM_API_LEVEL_COMPAT@
#define NVIM_API_PRERELEASE @NVIM_API_PRERELEASE@

#define NVIM_VERSION_CFLAGS "@NVIM_VERSION_CFLAGS@"
#define NVIM_VERSION_BUILD_TYPE "@NVIM_VERSION_BUILD_TYPE@"
#define NVIM_VERSION_CFLAGS "${CMAKE_C_COMPILER} $<JOIN:$<TARGET_PROPERTY:nvim,COMPILE_OPTIONS>, > -D$<JOIN:$<TARGET_PROPERTY:nvim,COMPILE_DEFINITIONS>, -D> -I$<JOIN:$<REMOVE_DUPLICATES:$<TARGET_PROPERTY:nvim,INCLUDE_DIRECTORIES>>, -I>"
#define NVIM_VERSION_BUILD_TYPE "$<CONFIG>"

#endif // AUTO_VERSIONDEF_H
57 changes: 0 additions & 57 deletions cmake/GetCompileFlags.cmake

This file was deleted.

58 changes: 18 additions & 40 deletions src/nvim/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -219,34 +219,6 @@ endif()
list(REMOVE_ITEM CMAKE_REQUIRED_INCLUDES "${TreeSitter_INCLUDE_DIRS}")
list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES "${TreeSitter_LIBRARIES}")

# Include <string.h> because some toolchains define _FORTIFY_SOURCE=2 in
# internal header files, which should in turn be #included by <string.h>.
check_c_source_compiles("
#include <string.h>
#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 1
#error \"_FORTIFY_SOURCE > 1\"
#endif
int
main(void)
{
return 0;
}
" HAS_ACCEPTABLE_FORTIFY)

if(NOT HAS_ACCEPTABLE_FORTIFY)
message(STATUS "Unsupported _FORTIFY_SOURCE found, forcing _FORTIFY_SOURCE=1")
# Extract possible prefix to _FORTIFY_SOURCE (e.g. -Wp,-D_FORTIFY_SOURCE).
string(REGEX MATCH "[^\ ]+-D_FORTIFY_SOURCE" _FORTIFY_SOURCE_PREFIX "${CMAKE_C_FLAGS}")
string(REPLACE "-D_FORTIFY_SOURCE" "" _FORTIFY_SOURCE_PREFIX "${_FORTIFY_SOURCE_PREFIX}" )
if(NOT _FORTIFY_SOURCE_PREFIX STREQUAL "")
message(STATUS "Detected _FORTIFY_SOURCE Prefix=${_FORTIFY_SOURCE_PREFIX}")
endif()
# -U in add_definitions doesn't end up in the correct spot, so we add it to
# the flags variable instead.
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${_FORTIFY_SOURCE_PREFIX}-U_FORTIFY_SOURCE ${_FORTIFY_SOURCE_PREFIX}-D_FORTIFY_SOURCE=1")
endif()

target_compile_definitions(main_lib INTERFACE INCLUDE_GENERATED_DECLARATIONS)

# Remove --sort-common from linker flags, as this seems to cause bugs (see #2641, #3374).
Expand Down Expand Up @@ -440,7 +412,15 @@ else()
${EXTERNAL_SOURCES} PROPERTIES COMPILE_FLAGS "${COMPILE_FLAGS} -Wno-conversion -Wno-missing-noreturn -Wno-missing-format-attribute -Wno-double-promotion -Wno-strict-prototypes")
endif()

if(NOT "${MIN_LOG_LEVEL}" MATCHES "^$")
# Log level (MIN_LOG_LEVEL in log.h)
if("${MIN_LOG_LEVEL}" MATCHES "^$")
# Minimize logging for release-type builds.
target_compile_definitions(main_lib INTERFACE MIN_LOG_LEVEL=$<IF:$<CONFIG:Debug>,1,3>)
else()
if(NOT MIN_LOG_LEVEL MATCHES "^[0-3]$")
message(FATAL_ERROR "invalid MIN_LOG_LEVEL: " ${MIN_LOG_LEVEL})
endif()
message(STATUS "MIN_LOG_LEVEL=${MIN_LOG_LEVEL}")
target_compile_definitions(main_lib INTERFACE MIN_LOG_LEVEL=${MIN_LOG_LEVEL})
endif()

Expand All @@ -464,9 +444,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND CMAKE_OSX_SYSROOT)
list(APPEND gen_cflags "${CMAKE_OSX_SYSROOT}")
endif()
string(TOUPPER "${CMAKE_BUILD_TYPE}" build_type)
separate_arguments(C_FLAGS_ARRAY UNIX_COMMAND ${CMAKE_C_FLAGS})
separate_arguments(C_FLAGS_${build_type}_ARRAY UNIX_COMMAND ${CMAKE_C_FLAGS_${build_type}})
set(gen_cflags ${gen_cflags} ${C_FLAGS_${build_type}_ARRAY} ${C_FLAGS_ARRAY})
set(gen_cflags ${gen_cflags} -O2)

set(NVIM_VERSION_GIT_H ${PROJECT_BINARY_DIR}/cmake.config/auto/versiondef_git.h)
add_custom_target(update_version_stamp
Expand Down Expand Up @@ -681,10 +659,10 @@ endif()

if(NOT LUAJIT_FOUND)
message(STATUS "luajit not found, skipping unit tests")
elseif(CMAKE_BUILD_TYPE MATCHES Debug)
else()
glob_wrapper(UNIT_TEST_FIXTURES ${PROJECT_SOURCE_DIR}/test/unit/fixtures/*.c)
list(APPEND NVIM_SOURCES ${UNIT_TEST_FIXTURES})
target_compile_definitions(main_lib INTERFACE UNIT_TESTING)
target_sources(nvim PRIVATE $<$<CONFIG:Debug>:${UNIT_TEST_FIXTURES}>)
target_compile_definitions(nvim PRIVATE $<$<CONFIG:Debug>:UNIT_TESTING>)
endif()

target_sources(nvim PRIVATE ${NVIM_GENERATED_FOR_SOURCES} ${NVIM_GENERATED_FOR_HEADERS}
Expand All @@ -709,8 +687,11 @@ endif()
if(ENABLE_LTO)
include(CheckIPOSupported)
check_ipo_supported(RESULT IPO_SUPPORTED)
if(IPO_SUPPORTED AND (NOT CMAKE_BUILD_TYPE MATCHES Debug))
set_target_properties(nvim PROPERTIES INTERPROCEDURAL_OPTIMIZATION TRUE)
if(IPO_SUPPORTED)
set_target_properties(nvim PROPERTIES
INTERPROCEDURAL_OPTIMIZATION_RELEASE TRUE
INTERPROCEDURAL_OPTIMIZATION_RELWITHDEBINFO TRUE
INTERPROCEDURAL_OPTIMIZATION_MINSIZEREL TRUE)
endif()
endif()

Expand Down Expand Up @@ -970,6 +951,3 @@ add_custom_target(generated-sources DEPENDS
)

add_subdirectory(po)

include(GetCompileFlags)
get_compile_flags(NVIM_VERSION_CFLAGS)

0 comments on commit f266e24

Please sign in to comment.