Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rapids_cpm_package_override: Always load overrides #778

Open
wants to merge 5 commits into
base: branch-25.04
Choose a base branch
from
Open

rapids_cpm_package_override: Always load overrides #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
17d7388
rapids_cpm_package_override: Always load overrides
robertmaynard Feb 13, 2025
7ddb821
Correctly query for the state of override_ignored
robertmaynard Feb 26, 2025
4c8901c
Test the value from rapids-cmake properties set instead they are just…
robertmaynard Feb 27, 2025
327e355
Correct range of versions in docs
robertmaynard Feb 27, 2025
67cb565
Correct range of versions in docs
robertmaynard Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions rapids-cmake/cpm/detail/package_details.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2021-2024, NVIDIA CORPORATION.
# Copyright (c) 2021-2025, 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 Down Expand Up @@ -95,10 +95,17 @@ function(rapids_cpm_package_details package_name version_var url_var tag_var sha
# Ensure that always_download is not set by default so that the if(DEFINED always_download) check
# below works as expected in the default case.
unset(always_download)
unset(override_ignored)
if(override_json_data AND json_data AND git_details_overridden)
# `always_download` default value requires the package to exist in both the default and override
# and that the git url / git tag have been modified.
set(always_download ON)
# and that the git url / git tag have been modified. We also need to make sure that when using
# an override that it isn't disabled due to `CPM_<pkg>_SOURCE`
string(TOLOWER "${package_name}" normalized_pkg_name)
get_property(override_ignored GLOBAL
PROPERTY rapids_cpm_${normalized_pkg_name}_override_ignored)
if(NOT (override_ignored OR DEFINED CPM_${package_name}_SOURCE))
set(always_download ON)
endif()
endif()
rapids_cpm_json_get_value(always_download)

Expand Down
27 changes: 22 additions & 5 deletions rapids-cmake/cpm/package_override.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2021-2024, NVIDIA CORPORATION.
# Copyright (c) 2021-2025, 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 Down Expand Up @@ -56,10 +56,14 @@ as the override.

.. note::

.. versionadded:: v23.10.00
.. versionadded:: v24.04.00

When the variable `CPM_<package_name>_SOURCE` exists, any override entries
for `package_name` will be ignored.
for `package_name` will be parsed but ignored.

For versions between v23.10 and v25.02 ( inclusive both sides ) the variable
`CPM_<package_name>_SOURCE` will cause any override entries for `package_name`
to be ignored and not parsed.


.. note::
Expand Down Expand Up @@ -91,7 +95,8 @@ function(rapids_cpm_package_override _rapids_override_filepath)
string(TOLOWER "${package_name}" normalized_pkg_name)
get_property(override_exists GLOBAL PROPERTY rapids_cpm_${normalized_pkg_name}_override_json
SET)
if(override_exists OR DEFINED CPM_${package_name}_SOURCE)
if(override_exists)
# Early terminate if this project already has an override
continue()
endif()

Expand All @@ -107,12 +112,24 @@ function(rapids_cpm_package_override _rapids_override_filepath)
set(package_proper_name ${package_name}) # Required for FetchContent_Declare
endif()

# only add the first override for a project we encounter
# Always load overrides into our internal properties even when CPM_${package_name}_SOURCE is
# set. We add the override_ignored so that `package_details` can maintain behavior around
# `CPM_DOWNLOAD_ALL` when an override is loaded but not used
#
# We are using this behavior so that advanced users can retrieve the contents of an override
# even when not in use
string(JSON data GET "${json_data}" packages "${package_name}")
set_property(GLOBAL PROPERTY rapids_cpm_${normalized_pkg_name}_override_json "${data}")
set_property(GLOBAL PROPERTY rapids_cpm_${normalized_pkg_name}_override_json_file
"${_rapids_override_filepath}")

if(DEFINED CPM_${package_name}_SOURCE)
set_property(GLOBAL PROPERTY rapids_cpm_${normalized_pkg_name}_override_ignored "ON")
continue()
else()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: no need for else() due to continue()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want the rapids_cpm_${normalized_pkg_name}_override_ignored to exist after any parsing has occured.
This gives us a ternary to determine that parsing has occured plus the outcome.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I mean that

if(cond)
  continue()
else()
  foo()
endif()

is identical to

if(cond)
  continue()
endif()
foo()

set_property(GLOBAL PROPERTY rapids_cpm_${normalized_pkg_name}_override_ignored "OFF")
endif()

# establish the fetch content
include(FetchContent)
include("${rapids-cmake-dir}/cpm/detail/package_details.cmake")
Expand Down
20 changes: 15 additions & 5 deletions testing/cpm/cpm_package_override-obey-cpm-source-var.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2021-2024, NVIDIA CORPORATION.
# Copyright (c) 2021-2025, 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 Down Expand Up @@ -47,10 +47,10 @@ rapids_cpm_package_override(${CMAKE_CURRENT_BINARY_DIR}/override.json)
include("${rapids-cmake-dir}/cpm/detail/package_details.cmake")

rapids_cpm_package_details(rmm version repository tag shallow exclude)
if(repository MATCHES "new_rmm_url")
if(NOT repository MATCHES "new_rmm_url")
message(FATAL_ERROR "custom url field should not be set, due to CPM_rmm_SOURCE")
endif()
if(shallow MATCHES "OFF")
if(NOT shallow MATCHES "OFF")
message(FATAL_ERROR "shallow field should not be set, due to CPM_rmm_SOURCE")
endif()
if(CPM_DOWNLOAD_ALL)
Expand All @@ -61,6 +61,16 @@ unset(version)
unset(repository)
unset(tag)
rapids_cpm_package_details(not_in_base version repository tag shallow exclude)
if(version OR repository OR tag)
message(FATAL_ERROR "rapids_cpm_package_details should not return anything for package that doesn't exist")
if(NOT (version AND repository AND tag))
message(FATAL_ERROR "rapids_cpm_package_details should still have details for package that doesn't exist")
endif()

get_property(override_ignored GLOBAL PROPERTY rapids_cpm_rmm_override_ignored)
if(NOT override_ignored)
message(FATAL_ERROR "rapids_cpm_package override for `not_in_base` isn't being ignored")
endif()

get_property(override_ignored GLOBAL PROPERTY rapids_cpm_not_in_base_override_ignored)
if(NOT override_ignored)
message(FATAL_ERROR "rapids_cpm_package override for `not_in_base` isn't being ignored")
endif()