Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix failures in cpm_generate_pins-nested tests (#724)
[Recent nightly failures of the `cpm_generate_pins-nested` test](https://github.com/rapidsai/rapids-cmake/actions/runs/12133373349/attempts/1) are unexpected because neither rapids-cmake, CPM, nor CMake have changed recently. [The error](https://github.com/rapidsai/rapids-cmake/actions/runs/12133373349/job/33828828013#step:8:5066) shows that [the list of projects to verify](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/testing/cpm/verify_generated_pins/CMakeLists.txt#L28) is being completely filtered out because none of them have a `<project>_SOURCE_DIR` variable set. This led me down a rabbit hole of finding a number of different issues in both rapids-cmake itself and in tests. I'll detail them one by one below. ### Problem 1: Our logic for when to download packages for which rapids-cmake provides specialized `rapids_cpm*` commands is wrong The first piece of evidence here is the difference between recent builds of our CI images. [The most recent builds of miniforge-cuda have spdlog in the base environment](https://github.com/rapidsai/ci-imgs/actions/runs/12145567995/job/33867533318#step:8:274), while [previous builds do not](https://github.com/rapidsai/ci-imgs/actions/runs/12036072167/job/33556500014#step:8:307). That should not affect our tests [because we set `CPM_DOWNLOAD_ALL` when populating the CMake cache](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/testing/utils/fill_cache/CMakeLists.txt#L36), but it turns out that #348 introduced a bug where [the `always_download` variable gets defaulted to OFF](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/rapids-cmake/cpm/detail/package_details.cmake#L95) and because of [how it propagates to `CPM_DOWNLOAD_ALL` in parent scope](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/rapids-cmake/cpm/detail/package_details.cmake#L118) we wind up with `CPM_DOWNLOAD_ALL` always being false. As a result, we have actually been ignoring `CPM_DOWNLOAD_ALL` for any packages for which `rapids_cpm_package_details` is called (the more specific `CPM_DOWNLOAD_<project>` should still have been working since that takes precedence). Solution: We unset the `always_download` variable initially instead of setting it to off so that the check is handled correctly. ### Problem 2: Even with the above logic fixed, the actual test will use the spdlog from the environment The above fix ensures that when we populate the CPM source cache in our tests we download spdlog instead of finding it in the environment. However, when tests are subsequently run they will still prefer the package unless `CPM_DOWNLOAD_ALL` (or `CPM_DOWNLOAD_<package>`) is set because of how CPM prioritizes. Solution: To resolve this, for all rapids-cpm tests we are now setting `CPM_DOWNLOAD_<pkg>` for all of the packages that have been added to our CPM cache. This fix is safe since it effectively just avoids finding local copies of the built package; the source will always come from the cache by design. This change is sufficient for tests to pass, but on closer inspection we see that only spdlog is verified by the verification script and not rmm or fmt. That's because of the next problems. ### Problem 3: All of our tests of pinning are incorrectly checking for rmm with the wrong case [The `project` call in rmm uses the uppercase name "RMM"](https://github.com/rapidsai/rmm/blob/branch-25.02/CMakeLists.txt#L25), but [the entry in rapids-cmake's versions.json](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/rapids-cmake/cpm/versions.json#L66) uses the lowercase name. As a result, our current logic for verifying generate pins can never test rmm because the project [is initially filtered based on `<project>_SOURCE_DIR` being defined](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/testing/cpm/verify_generated_pins.cmake#L33), which will use the uppercase name from the `project` call, but when actually verifying we use `rapids_cpm_package_details` to get the versions.json data _using the same string_, which will now have the wrong case if we passed the uppercase version originally. ### Problem 4: All of our tests of pinning are incorrectly checking for fmt with the wrong case This is the same problem as the above. [fmt uses `project(FMT)`](https://github.com/fmtlib/fmt/blob/master/CMakeLists.txt#L151). Solution for 3 and 4: We now use the `CPM_PACKAGE_<project>_SOURCE_DIR` variables instead of the `<project>_SOURCE_DIR` variables. The former are guaranteed to be using the export names of the package and will therefore always be case-consistent with the values in versions.json. ### Other changes in this PR - [This gtest test](https://github.com/rapidsai/rapids-cmake/blob/branch-25.02/testing/cpm/cpm_find-gtest-no-gmock/CMakeLists.txt) is explicitly based on finding a mocked up version of the package locally, so with the changes in this PR we now have to turn of `CPM_DOWNLOAD_GTest` explicitly inside the test. - I removed scikit-build from the testing CPM cache since as of #614 we no longer support its usage via the legacy rapids-cython module. The change to `verify_generated_pins.cmake` ensures that all of the above changes are actually having the desired effect, since now rather than silently passing when only a subset of projects requesting verification are downloaded we will error. All projects must be pulled correctly from the CPM cache during the test (which implicitly checks all four problems above) in order for tests to pass.
- Loading branch information
