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 required cmake to 3.12 #164

Merged
merged 1 commit into from
Feb 24, 2025
Merged

Bump minimum required cmake to 3.12 #164

merged 1 commit into from
Feb 24, 2025

Conversation

maflcko
Copy link
Contributor

@maflcko maflcko commented Feb 22, 2025

This project requires C++20. However, according to https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html, the value 20 was only added in cmake 3.12.

Thus, reflect the reality also in the cmake required minimum version.

As a side-effect this also avoids a deprecation message when compiling with cmake 3.31, according to https://cmake.org/cmake/help/latest/release/3.31.html#deprecated-and-removed-features

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 729ff16.

The minimum required CMake version was last bumped to support C++17 in #69.

Copy link

@hodlinator hodlinator left a comment

Choose a reason for hiding this comment

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

ACK 729ff16

Verified to remove the CMake deprecation warning when running cmake -B build with CMake 3.31.5.

@ryanofsky ryanofsky merged commit 1954f7f into master Feb 24, 2025
@maflcko maflcko deleted the maflcko-patch-1 branch February 24, 2025 12:07
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Feb 24, 2025
Bump libmultiprocess library to include bugfix
bitcoin-core/libmultiprocess#159 which should fix
intermittent CI failure reported
bitcoin#31921

This change is bumping the libmultiprocess version instead of cherry picking
the bugfix. It could cherry-pick the bugfix instead, but there are reasons to
prefer just bumping the version:

- Bugfix might interact with earlier PRs, and the latest version is better
  tested with testing done in many CI configurations in bitcoin#30975 and bitcoin#31802

- Even though we are in feature freeze for a release, the MULTIPROCESS=1 option
  is currently not enabled for release, so this PR only affect CI builds and
  local builds, not the release build.

bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version
bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary
bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order
bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers
bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups
bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning
bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level
bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings
bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings
bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g
bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls
bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format
bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done
bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Feb 24, 2025
Bump libmultiprocess library to include bugfix
bitcoin-core/libmultiprocess#159 which should fix
intermittent CI failures reported in
bitcoin#31921

This change is bumping the libmultiprocess version instead of cherry picking
the bugfix. It could cherry-pick the bugfix instead, but there are reasons to
prefer bumping the version:

- Bugfix might interact with earlier PRs, and the latest version is better
  tested with testing done in many CI configurations in bitcoin#30975 and bitcoin#31802

- Even though we are in feature freeze for a release, the MULTIPROCESS=1 option
  is currently not enabled for release, so this PR only affect CI builds and
  local builds, not the release build.

This update brings in the following changes:

bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version
bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary
bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order
bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers
bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups
bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning
bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level
bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings
bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings
bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g
bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls
bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format
bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done
bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Feb 24, 2025
Bump libmultiprocess library to include bugfix
bitcoin-core/libmultiprocess#159 which should fix
intermittent CI failures reported in
bitcoin#31921

This change is bumping the libmultiprocess version instead of cherry picking
the bugfix. It could cherry-pick the bugfix instead, but there are reasons to
prefer bumping the version:

- Bugfix might interact with earlier PRs, and the latest version is better
  tested with testing done in many CI configurations in bitcoin#30975 and bitcoin#31802

- Even though we are in feature freeze for a release, the MULTIPROCESS=1 option
  is currently not enabled for release, so this PR only affect CI builds and
  local builds, not the release build.

This update brings in the following changes:

bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version
bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary
bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order
bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers
bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups
bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning
bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level
bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings
bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings
bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g
bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls
bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format
bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done
bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
fanquake pushed a commit to bitcoin/bitcoin that referenced this pull request Feb 25, 2025
Bump libmultiprocess library to include bugfix
bitcoin-core/libmultiprocess#159 which should fix
intermittent CI failures reported in
#31921

This change is bumping the libmultiprocess version instead of cherry picking
the bugfix. It could cherry-pick the bugfix instead, but there are reasons to
prefer bumping the version:

- Bugfix might interact with earlier PRs, and the latest version is better
  tested with testing done in many CI configurations in #30975 and #31802

- Even though we are in feature freeze for a release, the MULTIPROCESS=1 option
  is currently not enabled for release, so this PR only affect CI builds and
  local builds, not the release build.

This update brings in the following changes:

bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version
bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary
bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order
bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers
bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups
bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning
bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level
bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings
bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings
bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g
bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls
bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format
bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done
bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
fanquake added a commit to bitcoin/bitcoin that referenced this pull request Feb 25, 2025
01f7715 depends: Update libmultiprocess library to fix CI failure (Ryan Ofsky)

Pull request description:

  Bump libmultiprocess library to include bugfix bitcoin-core/libmultiprocess#159 which should fix intermittent CI failures reported in #31921

  This change is bumping the libmultiprocess version instead of cherry picking the bugfix. It could cherry-pick the bugfix instead, but there are reasons to prefer bumping the version:

  - Bugfix might interact with earlier PRs, and the latest version is better tested with testing done in many CI configurations in [#30975](#30975) and [#31802](#31802)

  - Even though we are in feature freeze for a release, the MULTIPROCESS=1 option is currently not enabled for release, so this PR only affect CI builds and local builds, not the release build.

  This update brings in the following changes:

  bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version
  bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary
  bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order
  bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers
  bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups
  bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning
  bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level
  bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings
  bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings
  bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g
  bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls
  bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format
  bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done
  bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
  bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12

  ---

  This PR is part of the [process separation project](#28722).

ACKs for top commit:
  fanquake:
    ACK 01f7715

Tree-SHA512: a6a795e4d4e13e9d35c9346f3c83d5da817f1452bdc4a9412aeadc4c652ad6e5047f4c77756570594a70ec9095cc786772a0469e306dc19bb5a508fd515c37f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants