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

Bump minimum cmake version to 3.5 to avoid deprecation warning #1097

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

dundargoc
Copy link

@dundargoc dundargoc commented Oct 16, 2023

Bump minimum cmake version to 3.5 to avoid deprecation warning

Building msgpack gives currently gives the following warning:
"CMake Deprecation Warning at CMakeLists.txt:1 (CMAKE_MINIMUM_REQUIRED):
Compatibility with CMake < 3.5 will be removed from a future version of
CMake.

Update the VERSION argument value or use a ... suffix to tell
CMake that the project does not need compatibility with older versions."

Reuse same solution as fc18087 as the
policy CMP0060 introduced in cmake 3.3 causes problems with when linking
for 32-bit otherwise.

@dundargoc dundargoc marked this pull request as draft October 17, 2023 09:11
@dundargoc dundargoc force-pushed the cmake-version branch 4 times, most recently from d289a35 to df5726b Compare October 17, 2023 09:54
@dundargoc dundargoc marked this pull request as ready for review October 17, 2023 09:54
@dundargoc dundargoc marked this pull request as draft October 17, 2023 09:55
@dundargoc dundargoc force-pushed the cmake-version branch 4 times, most recently from bcdc582 to 4e3ae6d Compare October 17, 2023 12:01
@dundargoc dundargoc marked this pull request as ready for review October 17, 2023 12:03
@redboltz
Copy link
Contributor

Thank you for sending the PR.
The current cmake minimum required version is 2.8.12 because CentOS7's cmake package version is 2.8.12.
According to https://www.centos.org/centos-linux/ , CentOS 7 support will finish on June 30th, 2024.
After CentOS 7 will finish, then msgpack-c can update cmake minimum requred version.
The updated version would be the minimum version that is contains by major distribution.

So if you want to merge the PR before June 30th, 2024, you need to support 2.8.12. Something conditional branch in CMakeLists.txt is required.

@dundargoc
Copy link
Author

Aren't there two versions of cmake in centos? I think one is cmake and one is cmake3, which should be up to date.

@redboltz
Copy link
Contributor

Aren't there two versions of cmake in centos? I think one is cmake and one is cmake3, which should be up to date.

I could't find cmake3 package in CentOS7.
https://centos.pkgs.org/7/centos-x86_64/
Am I missing something?

Building msgpack gives currently gives the following warning:
"CMake Deprecation Warning at CMakeLists.txt:1 (CMAKE_MINIMUM_REQUIRED):
  Compatibility with CMake < 3.5 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions."

Reuse same solution as fc18087 as the
policy CMP0060 introduced in cmake 3.3 causes problems with when linking
for 32-bit otherwise.
@dundargoc
Copy link
Author

Huh, I think I mixed it up with the cmake3 from the epel repo. My bad.

I've added a conditional. Please take a look again.

@codecov-commenter
Copy link

Codecov Report

Merging #1097 (8957d6e) into c_master (3a41b24) will not change coverage.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##           c_master    #1097   +/-   ##
=========================================
  Coverage     55.45%   55.45%           
=========================================
  Files             8        8           
  Lines          1044     1044           
=========================================
  Hits            579      579           
  Misses          465      465           

@redboltz
Copy link
Contributor

redboltz commented Nov 9, 2023

Thank you for updating! Looks good to me.

@redboltz redboltz merged commit 37744bd into msgpack:c_master Nov 9, 2023
19 checks passed
@dundargoc dundargoc deleted the cmake-version branch November 9, 2023 10:30
@redboltz redboltz added the C label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants