[FEA] Make cudftestutil a header-only package #16658
Labels
feature request
New feature or request
Comments
3 tasks
|
for cudf itself we made |
3 tasks
rapids-bot bot
pushed a commit
that referenced
this issue
Oct 14, 2024
This merge request follows up on #16658. It removes the dependency on GTest by cudftestutil. It satisfies the requirement that we only need API compatibility with the GTest API and we don't expose the GTest symbols to our consumers nor ship any binary artifact. The source files defining the symbols are late-binded to the resulting executable (via library INTERFACE sources). The user has to link to manually link the GTest and GMock libraries to the final executable as illustrated below. Closes #16658 ### Usage CMakeLists.txt: ```cmake add_executable(test1 test1.cpp) target_link_libraries(test1 PRIVATE GTest::gtest GTest::gmock GTest::gtest_main cudf::cudftestutil cudf::cudftestutil_impl) ``` Authors: - Basit Ayantunde (https://github.com/lamarrr) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Robert Maynard (https://github.com/robertmaynard) - David Wendt (https://github.com/davidwendt) - Mike Sarahan (https://github.com/msarahan) URL: #16839
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
Currently cudf publishes a cudftestutil library containing utility functions used in libcudf testing. This library was primarily intended for libcudf's own usage, but over time its usage has proliferated to many of libcudf's consumers including cuspatial and the Spark JNI plugin. Unfortunately, there are numerous issues with packaging up these utilities as a library due to the desire for consumers to be able to install this library in the same environment as different versions of gtest, or to avoid needing gtest altogether. We attempted to resolve the second issue across all of RAPIDS by statically linking gtest (see rapidsai/build-planning#32), but there were additional complexities for cudf due to the existence of cudftestutil. Upon further experimentation, we are discovering that having a working test utility and allowing users the flexibility to install different versions of gtest are incompatible requirements due to the symbol visibility issues described here. Therefore, a new solution is needed.
Describe the solution you'd like
I propose that we drop the cudftestutil compiled library altogether and convert it into a header-only library that can be used by all consumers. This would allow all necessary symbols to be compiled privately and uniquely into each test executable such that there is no concern with conflicting symbols while also allowing users to bring their own preferred version of gtest. As long as the utilities are API-compatible with the desired versions of gtest, ABI-compatibility concerns would no longer be in play, nor would symbol visibility since there would not be any DSO (cudftestutil or gtest) to speak of. Note that this assumes that test executables produced by RAPIDS libraries like also continue to statically link gtest so that the resulting test packages could be safely installed into user environments with a different version of gtest, but we have no reason to change that behavior.
Describe alternatives you've considered
We've tried a number of alternatives, such as changing cudftestutil from a static library to a shared one so that we could compile in the necessary components of a static gtest, but none of these worked. A header-only library seems like the best remaining option aside from removing the utils altogether.
The text was updated successfully, but these errors were encountered: