Skip to content
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

Inline contents of simple cmake/thirdparty files #64

Open
harrism opened this issue May 23, 2024 · 0 comments
Open

Inline contents of simple cmake/thirdparty files #64

harrism opened this issue May 23, 2024 · 0 comments
Labels
feature request New feature or request

Comments

@harrism
Copy link
Member

harrism commented May 23, 2024

Prompted by a comment from @vyasr rapidsai/rmm#1558 (comment)

We should eliminate cmake/thirdparty/x.cmake files that have just these two repeated lines in them:

# Use CPM to find or clone CCCL
function(find_and_configure_cccl)

  include(${rapids-cmake-dir}/cpm/cccl.cmake)
  rapids_cpm_cccl(BUILD_EXPORT_SET rmm-exports INSTALL_EXPORT_SET rmm-exports)

endfunction()

find_and_configure_cccl()

As Vyas pointed out, these files are trivial and repetitive, and lead to more code to maintain than just inlining the include and rapids_cpm_x call.

I propose two things:

  1. Add a new function to rapids-cmake that allows including a list of its module includes:
rapids_cpm_include(cccl.cmake, fmt.cmake, nvtx.cmake)

This would expand to

include(${rapids-cmake-dir}/cpm/cccl.cmake)
include(${rapids-cmake-dir}/cpm/fmt.cmake)
include(${rapids-cmake-dir}/cpm/nvtx.cmake)
  1. In all RAPIDS repos, replace cmake/thirdparty/x.cmake files that only contain include and rapids_cpm_x with a single call to rapids_cpm_include and inlined rapids_cpm_x, like so:
rapids_cpm_include(cccl.cmake, fmt.cmake, nvtx.cmake)
rapids_cpm_cccl(BUILD_EXPORT_SET rmm-exports INSTALL_EXPORT_SET rmm-exports)
rapids_cpm_nvtx3(BUILD_EXPORT_SET rmm-exports INSTALL_EXPORT_SET rmm-exports)
rapids_cpm_fmt(BUILD_EXPORT_SET rmm-exports INSTALL_EXPORT_SET rmm-exports)

Note that more complex cmake/thirdparty/x.cmake files should be kept. For example: https://github.com/rapidsai/rmm/blob/branch-24.06/cmake/thirdparty/get_spdlog.cmake

@harrism harrism added the feature request New feature or request label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant