-
Notifications
You must be signed in to change notification settings - Fork 911
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
Link libcudf_kafka against cuDF CMake export targets (CPM) #7484
Closed
Closed
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
48edee7
REL v0.13.0 release
GPUtester 54ceb3a
enable cudf_kafka to use CPMFindPackage
jdye64 9e7f3f9
fixed copy/paste errors
jdye64 25468f6
enable cudf_kafka to use CPMFindPackage
jdye64 beaf86e
update libcudf_kafka version with each release
jdye64 464bcdd
change version to 0.19 from 0.19.0 which leads to a branch that does …
jdye64 c3df905
checkpoint commit, does it build without tests in ci?
jdye64 089855c
updated to include ConfigureCUDA like cudf
jdye64 c8ffeaa
update GPU Eval script location
jdye64 8fb7ae5
Add Set/Get GPU eval cmake files
jdye64 be8ea10
Merge remote-tracking branch 'upstream/branch-0.19' into link_against…
jdye64 2a84c8f
Merge remote-tracking branch 'upstream/branch-0.19' into link_against…
jdye64 52a138b
isort modified file
jdye64 f1c2682
use cuDF branch that exports cudftestutil, build tests
trxcllnt a52f735
updated to include the headers for libcudf
jdye64 1e0d1f6
oops, link against actual cudf_kafka target
jdye64 9d235fb
set target for includes
jdye64 4c5b318
remove option to build arrow static as the property is inheritied fro…
jdye64 263ea8e
Removed cmake get gtest as it is no longer needed
jdye64 904253d
added back arrow static option as it was still needed
jdye64 f40aabe
add cmake option for using arrow static or not
jdye64 ed3148d
add cmake option for using arrow static or not
jdye64 7368830
move cudf repo from test repo back to rapidsai repo
jdye64 73a68a3
default to cudf_use_arrow_static being OFF
jdye64 b97138d
remove reference to cudf static arrow as it is no longer needed
jdye64 ddfe13b
remove unused cpm options, and remove commented out librdkafka cpm code
jdye64 0da5d04
use parent cmake thirdparty/Modules where needed
jdye64 985f3ea
Add CUDF_SOURCE_DIR so that parent cudf cmake Modules can resolve the…
jdye64 3c75483
create RDKAFKA target
jdye64 5656c4f
Remove cmake BUILD_TESTS and ARROW_STATIC arguments
jdye64 74a98d6
couple of build fixes
jdye64 bb60e88
small build updates
jdye64 21a0502
updates from review
jdye64 bc3880f
remove custom test target from build.sh file
jdye64 59855da
return if rmm::rmm target already exists
jdye64 db4704c
syntax issue
jdye64 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
12 changes: 0 additions & 12 deletions
12
cpp/libcudf_kafka/cmake/Templates/GoogleTest.CMakeLists.txt.cmake
This file was deleted.
Oops, something went wrong.
52 changes: 52 additions & 0 deletions
52
cpp/libcudf_kafka/cmake/thirdparty/CUDF_KAFKA_GetCUDF.cmake
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| #============================================================================= | ||
| # Copyright (c) 2021, NVIDIA CORPORATION. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| #============================================================================= | ||
|
|
||
| function(cudfkafka_save_if_enabled var) | ||
| if(CUDF_KAFKA_${var}) | ||
| unset(${var} PARENT_SCOPE) | ||
| unset(${var} CACHE) | ||
| endif() | ||
| endfunction() | ||
|
|
||
| function(cudfkafka_restore_if_enabled var) | ||
| if(CUDF_KAFKA_${var}) | ||
| set(${var} ON CACHE INTERNAL "" FORCE) | ||
| endif() | ||
| endfunction() | ||
|
|
||
| function(find_and_configure_cudf VERSION) | ||
kkraus14 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| cudfkafka_save_if_enabled(BUILD_TESTS) | ||
| cudfkafka_save_if_enabled(BUILD_BENCHMARKS) | ||
| CPMFindPackage(NAME cudf | ||
| VERSION ${VERSION} | ||
| GIT_REPOSITORY https://github.com/rapidsai/cudf.git | ||
| GIT_TAG branch-${VERSION} | ||
| GIT_SHALLOW TRUE | ||
| SOURCE_SUBDIR cpp | ||
| OPTIONS "BUILD_TESTS OFF" | ||
| "BUILD_BENCHMARKS OFF") | ||
| cudfkafka_restore_if_enabled(BUILD_TESTS) | ||
| cudfkafka_restore_if_enabled(BUILD_BENCHMARKS) | ||
|
|
||
| if(NOT cudf_BINARY_DIR IN_LIST CMAKE_PREFIX_PATH) | ||
| list(APPEND CMAKE_PREFIX_PATH "${cudf_BINARY_DIR}") | ||
| set(CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH} PARENT_SCOPE) | ||
| endif() | ||
|
|
||
| endfunction() | ||
|
|
||
| set(CUDF_KAFKA_MIN_VERSION_cudf 0.19) | ||
| find_and_configure_cudf(${CUDF_KAFKA_MIN_VERSION_cudf}) | ||
25 changes: 25 additions & 0 deletions
25
cpp/libcudf_kafka/cmake/thirdparty/CUDF_KAFKA_GetRDKafka.cmake
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #============================================================================= | ||
| # Copyright (c) 2021, NVIDIA CORPORATION. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| #============================================================================= | ||
|
|
||
| find_path(RDKAFKA_INCLUDE "librdkafka" HINTS "$ENV{RDKAFKA_ROOT}/include") | ||
jdye64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| find_library(RDKAFKA++_LIBRARY "rdkafka++" HINTS "$ENV{RDKAFKA_ROOT}/lib" "$ENV{RDKAFKA_ROOT}/build") | ||
|
|
||
| if(RDKAFKA_INCLUDE AND RDKAFKA++_LIBRARY) | ||
| add_library(rdkafka INTERFACE) | ||
| target_link_libraries(rdkafka INTERFACE "${RDKAFKA++_LIBRARY}") | ||
| target_include_directories(rdkafka INTERFACE "${RDKAFKA_INCLUDE}") | ||
| add_library(RDKAFKA::RDKAFKA ALIAS rdkafka) | ||
| endif() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think this is causing problems. There's no device code / actual cuda configuration needed for libcudf, and we're doing this before importing cudf / rmm / etc. which causes the
CUDAlanguage to be enabled which breaks things I think?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.
In my dummy CMakeLists.txt for #7658,
CUDAhad to be enabled for thefind_package(cudf)to succeed.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.
Ok, that would make sense actually since I removed CUDA in a previous commit. Both @kkraus14 and I were both under the impression it was not needed since libcudf_kafka doesn't use CUDA explicitly. Are you saying I need to add it back?
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.
My thought is that if someone needs to enable
CUDAbefore doingfind_package(cudf), we have something broken in our cudf cmake config.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.
It looks like
cuftestutilandcudfboth have some CUDA information in the export information. That would require downstream consumers to enable CUDA.cuftestutilis tricky as it is a static library with CUDA sources, so CMake needs to export that when linking downstream it has CUDA dependencies ( so you need to enable the language )cudfis leaking via compile flags, would could be fixedThere 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.
Both of these should be resolved by having
cudf-config.cmakehave (enable_language(CUDA)) in the short term while we sort out the above two issues.