[RELEASE] rapids-cmake v24.12 #720
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Forward-merge branch-24.10 into branch-24.12
Forward-merge branch-24.10 into branch-24.12
) We work around `CTestTesfile.cmake` not being idepotent when `gtest_discover_tests` is being used in the project. This allows rapids-cmake to parse the `CTestTestfile.cmake` without having the gtest infrastructure error out. Fixes #690 Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #697
This PR bumps the cuco version to the latest to fetch the Hyperloglog renaming change. Authors: - Yunsong Wang (https://github.com/PointKernel) Approvers: - Robert Maynard (https://github.com/robertmaynard) URL: #698
Forward-merge branch-24.10 into branch-24.12
When a patch fails, rapids-cmake will log the failure under $CMAKE_BUILD_DIR/rapids-cmake/<the-dep>/log and continue. Now with this change rapids-cmake will collect all patches with the `required` field set to `true` that fail and throw an error. Fixes [#699](#699) Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #700
This fixes a failure in docs builds that comes from a warning. See rapidsai/rmm#1705 and rapidsai/rmm#1706 for more information. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #705
Support the latest RC of CCCL in rapids-cmake. This requires a reworking of our install logic as upstream has refactored things. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Bradley Dice (https://github.com/bdice) URL: #706
This PR bumps the cuco version to fetch the latest `multiset::retrieve` APIs. Authors: - Yunsong Wang (https://github.com/PointKernel) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #707
This PR updates the nvcomp version pinning to 4.1.0.6. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Robert Maynard (https://github.com/robertmaynard) URL: #709
Previous logic had the CCCL rules updated for 2.7 but that was due to incorrect information. Updated tests to confirm the install logic against 2.7 and 2.8 Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Bradley Dice (https://github.com/bdice) URL: #711
It is a common mistake to presume that all entries in a package override json should use lower case names, since the vast majority of default packages are all lower case. This causes silent 'failures' when the override doesn't work. We now normalize all package data internally so that overrides for `ABC`, `abc`, and `Abc` all map to the same package. In addition we have added information tracking the proper name of bundled packages. This allows us to warn when an override uses the non correct package name, and treat it like the proper name. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #708
#708 broke patches specified via overrides due to the missing case normalization. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Robert Maynard (https://github.com/robertmaynard) URL: #715
…712) The `rapids_export_write_language` function would generate CMake code that would try and do `set(var PARENT_SCOPE)` in the root directory. This would cause CMake developer warnings to appear. This resolves the issues by adding root level guards around the `set` commands, and setting up each rapids-cmake test to have developer warnings as errors. Due to #680 we mark some tests with `NO_DEV_ERRORS` so that they continue to pass while we wait to 2025 to bump the minimum required CMake version. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Bradley Dice (https://github.com/bdice) URL: #712
…e_override (#714) The override file provided on the command line is now just the first override file always used. Therefore other calls to `rapids_cpm_package_override` with no intersection will still work. Fixes #713 Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #714
…age names (#716) Follow PR for #715 that adds tests to make sure this kind of issue doesn't show up again. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #716
jameslamb
approved these changes
Nov 25, 2024
[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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
❄️ Code freeze for
branch-24.12and v24.12 releaseWhat does this mean?
Only critical/hotfix level issues should be merged into
branch-24.12until release (merging of this PR).What is the purpose of this PR?
branch-24.12intomainfor the release