[REVIEW] cmake: prefer locally installed thirdparty packages

[germasch]

This PR essentially just changes CPMAddPackage to CPMFindPackage, except for thrust , which is currently still handled in a separate fashion (DOWNLOAD_ONLY). This makes CMake search for existing locally installed packages (at proper compatible versions) first, and if those don't exist, it falls back to fetching the packages, ie., to current behavior.

In theory, this shouldn't break anyone's workflow, since it falls back to current behavior. It does reduce the amount of duplicated libraries and potential resulting conflicts, and also should shorten build times when pre-installed local libraries can be used. In practice, I wouldn't be surprised to find some hiccups, though.

This is a step towards my goal to make RMM more easily packagable in a cmake-compatible fashion -- when using a package manager, RMM should use, e.g., the spdlog provided by the package manager, not pull in its own. This also applies to integration with conda -- currently the conda environment prescribes spdlog=1.7.0, but RMM will download and use its own (which is also 1.7.0).

Nevertheless, this PR isn't the only way to support locally installed packages -- one can also stick with the current default by keeping CPMAddPackage and setting a CMake variable to prefer locally installed packages when desired (e.g., when packaging).

[GPUtester]

Can one of the admins verify this patch?

[GPUtester]

Can one of the admins verify this patch?

[kkraus14]

ok to test

[germasch]

In the CI, it picked up spdlog and gtest, so success so far ;)

-- CPM: using local package spdlog@1.7.0
-- CPM: adding package thrust@1.10.0 (1.10.0)
-- CPM: using local package GTest@1.10.0

[kkraus14]

Is the benchmark::benchmark target consistent whether the package is found or added?

[germasch]

Yes, both spdlog and benchmark provide namespaced targets when used via add_subdirectory. gtest does not, that's why I added the aliases myself.

But I guess it brings up a good point, that is, the CI will not catch if things don't work with downloaded packages. You could add more CI checks that run with CPM_DOWNLOAD_ALL=ON, that would catch errors on that end.

[kkraus14]

@harrism can you take a look as well before we merge?

[harrism]

@germasch after this PR and after building multiple times, I see

-- CPM: using local package spdlog@1.7.0
-- CPM: adding package thrust@1.10.0 (1.10.0)
-- CPM: using local package GTest@1.10.0
-- CPM: adding package benchmark@1.5.2 (v1.5.2)

Why does it only find local packages for spdlog and gtest?

[kkraus14]

Why does it only find local packages for spdlog and gtest?

It will only say using local package if it found the package outside of the CPM cache, i.e. via a conda package.

[germasch]

For benchmark, I suppose because it's not installed in the environment. I can add it to the conda.yml files to see whether that's enough (I haven't looked how the CI sets up its environment).

For thrust, I'm still working on how to best do handle an external thrust install in the first place, so it's not even trying yet.

[kkraus14]

Benchmarks don't look like they're currently being built in CI so it's not trying to grab benchmark at all.

@harrism was that from your local dev in rapids-compose? What does conda list | grep benchmark give you?

[harrism]

benchmark doesn't need to be in conda. I am surprised gtest is there. I just thought these would get cached somewhere after the first time they are fetched. Thought that was an advantage of CPM.

[harrism]

(rapids) rapids@compose:~/rmm/build/release$ conda list | grep benchmark
benchmark                 1.5.1                he1b5a44_2    conda-forge
pytest-benchmark          3.2.3              pyh9f0ad1d_0    conda-forge
rapids-pytest-benchmark   0.0.12                     py_0    rapidsai

[kkraus14]

They are cached, it will just only say using local package if it's found outside of CPM handling it: https://github.com/TheLartians/CPM.cmake/blob/master/cmake/CPM.cmake#L420-L431

It will say adding package ... before calling into FetchContent_Declare which is what handles caching the added package.

[kkraus14]

Looks like our conda package has the wrong version of benchmark as compared to CPM which is why CPM isn't finding it locally.

[germasch]

Well, so for one, we ask for 1.5.2. (Which should exist in conda). The other potential issue is that there's at least one bug in what benchmark itself thinks its version is: google/benchmark#1045. There may be something else off -- could you look where benchmark is installed, .../lib/cmake/benchmark/benchmarkConfigVersion.cmake what it says its version is?

[harrism]

They are cached,

Thrust takes significant time, so it doesn't seem cached...

[harrism]

.../lib/cmake/benchmark/benchmarkConfigVersion.cmake

Not sure where to find that path.

[germasch]

If you set CPM_SOURCE_CACHE to some local directory, it'll cache the repos there.

[harrism]

Is that a cmake variable or an env variable?

[germasch]

.../lib/cmake/benchmark/benchmarkConfigVersion.cmake

Not sure where to find that path.

Wherever conda installs its packages.

[harrism]

We should document this.

[germasch]

Is that a cmake variable or an env variable?

Either works.

[harrism]

(rapids) rapids@compose:~$ more compose/etc/conda/envs/rapids/lib/cmake/benchmark/benchmarkConfigVersion.cmake 
# This is a basic version file for the Config-mode of find_package().
# It is used by write_basic_package_version_file() as input file for configure_file()
# to create a version-file which can be installed along a config.cmake file.
#
# The created file sets PACKAGE_VERSION_EXACT if the current version string and
# the requested version string are exactly the same and it sets
# PACKAGE_VERSION_COMPATIBLE if the current version is >= requested version,
# but only if the requested major version is the same as the current one.
# The variable CVF_VERSION must be set before calling configure_file().


set(PACKAGE_VERSION "0.0.0")

if(PACKAGE_VERSION VERSION_LESS PACKAGE_FIND_VERSION)
  set(PACKAGE_VERSION_COMPATIBLE FALSE)
else()

  if("0.0.0" MATCHES "^([0-9]+)\\.")
    set(CVF_VERSION_MAJOR "${CMAKE_MATCH_1}")
  else()
    set(CVF_VERSION_MAJOR "0.0.0")
  endif()

  if(PACKAGE_FIND_VERSION_MAJOR STREQUAL CVF_VERSION_MAJOR)
    set(PACKAGE_VERSION_COMPATIBLE TRUE)
  else()
    set(PACKAGE_VERSION_COMPATIBLE FALSE)
  endif()

  if(PACKAGE_FIND_VERSION STREQUAL PACKAGE_VERSION)
      set(PACKAGE_VERSION_EXACT TRUE)
  endif()
endif()


# if the installed project requested no architecture check, don't perform the check
if("FALSE")
  return()
endif()

# if the installed or the using project don't have CMAKE_SIZEOF_VOID_P set, ignore it:
if("${CMAKE_SIZEOF_VOID_P}" STREQUAL "" OR "8" STREQUAL "")
  return()
endif()

# check that the installed version has the same 32/64bit-ness as the one which is currently searching
:
if(NOT CMAKE_SIZEOF_VOID_P STREQUAL "8")
  math(EXPR installedBits "8 * 8")
  set(PACKAGE_VERSION "${PACKAGE_VERSION} (${installedBits}bit)")
  set(PACKAGE_VERSION_UNSUITABLE TRUE)
endif()

[harrism]

0.0.0 apparently. :)

[germasch]

0.0.0 apparently. :)

Yeah, that's what I was afraid of -- that's what happens in benchmark installed by my package manager. I'll work on getting that fixed in upstream benchmark, but that may take a little while. You can install your own benchmark locally (and add it to CMAKE_PREFIX_PATH if its not in a standard location), then RMM will pick it up (for me it does, anyway).

[germasch]

FWIW, google benchmark releases will hopefully provide their actual version in their cmake config once this gets resolved: google/benchmark#1047