Skip to content

Fix CMake minimum required version #69

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

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Aug 26, 2022

The "C++17" value of the CXX_STANDARD target property, which was introduced in #25, is available in CMake 3.8 and newer.

Tested with old CMake versions available here.

The "C++17" value of the `CXX_STANDARD` target property, which was
introduced in bitcoin-core#25, is available in
CMake 3.8 and newer.
@@ -2,7 +2,7 @@
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

cmake_minimum_required(VERSION 3.0)
cmake_minimum_required(VERSION 3.8)
Copy link
Member Author

Choose a reason for hiding this comment

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

Considering https://repology.org/project/cmake/versions, maybe 3.10 is also justified?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Considering https://repology.org/project/cmake/versions, maybe 3.10 is also justified?

I'm not actually sure what specifically here requires 3.10, but would welcome any followup PR that changes the minimum version or adds a comment explaining the current minimum.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not actually sure what specifically here requires 3.10

Nothing requires 3.10. Just thinking about availability of CMake 3.8 and 3.9 on real systems.

Copy link
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK 6902bfd. Thanks for the PR!

@@ -2,7 +2,7 @@
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

cmake_minimum_required(VERSION 3.0)
cmake_minimum_required(VERSION 3.8)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Considering https://repology.org/project/cmake/versions, maybe 3.10 is also justified?

I'm not actually sure what specifically here requires 3.10, but would welcome any followup PR that changes the minimum version or adds a comment explaining the current minimum.

@ryanofsky ryanofsky merged commit c6849de into bitcoin-core:master Aug 29, 2022
@hebasto hebasto deleted the 220826-minver branch August 29, 2022 22:37
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants