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

Make sure rmm::rmm CMake target is visibile to cudf users #7524

Conversation

robertmaynard
Copy link
Contributor

Presume that a project is using cudf via CPM like the following, and the machine doesn't have cudf installed, but does have rmm.

CPMAddPackage(NAME  cudf
        VERSION         "0.19.0"
        GIT_REPOSITORY  https://github.com/rapidsai/cudf.git
        GIT_TAG         branch-0.19
        GIT_SHALLOW     TRUE
        SOURCE_SUBDIR   cpp
        OPTIONS         "BUILD_TESTS OFF"
                        "BUILD_BENCHMARKS OFF"
                        "ARROW_STATIC_LIB ON"
                        "JITIFY_USE_CACHE ON"
                        "CUDA_STATIC_RUNTIME ON"
                        "DISABLE_DEPRECATION_WARNING ON"
                        "AUTO_DETECT_CUDA_ARCHITECTURES ON"
    )

add_library(cudf_example cudf_example.cu)
target_link_libraries(cudf_example PRIVATE cudf::cudf)

add_library(rmm_example rmm_example.cu)
target_link_libraries(rmm_example PRIVATE rmm::rmm)

While CPM will fail to find cudf, it will find the local install of rmm and use it. This poses a problem as CMake import targets have different default visibility compared to 'real' targets. This means that while cudf::cudf can see and resolve rmm::rmm the rmm_example executable won't be able to.

This change makes it possible for users of cudf via CPM to directly access the rmm::rmm target

@robertmaynard robertmaynard requested a review from a team as a code owner March 5, 2021 22:15
@github-actions github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Mar 5, 2021
@kkraus14 kkraus14 added non-breaking Non-breaking change bug Something isn't working labels Mar 6, 2021
@robertmaynard robertmaynard force-pushed the make_sure_rmm_target_is_global branch from d347970 to b8e5b55 Compare March 8, 2021 22:46
@kkraus14 kkraus14 added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Mar 8, 2021
@robertmaynard robertmaynard force-pushed the make_sure_rmm_target_is_global branch from b8e5b55 to 0ede297 Compare March 9, 2021 16:02
@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 9, 2021

11:15:55 CMake Error at cmake/thirdparty/CUDF_GetRMM.cmake:54 (get_target_property):
11:15:55   get_target_property called with incorrect number of arguments
11:15:55 Call Stack (most recent call first):
11:15:55   cmake/thirdparty/CUDF_GetRMM.cmake:67 (find_and_configure_rmm)
11:15:55   CMakeLists.txt:130 (include)

@robertmaynard robertmaynard force-pushed the make_sure_rmm_target_is_global branch from 0ede297 to 2f392ca Compare March 9, 2021 17:18
@robertmaynard robertmaynard force-pushed the make_sure_rmm_target_is_global branch from 2f392ca to f01e50a Compare March 9, 2021 17:36
@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 9, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit ca21f5f into rapidsai:branch-0.19 Mar 9, 2021
@robertmaynard robertmaynard deleted the make_sure_rmm_target_is_global branch March 10, 2021 14:32
hyperbolic2346 pushed a commit to hyperbolic2346/cudf that referenced this pull request Mar 25, 2021
)

Presume that a project is using `cudf` via CPM like the following, and the machine doesn't have cudf installed, but does have rmm.
```
CPMAddPackage(NAME  cudf
        VERSION         "0.19.0"
        GIT_REPOSITORY  https://github.com/rapidsai/cudf.git
        GIT_TAG         branch-0.19
        GIT_SHALLOW     TRUE
        SOURCE_SUBDIR   cpp
        OPTIONS         "BUILD_TESTS OFF"
                        "BUILD_BENCHMARKS OFF"
                        "ARROW_STATIC_LIB ON"
                        "JITIFY_USE_CACHE ON"
                        "CUDA_STATIC_RUNTIME ON"
                        "DISABLE_DEPRECATION_WARNING ON"
                        "AUTO_DETECT_CUDA_ARCHITECTURES ON"
    )

add_library(cudf_example cudf_example.cu)
target_link_libraries(cudf_example PRIVATE cudf::cudf)

add_library(rmm_example rmm_example.cu)
target_link_libraries(rmm_example PRIVATE rmm::rmm)
```

While CPM will fail to find `cudf`, it will find the local install of `rmm` and use it. This poses a problem as CMake import targets have different default visibility compared to 'real' targets. This means that while `cudf::cudf` can see and resolve `rmm::rmm` the `rmm_example` executable won't be able to.

This change makes it possible for users of cudf via CPM to directly access the `rmm::rmm` target

Authors:
  - Robert Maynard (@robertmaynard)

Approvers:
  - Keith Kraus (@kkraus14)

URL: rapidsai#7524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants