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.

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

Write a version file for the CMake package #1165

Merged
merged 2 commits into from
Feb 24, 2023
Merged

Write a version file for the CMake package #1165

merged 2 commits into from
Feb 24, 2023

Conversation

Neverlord
Copy link
Contributor

This enables users to ask for a specific version of hiredis via find_package.

Setting COMPATIBILITY SameMajorVersion means that CMake considers newer versions as compatible as long as the major version matches, e.g., asking for 1.1 will succeed when finding 1.2 but not when finding 2.0. I hope this is the desired behavior. :)

CMake docs: https://cmake.org/cmake/help/v3.0/module/CMakePackageConfigHelpers.html#module:CMakePackageConfigHelpers.

@chayim
Copy link
Contributor

chayim commented Feb 23, 2023

Looks clean to me :D.

CMakeLists.txt Outdated
@@ -154,6 +154,9 @@ else()
endif()
SET(INCLUDE_INSTALL_DIR include)
include(CMakePackageConfigHelpers)
write_basic_package_version_file("${CMAKE_CURRENT_BINARY_DIR}/hiredis-config-version.cmake"
VERSION ${VERSION}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can even skip this line :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, good catch.

> If no VERSION is given, the PROJECT_VERSION variable is used.

Since we set the project version to `${VERSION}`, we can safely skip
passing it to `write_basic_package_version_file` as well.
@michael-grunder michael-grunder merged commit 1cbd5bc into redis:master Feb 24, 2023
@Neverlord Neverlord deleted the cmake-version-file branch February 24, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants