Apply code review comments to the cmake file

y-scope · Oct 25, 2024 · e288ceb · e288ceb
1 parent b042432
commit e288ceb
Showing 1 changed file with 44 additions and 67 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,14 +1,12 @@
-# Minimum cmake version required by scikit-build-core: 3.15
-cmake_minimum_required(VERSION 3.15)
-
-set(CLP_FFI_PY_INSTALL_LIBS ON CACHE BOOL "Whether to install the built libraries." FORCE)
+cmake_minimum_required(VERSION 3.26..3.30)
 
 # Define the variables necessary to build and install the project differently depending on whether
 # the project is being configured from scikit-build-core. If configured from outside
 # scikit-build-core, we need to use placeholder values for the variables that are usually defined
 # by scikit-build-core.
 if(DEFINED SKBUILD_PROJECT_NAME)
-    set(CLP_FFI_PY
+    set(CLP_FFI_PY_INSTALL_LIBS ON CACHE BOOL "Enable installing the built libraries." FORCE)
+    set(CLP_FFI_PY_PROJECT_NAME
         ${SKBUILD_PROJECT_NAME}
         CACHE STRING
         "The name of the project parsed by scikit-build-core."
@@ -21,16 +19,8 @@ if(DEFINED SKBUILD_PROJECT_NAME)
         FORCE
     )
 else()
-    # The project is configured outside of scikit-build-core. Normally this is for build testing
-    # purpose, or loading the project into CLion. We don't install the built library in this config.
-    message(WARNING "Configured outside of scikit-build-core. Library installation is disabled.")
-    set(CLP_FFI_PY_INSTALL_LIBS
-        OFF
-        CACHE BOOL
-        "Disable installing the built library."
-        FORCE
-    )
-    set(CLP_FFI_PY "clp-ffi-py" CACHE STRING "Use a placeholder project name." FORCE)
+    set(CLP_FFI_PY_INSTALL_LIBS OFF CACHE BOOL "Disable installing the built library." FORCE)
+    set(CLP_FFI_PY_PROJECT_NAME "clp-ffi-py" CACHE STRING "Use a placeholder project name." FORCE)
     set(CLP_FFI_PY_VERSION "0.0.0" CACHE STRING "Use a placeholder version." FORCE)
     set(CMAKE_EXPORT_COMPILE_COMMANDS
         ON
@@ -40,12 +30,7 @@ else()
     )
 endif()
 
-project(
-    ${CLP_FFI_PY}
-    LANGUAGES
-        CXX
-    VERSION ${CLP_FFI_PY_VERSION}
-)
+project(${CLP_FFI_PY_PROJECT_NAME} LANGUAGES CXX VERSION ${CLP_FFI_PY_VERSION})
 
 # C-extension modules will be compiled as shared libraries. To enable dynamic linking, all libraries
 # (including the static ones) must be compiled with the position-independent-code option.
@@ -69,15 +54,13 @@ python_add_library(
 target_compile_features(${CLP_FFI_PY_LIB_IR} PRIVATE cxx_std_20)
 
 set(SRC_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/src")
-set(CLP_FFI_PY_SRC_ROOT
-    "${SRC_ROOT}/clp_ffi_py"
-)
+set(CLP_FFI_PY_LIB_SRC_DIR "${SRC_ROOT}/clp_ffi_py")
 set(CLP_CORE_ROOT "${SRC_ROOT}/clp/components/core")
 
 add_subdirectory(${CLP_CORE_ROOT}/src/clp/string_utils)
 
 # Note: we don't add headers hear since they are not considered to be a part of the project.
-set(SOURCE_FILES_CLP_FFI_CORE
+set(CLP_FFI_PY_CLP_CORE_SOURCES
     ${CLP_CORE_ROOT}/src/clp/BufferReader.cpp
     ${CLP_CORE_ROOT}/src/clp/ffi/ir_stream/decoding_methods.cpp
     ${CLP_CORE_ROOT}/src/clp/ffi/ir_stream/encoding_methods.cpp
@@ -88,45 +71,45 @@ set(SOURCE_FILES_CLP_FFI_CORE
 )
 
 # Note: headers should be included to allow CLion loads the project properly.
-set(SOURCE_FILES_CLP_FFI_PY_LIB_IR
-    ${CLP_FFI_PY_SRC_ROOT}/error_messages.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ExceptionFFI.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/deserialization_methods.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/deserialization_methods.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/LogEvent.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/Metadata.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/Metadata.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyDeserializer.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyDeserializer.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyDeserializerBuffer.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyDeserializerBuffer.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyFourByteSerializer.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyFourByteSerializer.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyLogEvent.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyLogEvent.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyMetadata.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyMetadata.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyQuery.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/PyQuery.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/Query.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/Query.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/serialization_methods.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/ir/native/serialization_methods.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/modules/ir_native.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/Py_utils.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/Py_utils.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/PyObjectCast.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/PyObjectUtils.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/Python.hpp
-    ${CLP_FFI_PY_SRC_ROOT}/utils.cpp
-    ${CLP_FFI_PY_SRC_ROOT}/utils.hpp
+set(CLP_FFI_PY_LIB_IR_SOURCES
+    ${CLP_FFI_PY_LIB_SRC_DIR}/error_messages.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ExceptionFFI.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/deserialization_methods.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/deserialization_methods.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/LogEvent.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/Metadata.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/Metadata.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyDeserializer.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyDeserializer.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyDeserializerBuffer.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyDeserializerBuffer.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyFourByteSerializer.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyFourByteSerializer.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyLogEvent.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyLogEvent.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyMetadata.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyMetadata.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyQuery.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/PyQuery.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/Query.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/Query.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/serialization_methods.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/ir/native/serialization_methods.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/modules/ir_native.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/Py_utils.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/Py_utils.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/PyObjectCast.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/PyObjectUtils.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/Python.hpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/utils.cpp
+    ${CLP_FFI_PY_LIB_SRC_DIR}/utils.hpp
 )
 
 target_sources(
     ${CLP_FFI_PY_LIB_IR}
     PRIVATE
-        ${SOURCE_FILES_CLP_FFI_CORE}
-        ${SOURCE_FILES_CLP_FFI_PY_LIB_IR}
+        ${CLP_FFI_PY_CLP_CORE_SOURCES}
+        ${CLP_FFI_PY_LIB_IR_SOURCES}
 )
 
 # NOTE: We mark the include directories below as system headers so that the compiler (including
@@ -139,11 +122,7 @@ target_include_directories(
         ${CLP_CORE_ROOT}/submodules
 )
 
-target_include_directories(
-    ${CLP_FFI_PY_LIB_IR}
-    PRIVATE
-        ${SRC_ROOT}
-)
+target_include_directories(${CLP_FFI_PY_LIB_IR} PRIVATE ${SRC_ROOT})
 
 target_link_libraries(${CLP_FFI_PY_LIB_IR} PRIVATE clp::string_utils)
 
@@ -152,8 +131,6 @@ if(CLP_FFI_PY_INSTALL_LIBS)
         TARGETS
             ${CLP_FFI_PY_LIB_IR}
             DESTINATION
-            ${CLP_FFI_PY}/ir
-            COMPONENT
-            PythonModule
+            ${CLP_FFI_PY_PROJECT_NAME}/ir
     )
 endif()