-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove unnecessary libcuspatial
wheel builds, enforce cmake-format and cmake-lint
#1516
base: branch-25.02
Are you sure you want to change the base?
Conversation
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
/ok to test |
PUBLIC "$<BUILD_INTERFACE:${CUPROJ_SOURCE_DIR}/include>" | ||
PUBLIC "$<BUILD_INTERFACE:${CUPROJSHIM_SOURCE_DIR}/include>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only manual changes in this PR... fixes this finding from cmake-lint
:
python/cuproj/cuproj/cuprojshim/CMakeLists.txt:140,02: [E1122] Duplicate keyword argument PUBLIC
My read of https://cmake.org/cmake/help/latest/command/target_include_directories.html is that these forms of https://cmake.org/cmake/help/latest/command/target_include_directories.html are equivalent, so this is just a style thing and I don't think it should affect behavior at all.
@@ -38,5 +46,8 @@ | |||
"public_var_pattern": "[A-z][0-9A-z_]+", | |||
"argument_var_pattern": "[A-z][A-z0-9_]+", | |||
"keyword_pattern": "[A-z][0-9A-z_]+" | |||
}, | |||
"markup": { | |||
"enable_markup": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevents cmake-format
from reformatting comments.
ref: https://cmake-format.readthedocs.io/en/latest/configuration.html
@@ -12,6 +12,14 @@ | |||
"FIND_PACKAGE_ARGUMENTS": "*" | |||
} | |||
}, | |||
"rapids_cython_create_modules": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Teaching cmake-format
the signature of this function helps it to split it up into one keyword argument per line.
libcuspatial
wheel builds, enforce cmake-format and cmake-lintlibcuspatial
wheel builds, enforce cmake-format and cmake-lint
Description
Contributes to rapidsai/build-planning#136
Proposes the following:
libcuspatial
nightly wheels once per `(CUDA version, CPU architecture) combination, to save CI resources (see avoid unnecessary wheel builds ucxx#344)wheel-build-cuspatial
depend onwheel-build-libcuspatial
in nightly CI, notwheel-publish-libcuspatial
wheel-build-cuspatial
just needslibcuspatial
wheels from S3, not nightly PyPIcmake-format
andcmake-lint
here to standardize CMake code style, similar to the way that other RAPIDS libraries doNotes for Reviewers
cmake-format
/cmake-lint
changesThe new files
cpp/scripts/run-cmake-format.sh
andcpp/cmake/config.json
were borrowed and then lightly adapted from cuDF (code link).All changes in
CMakeLists.txt
and.cmake
files here were made automatically bycmake-format
, like this:Checklist