Skip to content

Commit

Permalink
Remove public gtest dependency from libcudf conda package (#15534)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
robertmaynard authored Apr 23, 2024
1 parent 73306f1 commit 7341866
Show file tree
Hide file tree
Showing 13 changed files with 14 additions and 139 deletions.
3 changes: 0 additions & 3 deletions conda/environments/all_cuda-118_arch-x86_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ channels:
- nvidia
dependencies:
- aiobotocore>=2.2.0
- benchmark==1.8.0
- boto3>=1.21.21
- botocore>=1.24.21
- breathe>=4.35.0
Expand All @@ -34,8 +33,6 @@ dependencies:
- fmt>=10.1.1,<11
- fsspec>=0.6.0
- gcc_linux-64=11.*
- gmock>=1.13.0
- gtest>=1.13.0
- hypothesis
- identify>=2.5.20
- ipython
Expand Down
3 changes: 0 additions & 3 deletions conda/environments/all_cuda-122_arch-x86_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ channels:
- nvidia
dependencies:
- aiobotocore>=2.2.0
- benchmark==1.8.0
- boto3>=1.21.21
- botocore>=1.24.21
- breathe>=4.35.0
Expand All @@ -35,8 +34,6 @@ dependencies:
- fmt>=10.1.1,<11
- fsspec>=0.6.0
- gcc_linux-64=11.*
- gmock>=1.13.0
- gtest>=1.13.0
- hypothesis
- identify>=2.5.20
- ipython
Expand Down
6 changes: 0 additions & 6 deletions conda/recipes/libcudf/conda_build_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@ sysroot_version:
cmake_version:
- ">=3.26.4"

gbench_version:
- "==1.8.0"

gtest_version:
- ">=1.13.0"

libarrow_version:
- "==14.0.2"

Expand Down
11 changes: 0 additions & 11 deletions conda/recipes/libcudf/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ requirements:
- librdkafka {{ librdkafka_version }}
- fmt {{ fmt_version }}
- spdlog {{ spdlog_version }}
- benchmark {{ gbench_version }}
- gtest {{ gtest_version }}
- gmock {{ gtest_version }}
- zlib {{ zlib_version }}

outputs:
Expand Down Expand Up @@ -108,8 +105,6 @@ outputs:
- librmm ={{ minor_version }}
- libkvikio ={{ minor_version }}
- dlpack {{ dlpack_version }}
- gtest {{ gtest_version }}
- gmock {{ gtest_version }}
test:
commands:
- test -f $PREFIX/lib/libcudf.so
Expand Down Expand Up @@ -221,9 +216,6 @@ outputs:
{% else %}
- libcurand-dev
{% endif %}
- benchmark {{ gbench_version }}
- gtest {{ gtest_version }}
- gmock {{ gtest_version }}
run:
- {{ pin_compatible('cuda-version', max_pin='x', min_pin='x') }}
- {{ pin_subpackage('libcudf', exact=True) }}
Expand All @@ -233,9 +225,6 @@ outputs:
{% else %}
- libcurand
{% endif %}
- benchmark {{ gbench_version }}
- gtest {{ gtest_version }}
- gmock {{ gtest_version }}
about:
home: https://rapids.ai/
license: Apache-2.0
Expand Down
12 changes: 5 additions & 7 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -847,14 +847,12 @@ if(CUDF_BUILD_TESTUTIL)

add_library(cudf::cudftest_default_stream ALIAS cudftest_default_stream)

# Needs to be static so that we support usage of static builds of gtest which doesn't compile with
# fPIC enabled and therefore can't be embedded into shared libraries.
add_library(
cudftestutil STATIC
cudftestutil SHARED
tests/io/metadata_utilities.cpp
tests/utilities/base_fixture.cpp
tests/utilities/column_utilities.cu
tests/utilities/debug_utilities.cu
tests/utilities/random_seed.cpp
tests/utilities/table_utilities.cu
tests/utilities/tdigest_utilities.cu
)
Expand All @@ -879,8 +877,8 @@ if(CUDF_BUILD_TESTUTIL)

target_link_libraries(
cudftestutil
PUBLIC GTest::gmock GTest::gtest Threads::Threads cudf cudftest_default_stream
PRIVATE $<TARGET_NAME_IF_EXISTS:conda_env>
PUBLIC Threads::Threads cudf cudftest_default_stream
PRIVATE GTest::gmock GTest::gtest $<TARGET_NAME_IF_EXISTS:conda_env>
)

target_include_directories(
Expand Down Expand Up @@ -959,7 +957,7 @@ endif()
if(CUDF_BUILD_BENCHMARKS)
# Find or install GoogleBench
include(${rapids-cmake-dir}/cpm/gbench.cmake)
rapids_cpm_gbench()
rapids_cpm_gbench(BUILD_STATIC)

# Find or install nvbench
include(cmake/thirdparty/get_nvbench.cmake)
Expand Down
2 changes: 1 addition & 1 deletion cpp/benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ target_include_directories(

# Use an OBJECT library so we only compile these helper source files only once
add_library(
cudf_benchmark_common OBJECT "${CUDF_SOURCE_DIR}/tests/utilities/base_fixture.cpp"
cudf_benchmark_common OBJECT "${CUDF_SOURCE_DIR}/tests/utilities/random_seed.cpp"
synchronization/synchronization.cpp io/cuio_common.cpp
)
target_link_libraries(cudf_benchmark_common PRIVATE cudf_datagen $<TARGET_NAME_IF_EXISTS:conda_env>)
Expand Down
19 changes: 2 additions & 17 deletions cpp/cmake/thirdparty/get_gtest.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# =============================================================================
# Copyright (c) 2021-2023, NVIDIA CORPORATION.
# Copyright (c) 2021-2024, 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
Expand All @@ -17,22 +17,7 @@ function(find_and_configure_gtest)
include(${rapids-cmake-dir}/cpm/gtest.cmake)

# Find or install GoogleTest
rapids_cpm_gtest(BUILD_EXPORT_SET cudf-testing-exports INSTALL_EXPORT_SET cudf-testing-exports)

if(GTest_ADDED)
rapids_export(
BUILD GTest
VERSION ${GTest_VERSION}
EXPORT_SET GTestTargets
GLOBAL_TARGETS gtest gmock gtest_main gmock_main
NAMESPACE GTest::
)

include("${rapids-cmake-dir}/export/find_package_root.cmake")
rapids_export_find_package_root(
BUILD GTest [=[${CMAKE_CURRENT_LIST_DIR}]=] EXPORT_SET cudf-testing-exports
)
endif()
rapids_cpm_gtest(BUILD_STATIC)

endfunction()

Expand Down
1 change: 0 additions & 1 deletion cpp/include/cudf_test/column_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#pragma once

#include <cudf_test/column_utilities.hpp>
#include <cudf_test/cudf_gtest.hpp>
#include <cudf_test/default_stream.hpp>

#include <cudf/column/column.hpp>
Expand Down
82 changes: 1 addition & 81 deletions cpp/include/cudf_test/cudf_gtest.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2023, NVIDIA CORPORATION.
* Copyright (c) 2019-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,86 +16,6 @@

#pragma once

#ifdef GTEST_INCLUDE_GTEST_GTEST_H_
#error "Don't include gtest/gtest.h directly, include cudf_gtest.hpp instead"
#endif

/**
* @file cudf_gtest.hpp
* @brief Work around for GTests( <=v1.10 ) emulation of variadic templates in
* @verbatim ::Testing::Types @endverbatim
*
* @note Instead of including `gtest/gtest.h`, all libcudf test files should
* include `cudf_gtest.hpp` instead.
*
* Removes the 50 type limit in a type-parameterized test list.
*
* Uses macros to rename GTests's emulated variadic template types and then
* redefines them properly.
*/

// @cond
#if __has_include(<gtest/internal/gtest-type-util.h.pump>)
// gtest doesn't provide a version header so we need to
// use a file existence trick.
// gtest-type-util.h.pump only exists in versions < 1.11
#define Types Types_NOT_USED
#define Types0 Types0_NOT_USED
#define TypeList TypeList_NOT_USED
#define Templates Templates_NOT_USED
#define Templates0 Templates0_NOT_USED
#include <gtest/internal/gtest-type-util.h>
#undef Types
#undef Types0
#undef TypeList
#undef Templates
#undef Templates0

namespace testing {
template <class... TYPES>
struct Types {
using type = Types;
};

template <class T, class... TYPES>
struct Types<T, TYPES...> {
using Head = T;
using Tail = Types<TYPES...>;

using type = Types;
};

namespace internal {
using Types0 = Types<>;

template <GTEST_TEMPLATE_... TYPES>
struct Templates {};

template <GTEST_TEMPLATE_ HEAD, GTEST_TEMPLATE_... TAIL>
struct Templates<HEAD, TAIL...> {
using Head = internal::TemplateSel<HEAD>;
using Tail = Templates<TAIL...>;

using type = Templates;
};

using Templates0 = Templates<>;

template <typename T>
struct TypeList {
using type = Types<T>;
};

template <class... TYPES>
struct TypeList<Types<TYPES...>> {
using type = Types<TYPES...>;
};

} // namespace internal
} // namespace testing
#endif // gtest < 1.11
// @endcond

#include <gmock/gmock.h>
#include <gtest/gtest.h>

Expand Down
5 changes: 3 additions & 2 deletions cpp/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ function(ConfigureTest CMAKE_TEST_NAME)
)

target_link_libraries(
${CMAKE_TEST_NAME} PRIVATE cudftestutil GTest::gmock_main GTest::gtest_main nvtx3-cpp
$<TARGET_NAME_IF_EXISTS:conda_env> "${_CUDF_TEST_EXTRA_LIB}"
${CMAKE_TEST_NAME}
PRIVATE cudftestutil GTest::gmock GTest::gmock_main GTest::gtest GTest::gtest_main nvtx3-cpp
$<TARGET_NAME_IF_EXISTS:conda_env> "${_CUDF_TEST_EXTRA_LIB}"
)
rapids_cuda_set_runtime(${CMAKE_TEST_NAME} USE_STATIC ${CUDA_STATIC_RUNTIME})
rapids_test_add(
Expand Down
3 changes: 2 additions & 1 deletion cpp/tests/groupby/groupby_test_util.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023, NVIDIA CORPORATION.
* Copyright (c) 2020-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,6 +18,7 @@

#include <cudf_test/column_utilities.hpp>
#include <cudf_test/column_wrapper.hpp>
#include <cudf_test/cudf_gtest.hpp>
#include <cudf_test/table_utilities.hpp>

#include <cudf/column/column_view.hpp>
Expand Down
File renamed without changes.
6 changes: 0 additions & 6 deletions dependencies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,6 @@ dependencies:
- output_types: conda
packages:
- fmt>=10.1.1,<11
- &gbench benchmark==1.8.0
- &gtest gtest>=1.13.0
- &gmock gmock>=1.13.0
- librmm==24.6.*
- libkvikio==24.6.*
- librdkafka>=1.9.0,<1.10.0a0
Expand Down Expand Up @@ -585,9 +582,6 @@ dependencies:
- output_types: conda
packages:
- *cmake_ver
- *gbench
- *gtest
- *gmock
specific:
- output_types: conda
matrices:
Expand Down

0 comments on commit 7341866

Please sign in to comment.