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

[FEA] Remove libcudf dependency on gtest / gmock #13381

Closed
bdice opened this issue May 18, 2023 · 1 comment · Fixed by #15534
Closed

[FEA] Remove libcudf dependency on gtest / gmock #13381

bdice opened this issue May 18, 2023 · 1 comment · Fixed by #15534
Assignees
Labels
feature request New feature or request

Comments

@bdice
Copy link
Contributor

bdice commented May 18, 2023

Is your feature request related to a problem? Please describe.

@ajschmidt8, myself, and others have been confused in the past by libcudf's conda dependency on gtest and gmock. This is currently needed because the cudftestutil target (which depends on these) is shipped as part of the libcudf package. We'd like to remove the dependency of libcudf on gtest.

Describe the solution you'd like

@vyasr thinks we can split cudftestutil and cudftest_default_stream into a separate conda package. Perhaps it can be named libcudf-test-utils or similar.

@mdemoret-nv
Copy link
Contributor

+1 from the Morpheus team. This is going to require changing our version of GTest/GMock due to these libraries being included in the libcudf conda package (which wasnt the case in 23.04)

rapids-bot bot pushed a commit that referenced this issue Apr 23, 2024
Reworks the cudftestutil and dependency chain to remove the public gtest dependency in libcudf conda package.
The libcudftestutil was previously made static due to issues using a static system GTest that wasn't build with `fPIC`. Using  a GTest from `rapids-cmake` which is built with `fPIC` enabled, removes this restriction and allows us to remove the public depedency.

Some notes:
-  We need to align all of RAPIDS C++ projects on static GTest from `rapids-cmake` 
- None of the compiled components / classes of `libcudftestutils` publically depend on GTest
- Two of the libcudftestutils header only components bring include gtest. Since these headers aren't required to be used we are going to consider them optional.
- Therefore using these optional `libcudftestutils` header will require downstream users to bring in GTest.

Fixes #13381

Authors:
  - Robert Maynard (https://github.com/robertmaynard)
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Nghia Truong (https://github.com/ttnghia)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Jake Awe (https://github.com/AyodeAwe)

URL: #15534
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
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants