enforce cmake-format and cmake-lint, other small packaging changes

rapidsai · Jan 23, 2025 · 1b12401 · 1b12401
1 parent 5a5cd99
commit 1b12401
Show file tree

Hide file tree

Showing 14 changed files with 229 additions and 111 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -49,7 +49,7 @@ repos:
                 - --fix
                 - --rapids-version=25.02
       - repo: https://github.com/rapidsai/dependency-file-generator
-        rev: v1.16.0
+        rev: v1.17.0
         hooks:
             - id: rapids-dependency-file-generator
               args: ["--clean"]
@@ -63,6 +63,30 @@ repos:
               language: system
               pass_filenames: false
               verbose: true
+            - id: cmake-format
+              name: cmake-format
+              entry: ./cpp/scripts/run-cmake-format.sh cmake-format
+              language: python
+              types: [cmake]
+              #exclude: .*/thirdparty/.*
+              # Note that pre-commit autoupdate does not update the versions
+              # of dependencies, so we'll have to update this manually.
+              additional_dependencies:
+                - cmakelang==0.6.13
+              verbose: true
+              require_serial: true
+            - id: cmake-lint
+              name: cmake-lint
+              entry: ./cpp/scripts/run-cmake-format.sh cmake-lint
+              language: python
+              types: [cmake]
+              #exclude: .*/thirdparty/.*
+              # Note that pre-commit autoupdate does not update the versions
+              # of dependencies, so we'll have to update this manually.
+              additional_dependencies:
+                - cmakelang==0.6.13
+              verbose: true
+              require_serial: true
       - repo: https://github.com/codespell-project/codespell
         rev: v2.2.2
         hooks:

diff --git a/ci/check_style.sh b/ci/check_style.sh
@@ -16,5 +16,11 @@ rapids-dependency-file-generator \
 rapids-mamba-retry env create --yes -f env.yaml -n checks
 conda activate checks
 
+# get config for cmake-format checks
+FORMAT_FILE_URL="https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-25.02/cmake-format-rapids-cmake.json"
+export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json
+mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE})
+wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL}
+
 # Run pre-commit checks
 pre-commit run --all-files --show-diff-on-failure
diff --git a/ci/release/update-version.sh b/ci/release/update-version.sh
@@ -69,6 +69,7 @@ done
 
 # rapids-cmake version
 sed_runner 's/'"branch-.*\/RAPIDS.cmake"'/'"branch-${NEXT_RAPIDS_SHORT_TAG}\/RAPIDS.cmake"'/g' fetch_rapids.cmake
+sed_runner "s/branch-[[:digit:]]\{2\}.[[:digit:]]\{2\}/branch-${NEXT_RAPIDS_SHORT_TAG}/g" ./ci/check_style.sh
 
 for FILE in .github/workflows/*.yaml; do
   sed_runner "/shared-workflows/ s/@.*/@branch-${NEXT_RAPIDS_SHORT_TAG}/g" "${FILE}"

diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
@@ -16,7 +16,10 @@ if(_version_contents MATCHES "^([0-9]+)\\.([0-9]+)\\.([0-9]+).*$")
   set(libucxx_version "${CMAKE_MATCH_1}.${CMAKE_MATCH_2}.${CMAKE_MATCH_3}")
 else()
   string(REPLACE "\n" "\n  " _version_contents_formatted "${_version_contents}")
-  message(FATAL_ERROR "Could not determine ucxx version. Contents of VERSION file:\n  ${_version_contents_formatted}")
+  message(
+    FATAL_ERROR
+      "Could not determine ucxx version. Contents of VERSION file:\n  ${_version_contents_formatted}"
+  )
 endif()
 
 project(
@@ -56,7 +59,9 @@ set(UCXX_BUILD_TESTS ${BUILD_TESTS})
 set(UCXX_BUILD_BENCHMARKS ${BUILD_BENCHMARKS})
 set(UCXX_BUILD_EXAMPLES ${BUILD_EXAMPLES})
 
-set(UCXX_CXX_FLAGS -Wall -Wattributes -Werror -Wextra -Wsign-conversion -Wno-missing-field-initializers)
+set(UCXX_CXX_FLAGS -Wall -Wattributes -Werror -Wextra -Wsign-conversion
+                   -Wno-missing-field-initializers
+)
 set(UCXX_CXX_DEFINITIONS "")
 
 # Set RMM logging level
@@ -75,25 +80,18 @@ rapids_cmake_support_conda_env(conda_env MODIFY_PREFIX_PATH)
 
 # ##################################################################################################
 # * compiler options ------------------------------------------------------------------------------
-# Due to https://github.com/openucx/ucx/issues/9614, we cannot export the ucx
-# dependency because users would then have no control over whether ucx is found
-# multiple times, causing potential configure errors. Therefore, we use a raw
-# find_package call instead of rapids_find_package and skip exporting the ucx
-# dependency. Consumers of ucxx must find ucx themselves. Once we move the
-# minimum version to UCX 1.16 we can remove the above find_package in favor of
-# the commented out lines below. For the same reason, we must also gate this
-# find_package call behind a check for the target already existing so that
-# consumers can use tools like CPM.cmake to either find or build ucxx from
-# source if it cannot be found (i.e. both cases must allow prior finding of
-# ucx).
+#   Due to https://github.com/openucx/ucx/issues/9614, we cannot export the ucx dependency because
+#   users would then have no control over whether ucx is found multiple times, causing potential
+#   configure errors. Therefore, we use a raw find_package call instead of rapids_find_package and
+#   skip exporting the ucx dependency. Consumers of ucxx must find ucx themselves. Once we move the
+#   minimum version to UCX 1.16 we can remove the above find_package in favor of the commented out
+#   lines below. For the same reason, we must also gate this find_package call behind a check for
+#   the target already existing so that consumers can use tools like CPM.cmake to either find or
+#   build ucxx from source if it cannot be found (i.e. both cases must allow prior finding of ucx).
 if(NOT TARGET ucx::ucp)
   find_package(ucx REQUIRED)
 endif()
-#rapids_find_package(
-#  ucx REQUIRED
-#  BUILD_EXPORT_SET ucxx-exports
-#  INSTALL_EXPORT_SET ucxx-exports
-#)
+# rapids_find_package( ucx REQUIRED BUILD_EXPORT_SET ucxx-exports INSTALL_EXPORT_SET ucxx-exports )
 
 # ##################################################################################################
 # * dependencies ----------------------------------------------------------------------------------
@@ -169,9 +167,7 @@ set_target_properties(
              INTERFACE_POSITION_INDEPENDENT_CODE ON
 )
 
-target_compile_options(
-  ucxx PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_FLAGS}>"
-)
+target_compile_options(ucxx PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_FLAGS}>")
 
 # Specify include paths for the current target and dependents
 target_include_directories(
@@ -181,24 +177,21 @@ target_include_directories(
   INTERFACE "$<INSTALL_INTERFACE:include>"
 )
 
-target_compile_definitions(
-  ucxx PUBLIC "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_DEFINITIONS}>"
-)
+target_compile_definitions(ucxx PUBLIC "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_DEFINITIONS}>")
 
 # Enable RMM if necessary
 if(UCXX_ENABLE_RMM)
-    target_link_libraries(ucxx
-      PUBLIC rmm::rmm
-    )
+  target_link_libraries(ucxx PUBLIC rmm::rmm)
 
-    # Define spdlog level
-    target_compile_definitions(ucxx PUBLIC UCXX_ENABLE_RMM "SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${RMM_LOGGING_LEVEL}")
+  # Define spdlog level
+  target_compile_definitions(
+    ucxx PUBLIC UCXX_ENABLE_RMM "SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${RMM_LOGGING_LEVEL}"
+  )
 endif()
 
 # Specify the target module library dependencies
 target_link_libraries(ucxx PUBLIC ucx::ucp)
 
-
 # Add Conda library, and include paths if specified
 if(TARGET conda_env)
   target_link_libraries(ucxx PRIVATE conda_env)
@@ -251,9 +244,7 @@ install(
   EXPORT ucxx-exports
 )
 
-install(DIRECTORY ${UCXX_SOURCE_DIR}/include/ucxx
-                  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
-)
+install(DIRECTORY ${UCXX_SOURCE_DIR}/include/ucxx DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
 set(doc_string
     [=[

diff --git a/cpp/benchmarks/CMakeLists.txt b/cpp/benchmarks/CMakeLists.txt
@@ -28,14 +28,11 @@ function(ConfigureBench CMAKE_BENCH_NAME)
                CXX_STANDARD 17
                CXX_STANDARD_REQUIRED ON
   )
-  target_link_libraries(
-    ${CMAKE_BENCH_NAME} PRIVATE ucxx
-                                $<TARGET_NAME_IF_EXISTS:conda_env>
-  )
+  target_link_libraries(${CMAKE_BENCH_NAME} PRIVATE ucxx $<TARGET_NAME_IF_EXISTS:conda_env>)
   add_custom_command(
     OUTPUT UCXX_BENCHMARKS
     # COMMAND ${CMAKE_BENCH_NAME} --benchmark_out_format=json
-    #         --benchmark_out=results/${CMAKE_BENCH_NAME}.json
+    # --benchmark_out=results/${CMAKE_BENCH_NAME}.json
     COMMAND ${CMAKE_BENCH_NAME}
     APPEND
     COMMENT "Adding ${CMAKE_BENCH_NAME}"

diff --git a/cpp/cmake/config.json b/cpp/cmake/config.json
@@ -0,0 +1,39 @@
+{
+    "parse": {
+        "additional_commands": {
+            "CPMFindPackage": {
+                "kwargs": {
+                    "NAME": 1,
+                    "GITHUB_REPOSITORY": "?",
+                    "GIT_TAG": "?",
+                    "VERSION": "?",
+                    "GIT_SHALLOW": "?",
+                    "OPTIONS": "*",
+                    "FIND_PACKAGE_ARGUMENTS": "*"
+                }
+            }
+        }
+    },
+    "format": {
+        "line_width": 100,
+        "tab_size": 2,
+        "command_case": "unchanged",
+        "max_lines_hwrap": 1,
+        "max_pargs_hwrap": 999,
+        "dangle_parens": true
+    },
+    "lint": {
+        "disabled_codes": [
+            "C0111", "C0113", "C0301"
+        ],
+        "function_pattern": "[0-9A-z_]+",
+        "macro_pattern": "[0-9A-z_]+",
+        "global_var_pattern": "[A-z][0-9A-z_]+",
+        "internal_var_pattern": "_[A-z][0-9A-z_]+",
+        "local_var_pattern": "[A-z][A-z0-9_]+",
+        "private_var_pattern": "_[0-9A-z_]+",
+        "public_var_pattern": "[A-z][0-9A-z_]+",
+        "argument_var_pattern": "[A-z][A-z0-9_]+",
+        "keyword_pattern": "[A-z][0-9A-z_]+"
+    }
+}
diff --git a/cpp/examples/CMakeLists.txt b/cpp/examples/CMakeLists.txt
@@ -28,10 +28,7 @@ function(ConfigureBench CMAKE_BENCH_NAME)
                CXX_STANDARD 17
                CXX_STANDARD_REQUIRED ON
   )
-  target_link_libraries(
-    ${CMAKE_BENCH_NAME} PRIVATE ucxx
-                                $<TARGET_NAME_IF_EXISTS:conda_env>
-  )
+  target_link_libraries(${CMAKE_BENCH_NAME} PRIVATE ucxx $<TARGET_NAME_IF_EXISTS:conda_env>)
   add_custom_command(
     OUTPUT UCXX_EXAMPLES
     COMMAND ${CMAKE_BENCH_NAME}

diff --git a/cpp/python/CMakeLists.txt b/cpp/python/CMakeLists.txt
@@ -16,7 +16,10 @@ if(_version_contents MATCHES "^([0-9]+)\\.([0-9]+)\\.([0-9]+).*$")
   set(libucxx_version "${CMAKE_MATCH_1}.${CMAKE_MATCH_2}.${CMAKE_MATCH_3}")
 else()
   string(REPLACE "\n" "\n  " _version_contents_formatted "${_version_contents}")
-  message(FATAL_ERROR "Could not determine ucxx version. Contents of VERSION file:\n  ${_version_contents_formatted}")
+  message(
+    FATAL_ERROR
+      "Could not determine ucxx version. Contents of VERSION file:\n  ${_version_contents_formatted}"
+  )
 endif()
 
 project(
@@ -55,36 +58,24 @@ set(UCXX_PYTHON_LIB Python3::Python)
 
 # ##################################################################################################
 # * compiler options ------------------------------------------------------------------------------
-# Due to https://github.com/openucx/ucx/issues/9614, we cannot export the ucx
-# dependency because users would then have no control over whether ucx is found
-# multiple times, causing potential configure errors. Therefore, we use a raw
-# find_package call instead of rapids_find_package and skip exporting the ucx
-# dependency. Consumers of ucxx must find ucx themselves. Once we move the
-# minimum version to UCX 1.16 we can remove the above find_package in favor of
-# the commented out lines below. For the same reason, we must also gate this
-# find_package call behind a check for the target already existing so that
-# consumers can use tools like CPM.cmake to either find or build ucxx from
-# source if it cannot be found (i.e. both cases must allow prior finding of
-# ucx).
+#   Due to https://github.com/openucx/ucx/issues/9614, we cannot export the ucx dependency because
+#   users would then have no control over whether ucx is found multiple times, causing potential
+#   configure errors. Therefore, we use a raw find_package call instead of rapids_find_package and
+#   skip exporting the ucx dependency. Consumers of ucxx must find ucx themselves. Once we move the
+#   minimum version to UCX 1.16 we can remove the above find_package in favor of the commented out
+#   lines below. For the same reason, we must also gate this find_package call behind a check for
+#   the target already existing so that consumers can use tools like CPM.cmake to either find or
+#   build ucxx from source if it cannot be found (i.e. both cases must allow prior finding of ucx).
 if(NOT TARGET ucx::ucp)
   find_package(ucx REQUIRED)
 endif()
-#rapids_find_package(
-#  ucx REQUIRED
-#  BUILD_EXPORT_SET ucxx-exports
-#  INSTALL_EXPORT_SET ucxx-exports
-#)
+# rapids_find_package( ucx REQUIRED BUILD_EXPORT_SET ucxx-exports INSTALL_EXPORT_SET ucxx-exports )
 
 # ##################################################################################################
 # * python library --------------------------------------------------------------------------------
 add_library(
-  ucxx_python
-  src/exception.cpp
-  src/future.cpp
-  src/notifier.cpp
-  src/python_future.cpp
-  src/python_future_task_collector.cpp
-  src/worker.cpp
+  ucxx_python src/exception.cpp src/future.cpp src/notifier.cpp src/python_future.cpp
+              src/python_future_task_collector.cpp src/worker.cpp
 )
 
 set_target_properties(
@@ -99,34 +90,28 @@ set_target_properties(
              INTERFACE_POSITION_INDEPENDENT_CODE ON
 )
 
-target_compile_options(
-  ucxx_python PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_FLAGS}>"
-)
+target_compile_options(ucxx_python PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_FLAGS}>")
 
 get_filename_component(ucxx_dir "${CMAKE_CURRENT_SOURCE_DIR}" DIRECTORY)
 
 # Specify include paths for the current target and dependents
 target_include_directories(
   ucxx_python
-  PUBLIC "$<BUILD_INTERFACE:${ucxx_dir}/include>"
-         "$<BUILD_INTERFACE:${ucxx_dir}/python/include>"
+  PUBLIC "$<BUILD_INTERFACE:${ucxx_dir}/include>" "$<BUILD_INTERFACE:${ucxx_dir}/python/include>"
   PRIVATE "$<BUILD_INTERFACE:${ucxx_dir}/src>"
   INTERFACE "$<INSTALL_INTERFACE:include>"
 )
 
-target_compile_definitions(
-  ucxx_python PUBLIC "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_DEFINITIONS}>"
-)
+target_compile_definitions(ucxx_python PUBLIC "$<$<COMPILE_LANGUAGE:CXX>:${UCXX_CXX_DEFINITIONS}>")
 
 target_compile_definitions(ucxx_python PUBLIC UCXX_ENABLE_PYTHON)
 # Define spdlog level
-target_compile_definitions(ucxx_python PUBLIC "SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${RMM_LOGGING_LEVEL}")
+target_compile_definitions(
+  ucxx_python PUBLIC "SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${RMM_LOGGING_LEVEL}"
+)
 
 # Specify the target module library dependencies
-target_link_libraries(
-  ucxx_python
-  PUBLIC rmm::rmm ucx::ucp ucxx::ucxx ${UCXX_PYTHON_LIB}
-)
+target_link_libraries(ucxx_python PUBLIC rmm::rmm ucx::ucp ucxx::ucxx ${UCXX_PYTHON_LIB})
 
 # Add Conda library, and include paths if specified
 if(TARGET conda_env)
@@ -135,10 +120,12 @@ endif()
 
 include(GNUInstallDirs)
 add_library(ucxx::python ALIAS ucxx_python)
-install(DIRECTORY ${ucxx_dir}/python/include/ucxx
-                  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
+install(DIRECTORY ${ucxx_dir}/python/include/ucxx DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
+install(
+  TARGETS ucxx_python
+  DESTINATION ${CMAKE_INSTALL_LIBDIR}
+  EXPORT ucxx-python-exports
 )
-install(TARGETS ucxx_python DESTINATION ${CMAKE_INSTALL_LIBDIR} EXPORT ucxx-python-exports)
 
 rapids_export(
   INSTALL ucxx-python