Always build and export the cudf::cudftestutil target #7574
Conversation
github-actions
bot
added
CMake
trxcllnt
added
non-breaking
trxcllnt
added
3 - Ready for Review
| add_library(cudftestutil STATIC | ||
| tests/utilities/base_fixture.cpp | ||
| tests/utilities/column_utilities.cu | ||
| tests/utilities/table_utilities.cu | ||
| tests/strings/utilities.cu) |
There was a problem hiding this comment.
Given enough other downstream libraries are depending on this, should we just include it in libcudf.so instead of producing a separate static library that people static link?
There was a problem hiding this comment.
I really don't think so. That would artificially increase the size of libcudf.so for people who don't care about the test utilities. e.g., cuDF Python, Spark, etc.
There was a problem hiding this comment.
We distribute this in the packaging mechanisms so it doesn't matter for cuDF Python currently anyway, but that's a fair point. Maybe we should consider making this a separate shared library along the lines of libcudf_testutil or something like that. We could then eventually package it separately, but not high priority.
|
This seems like something that shouldn't always be on, but a separate CMake config that is enabled by |
|
@jrhemstad that would mean people couldn't use |
People already complain about the size of our packages. I assume if a library is going to be using our test utilities, it's a library we're tightly integrated with and they'll likely be building from source anyways. |
|
|
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7574 +/- ##
===============================================
+ Coverage 81.86% 82.39% +0.52%
===============================================
Files 101 101
Lines 16884 17350 +466
===============================================
+ Hits 13822 14295 +473
+ Misses 3062 3055 -7
Continue to review full report at Codecov.
|
|
rerun tests |
|
@jrhemstad apparently we already ship |
…-always-export-cudftestutil
…nstall in cudf-config.cmake
|
@gpucibot merge |
This PR includes many of the same changes as were made in [cudf/pull/7107](rapidsai/cudf#7107). * Export `cuspatial::cuspatial` CMake alias targets. * Fixes -Wall errors discovered when changing compile flags. * Use `CPMFindPackage` to find `libcudf` installed on the system or build `libcudf` from source. edit: Depends on rapidsai/cudf#7574 and rapidsai/cudf#7734 Authors: - Paul Taylor (@trxcllnt) Approvers: - AJ Schmidt (@ajschmidt8) - Mark Harris (@harrism) - Keith Kraus (@kkraus14) URL: #365
We should always build the static
cudftestutiltarget regardless of whetherBUILD_TESTS=ONwas passed.Needed by #7484 and rapidsai/cuspatial#365.