Skip to content

Commit

Permalink
[c++] General CMake clean-up (#2762) (#2773)
Browse files Browse the repository at this point in the history 
* Add options explicitly to targets instead of to everything. Note: this means compile options are no longer propagated to third-party libraries included with `FetchContent` and `ExternalProject_Add`.
* Move `find_package` calls and coverage flags to base `CMakeLists.txt` instead of adding them twice.
* Re-arrange some CMake calls to reduce redundancy and improve readability.
* Remove unused GIT_COMMIT_HASH compiler definition.
* Change TileDB deprecation flag to use Core flag that is not over-written.

Co-authored-by: Julia Dark <24235303+jp-dark@users.noreply.github.com>
  • Loading branch information
@github-actions @jp-dark
github-actions[bot] and jp-dark authored Jul 4, 2024
1 parent 673d13b commit e8cfe7c
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 129 deletions.
118 changes: 92 additions & 26 deletions libtiledbsoma/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,69 +165,135 @@ if(MSVC)
# C4702: unreachable code
# C4800: warning implicit cast int to bool
# C4996: deprecation warning about e.g. sscanf.
add_compile_options(/W4 /wd4101 /wd4146 /wd4244 /wd4251 /wd4456 /wd4457 /wd4702 /wd4800 /wd4996)
set(TILEDBSOMA_COMPILE_OPTIONS /W4 /wd4101 /wd4146 /wd4244 /wd4251 /wd4456 /wd4457 /wd4702 /wd4800 /wd4996)

# Warnings as errors:
if(TILEDBSOMA_ENABLE_WERROR)
add_compile_options(/WX)
set(TILEDBSOMA_WERROR_OPTION /WX)
else()
set(TILEDBSOMA_WERROR_OPTION "")
endif()

# Disable GDI (which we don't need, and causes some macro
# re-definition issues if wingdi.h is included)
add_compile_options(/DNOGDI)
list(APPEND TILEDBSOMA_COMPILE_OPTIONS /DNOGDI)

# Add /MPn flag from CMake invocation (if defined).
add_compile_options(${MSVC_MP_FLAG})
list(APPEND TILEDBSOMA_COMPILE_OPTIONS ${MSVC_MP_FLAG})

# Build-specific flags
add_compile_options(
"$<$<CONFIG:Debug>:/DDEBUG /Od /Zi /bigobj>"
"$<$<CONFIG:Release>:/DNDEBUG /Ox>"
"$<$<CONFIG:RelWithDebInfo>:/DNDEBUG /Ox /Zi>"
# Build-specific flags: Must use generator expressions to support multi-configuration
# build generators that set the config type at build time.
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<CONFIG:Debug>:/DDEBUG /Od /Zi /bigobj>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<CONFIG:Release>:/DNDEBUG /Ox>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<CONFIG:RelWithDebInfo>:/DNDEBUG /Ox /Zi>
)
else()
add_compile_options(-Wall -Wextra)

set(TILEDBSOMA_COMPILE_OPTIONS -Wall -Wextra)

if(TILEDBSOMA_ENABLE_WERROR)
add_compile_options(-Werror)
set(TILEDBSOMA_WERROR_OPTION -Werror)
else()
set(TILEDBSOMA_WERROR_OPTION "")
endif()

# Build-specific flags
if(CMAKE_BUILD_TYPE MATCHES "Debug")
add_compile_options(-DDEBUG -O0 -g3 -ggdb3 -gdwarf-3)
elseif(CMAKE_BUILD_TYPE MATCHES "Release")
add_compile_options(-DNDEBUG -O3)
elseif(CMAKE_BUILD_TYPE MATCHES "RelWithDebInfo")
add_compile_options(-DNDEBUG -O3 -g3 -ggdb3 -gdwarf-3)
elseif(CMAKE_BUILD_TYPE MATCHES "Coverage")
add_compile_options(-DDEBUG -g3 -gdwarf-3 --coverage)
add_link_options(--coverage)
endif()
# Build-specific flags: Must use generator expressions to support multi-configuration
# build generators that set the config type at build time.
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},Debug>: -DDEBUG -O0 -g3 -ggdb3 -gdwarf-3>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},Release>: -DNDEBUG -O3>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},RelWithDebInfo>: -DNDEBUG -O3 -g3 -ggdb3 -gdwarf-3>
)
list(APPEND
TILEDBSOMA_COMPILE_OPTIONS
$<$<STREQUAL:${CMAKE_BUILD_TYPE},>: -DDEBUG -g3 -gdwarf-3 --coverage>
)

# Use -Wno-literal-suffix on Linux with C++ sources.
# Use -Wno-literal-suffix on Linux for C++ libtiledbsoma target.
if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-literal-suffix>)
list(APPEND TILEDBSOMA_COMPILE_OPTIONS -Wno-literal-suffix)
endif()
endif()

# ###########################################################
# Compile options/definitions
# ###########################################################
if(SANITIZER)
string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER)

if(NOT CMAKE_BUILD_TYPE_LOWER MATCHES "debug")
message(FATAL_ERROR "Sanitizers only enabled for Debug build")
endif()

string(TOLOWER ${SANITIZER} SANITIZER)

if(NOT SANITIZER MATCHES "^(address|memory|leak|thread|undefined)$")
message(FATAL_ERROR "Unknown clang sanitizer: ${SANITIZER})")
else()
message(STATUS "The TileDB-SOMA library is compiled with sanitizer ${SANITIZER} enabled")
endif()

set(TILEDBSOMA_SANITIZER_OPTIONS
-g -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=${SANITIZER}
)
else()
set(TILEDBSOMA_SANITIZER_OPTIONS "")
endif()



# Definitions for all targets
add_definitions(-D_FILE_OFFSET_BITS=64)

# Disable incorrect availability check on conda build
# https://conda-forge.org/docs/maintainer/knowledge_base.html#newer-c-features-with-old-sdk
add_compile_options(-D_LIBCPP_DISABLE_AVAILABILITY)
add_definitions(-D_LIBCPP_DISABLE_AVAILABILITY)

# AVX2 flag
include(CheckAVX2Support)
CheckAVX2Support()

if(COMPILER_SUPPORTS_AVX2)
add_compile_options(${COMPILER_AVX2_FLAG})
list(APPEND TILEDBSOMA_COMPILE_OPTIONS ${COMPILER_AVX2_FLAG})
endif()


## Leaving this helper. Debugging generator expressions in CMake can be difficult.
## To view compile options uncomment the below and run the target.
## (Hint: This target is inside build/libtiledbsoma)
add_custom_target(
debugflag COMMAND ${CMAKE_COMMAND} -E echo "compile options: ${TILEDBSOMA_COMPILE_OPTIONS}"
)


# ###########################################################
# Regular build
# ###########################################################

# Adding coverage flags
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} $ENV{TILEDBSOMA_COVERAGE}")
set(CMAKE_C_FLAGS "${CMAKE_CXX_FLAGS} $ENV{TILEDBSOMA_COVERAGE}")

# Find required dependencies
# See ../cmake/Modules/*.cmake for provenance/version info
find_package(TileDB_EP REQUIRED)
find_package(Spdlog_EP REQUIRED)

add_subdirectory(src)

if(TILEDBSOMA_ENABLE_TESTING)
Expand Down
106 changes: 16 additions & 90 deletions libtiledbsoma/src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1,51 +1,5 @@
message(STATUS "Starting TileDB-SOMA build.")

set(TILEDBSOMA_INSTALL_TARGETS "")

# ###########################################################
# Adding coverage flags
# ###########################################################

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} $ENV{TILEDBSOMA_COVERAGE}")
set(CMAKE_C_FLAGS "${CMAKE_CXX_FLAGS} $ENV{TILEDBSOMA_COVERAGE}")

# ###########################################################
# Find required dependencies
# See ../cmake/Modules/*.cmake for provenance/version info
# ###########################################################
find_package(TileDB_EP REQUIRED)
find_package(Spdlog_EP REQUIRED)

# ###########################################################
# Get source commit hash
# ###########################################################
find_package(Git REQUIRED)

execute_process(
COMMAND "${GIT_EXECUTABLE}" describe --exact-match --tags HEAD
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}"
RESULT_VARIABLE res
OUTPUT_VARIABLE BUILD_COMMIT_HASH
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE)

# If we didn't find a tag name let's grab the SHA
if(res)
execute_process(
COMMAND "${GIT_EXECUTABLE}" describe --dirty=-modified --always
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}"
RESULT_VARIABLE res
OUTPUT_VARIABLE BUILD_COMMIT_HASH
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()

set_property(GLOBAL APPEND
PROPERTY CMAKE_CONFIGURE_DEPENDS
"${CMAKE_SOURCE_DIR}/.git/index")

message(STATUS "Building with commit hash ${BUILD_COMMIT_HASH}")

# ###########################################################
# Common object library
# ###########################################################
Expand All @@ -72,20 +26,21 @@ add_library(TILEDB_SOMA_OBJECTS OBJECT
${CMAKE_CURRENT_SOURCE_DIR}/utils/stats.cc
${CMAKE_CURRENT_SOURCE_DIR}/utils/util.cc
${CMAKE_CURRENT_SOURCE_DIR}/utils/version.cc

${CMAKE_CURRENT_SOURCE_DIR}/external/src/thread_pool/thread_pool.cc
${CMAKE_CURRENT_SOURCE_DIR}/external/src/thread_pool/status.cc
${CMAKE_CURRENT_SOURCE_DIR}/external/src/nanoarrow/nanoarrow.c
)

message(STATUS "Building TileDB without deprecation warnings")
target_compile_definitions(TILEDB_SOMA_OBJECTS PRIVATE
-DBUILD_COMMIT_HASH="${BUILD_COMMIT_HASH}"
-DTILEDB_DEPRECATED=
target_compile_definitions(TILEDB_SOMA_OBJECTS
PRIVATE
-DTILEDB_NO_API_DEPRECATION_WARNINGS
)

target_compile_options(
TILEDB_SOMA_OBJECTS PRIVATE
target_compile_options(TILEDB_SOMA_OBJECTS
PRIVATE
${TILEDBSOMA_COMPILE_OPTIONS}
${TILEDBSOMA_WERROR_OPTION}
${TILEDBSOMA_SANITIZER_OPTIONS}
)

set_property(TARGET TILEDB_SOMA_OBJECTS PROPERTY POSITION_INDEPENDENT_CODE ON)
Expand All @@ -107,39 +62,15 @@ target_include_directories(TILEDB_SOMA_OBJECTS
${pybind11_INCLUDE_DIRS}
)

# ###########################################################
# Compile options/definitions
# ###########################################################
if(SANITIZER)
string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER)

if(NOT CMAKE_BUILD_TYPE_LOWER MATCHES "debug")
message(FATAL_ERROR "Sanitizers only enabled for Debug build")
endif()

string(TOLOWER ${SANITIZER} SANITIZER)

if(NOT SANITIZER MATCHES "^(address|memory|leak|thread|undefined)$")
message(FATAL_ERROR "Unknown clang sanitizer: ${SANITIZER})")
else()
message(STATUS "The TileDB-SOMA library is compiled with sanitizer ${SANITIZER} enabled")
endif()

target_compile_options(TILEDB_SOMA_OBJECTS
PRIVATE
-g -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=${SANITIZER}
)
endif()

# ###########################################################
# tiledbsoma library target
# ###########################################################
set(TILEDBSOMA_INSTALL_TARGETS "")
if(TILEDBSOMA_BUILD_STATIC)
add_library(tiledbsoma_static STATIC
$<TARGET_OBJECTS:TILEDB_SOMA_OBJECTS>
)
list(APPEND TILEDBSOMA_INSTALL_TARGETS tiledbsoma_static)

if(WIN32)
# On Windows we must name the static library something else to avoid
# name clash with the DLL's "import library" .lib file.
Expand All @@ -153,28 +84,23 @@ if(TILEDBSOMA_BUILD_STATIC)
OUTPUT_NAME "tiledbsoma"
)
endif()
if(SANITIZER)
target_link_libraries(tiledbsoma_static
INTERFACE
-fsanitize=${SANITIZER}
)
endif()
else()
add_library(tiledbsoma SHARED
$<TARGET_OBJECTS:TILEDB_SOMA_OBJECTS>
)

list(APPEND TILEDBSOMA_INSTALL_TARGETS tiledbsoma)

target_link_libraries(tiledbsoma
PUBLIC
TileDB::tiledb_shared
spdlog::spdlog
)
endif()

# Sanitizer linker flags
if(SANITIZER)
if(TILEDBSOMA_BUILD_STATIC)
target_link_libraries(tiledbsoma_static
INTERFACE
-fsanitize=${SANITIZER}
)
else()
if(SANITIZER)
target_link_libraries(tiledbsoma
INTERFACE
-fsanitize=${SANITIZER}
Expand Down
22 changes: 9 additions & 13 deletions libtiledbsoma/test/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,6 @@
get_filename_component(TILEDBSOMA_SOURCE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../" ABSOLUTE)
add_compile_definitions(TILEDBSOMA_SOURCE_ROOT="${TILEDBSOMA_SOURCE_ROOT}")

# ###########################################################
# Adding coverage flags
# ###########################################################
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} $ENV{TILEDBSOMA_COVERAGE}")
set(CMAKE_C_FLAGS "${CMAKE_CXX_FLAGS} $ENV{TILEDBSOMA_COVERAGE}")

############################################################
# Dependencies
############################################################

find_package(TileDB_EP REQUIRED)
find_package(Spdlog_EP REQUIRED)

############################################################
# SOMA unit test
Expand Down Expand Up @@ -57,7 +45,15 @@ target_include_directories(unit_soma
$<TARGET_PROPERTY:spdlog::spdlog,INTERFACE_INCLUDE_DIRECTORIES>
)

target_compile_definitions(unit_soma PRIVATE CATCH_CONFIG_MAIN)
target_compile_definitions(unit_soma
PRIVATE
CATCH_CONFIG_MAIN
)
target_compile_options(unit_soma
PRIVATE
${TILEDBSOMA_COMPILE_OPTIONS}
${TILEDBSOMA_WERROR_OPTION}
)

if (NOT MSVC)
# Allow deprecated function for writing to an array with a timestamp
Expand Down

0 comments on commit e8cfe7c

Please sign in to comment.