Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

cmake: relax package compatibility constraints #1298

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

germasch
Copy link
Contributor

This relaxes the compatibility constraints to what cmake calls "SameMajorVersion", ie., the version found must be >= than the
version requested, and major has to match exactly.

This essentially uses the same code as cmake's write_basic_package_version_file().

Fixes #1293.

This relaxes the compatibility constraints to what cmake calls
"SameMajorVersion", ie., the version found must be >= than the
version requested, and major has to match exactly.

This essentially uses the same code as cmake's
`write_basic_package_version_file()`.
Copy link
Collaborator

@alliepiper alliepiper left a comment

Choose a reason for hiding this comment

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

Let's leave FALSE initialization of the UNSUITABLE and EXACT flags in. I've learned not to make assumptions about the initial state of variables used to communicate with CMake's internals, and prefer being explicit in these cases.

Otherwise this looks good to me. Thanks for taking the time to fix this up!

thrust/cmake/thrust-config-version.cmake Show resolved Hide resolved
thrust/cmake/thrust-config-version.cmake Show resolved Hide resolved
@alliepiper alliepiper added the only: cmake CMake changes only. Doesn't need internal NVIDIA CI. label Sep 28, 2020
@alliepiper alliepiper added this to the 1.11.0 milestone Sep 28, 2020
@germasch
Copy link
Contributor Author

I can put the initialization back in, I agree that it makes it more self-contained. I actually got curious, because the documentation does not really say anything about a default for those variables, but the cmake-provided standard config-version creator assumed them to be false. So I looked at the cmake source code. It does explicitly unset those variables (which makes them evaluate to false), so it is well defined the way it is, but again, it's clearer if we set them explicitly.

@germasch
Copy link
Contributor Author

germasch commented Oct 1, 2020

FWIW, not sure that was clear, I did put the initialization to FALSE back in.

@alliepiper
Copy link
Collaborator

LGTM, I'll merge this soon.

@alliepiper alliepiper self-assigned this Oct 1, 2020
@alliepiper alliepiper added the blocked Cannot make progress. label Oct 5, 2020
@alliepiper
Copy link
Collaborator

Will merge once I update CUB to match.

@alliepiper alliepiper merged commit 8e12c92 into NVIDIA:main Oct 15, 2020
alliepiper added a commit to alliepiper/cub that referenced this pull request Oct 15, 2020
Specifically, this ports the following PRs to CUB:

- NVIDIA/thrust#1297 Add install rule option for add_subdirectory users.
- NVIDIA/thrust#1298 Enforce semantic versioning in CMake version configs.
- NVIDIA/thrust#1300 Use FindPackageHandleStandardArgs in CMake config.
alliepiper added a commit to NVIDIA/cub that referenced this pull request Oct 19, 2020
Specifically, this ports the following PRs to CUB:

- NVIDIA/thrust#1297 Add install rule option for add_subdirectory users.
- NVIDIA/thrust#1298 Enforce semantic versioning in CMake version configs.
- NVIDIA/thrust#1300 Use FindPackageHandleStandardArgs in CMake config.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked Cannot make progress. only: cmake CMake changes only. Doesn't need internal NVIDIA CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reflect semantic version in find_package version checks
2 participants