rapidsai / rapids-cmake

https://docs.rapids.ai/api/rapids-cmake/stable/
Apache License 2.0
29 stars 46 forks source link

Fix `nvtx3` build export #615

Closed trxcllnt closed 4 months ago

trxcllnt commented 4 months ago

Description

Fixes the following error:

CMake Error at /home/coder/rmm/build/conda/cuda-12.2/latest/rmm-targets.cmake:52 (set_target_properties):
  The link interface of target "rmm::rmm" contains:
    nvtx::nvtx3-cpp
  but the target was not found.  Possible reasons include:
    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.
Call Stack (most recent call first):
  /home/coder/rmm/build/conda/cuda-12.2/latest/rmm-config.cmake:75 (include)
  build/conda/cuda-12.2/release/cmake/CPM_0.38.5.cmake:243 (find_package)
  build/conda/cuda-12.2/release/cmake/CPM_0.38.5.cmake:303 (cpm_find_package)
  build/conda/cuda-12.2/release/_deps/rapids-cmake-src/rapids-cmake/cpm/find.cmake:189 (CPMFindPackage)
  build/conda/cuda-12.2/release/_deps/rapids-cmake-src/rapids-cmake/cpm/rmm.cmake:75 (rapids_cpm_find)
  cmake/thirdparty/get_rmm.cmake:20 (rapids_cpm_rmm)
  cmake/thirdparty/get_rmm.cmake:24 (find_and_configure_rmm)
  CMakeLists.txt:192 (include)

Checklist

trxcllnt commented 4 months ago

I noticed we do this in RMM for spdlog, so maybe we should do that in rapids-cmake/cpm/spdlog.cmake instead?

robertmaynard commented 4 months ago

I noticed we do this in RMM for spdlog, so maybe we should do that in rapids-cmake/cpm/spdlog.cmake instead?

There is an open issue to track that refactoring. https://github.com/rapidsai/rapids-cmake/pull/592 started down that path as part of the update to a newer spdlog version.

robertmaynard commented 4 months ago

@trxcllnt I updated the PR to consistently use the name nvtx3 instead of NVTX3. Doing so makes the package logic consistent , and removes the need to do rapids_export_package(BUILD nvtx3 ${_RAPIDS_BUILD_EXPORT_SET} GLOBAL_TARGETS nvtx3-c nvtx3-cpp) as that is now properly done by rapids_cpm_find since it is given the corrected name.

Tested with a local rmm build tree and I see rmm the correct usage of the nvtx3 package in the rmm build tree.

robertmaynard commented 4 months ago

/merge