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.

Sign up for GitHub

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

Jump to bottom

cmake: Switch from tri-state options to boolean. Stage FOUR #169

Merged
merged 4 commits into from
May 1, 2024
Merged

cmake: Switch from tri-state options to boolean. Stage FOUR #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
017aeda
cmake [FIXUP]: Move `find_program(brew)` out from introspection file
hebasto Apr 29, 2024
820a538
cmake [KILL 3-STATE]: Switch `WITH_QRENCODE` to boolean w/ default ON
hebasto Apr 24, 2024
09b1d5b
fixup! ci: Test CMake edge cases
hebasto Apr 24, 2024
5de23c7
fixup! cmake: Add preset for MSVC build
hebasto Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/cmake.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ jobs:
env:
HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK: 1
run: |
brew install ccache cmake pkg-config boost libevent berkeley-db@4 qt@5 libnatpmp miniupnpc zeromq tree
brew install ccache cmake pkg-config boost libevent berkeley-db@4 qt@5 qrencode libnatpmp miniupnpc zeromq tree
echo "CCACHE_DIR=${{ runner.temp }}/ccache" >> "$GITHUB_ENV"

- name: CMake version
Expand Down
13 changes: 10 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,16 @@ tristate_option(WITH_USDT
)
cmake_dependent_option(WITH_EXTERNAL_SIGNER "Enable external signer support." ON "NOT WIN32" OFF)
set(ENABLE_EXTERNAL_SIGNER ${WITH_EXTERNAL_SIGNER})
tristate_option(WITH_QRENCODE "Enable QR code support." "if libqrencode is found." AUTO)

include(cmake/optional_qt.cmake)
cmake_dependent_option(WITH_QRENCODE "Enable QR code support." ON "WITH_GUI" OFF)
if(WITH_QRENCODE)
find_package(PkgConfig REQUIRED)
pkg_check_modules(libqrencode REQUIRED IMPORTED_TARGET libqrencode)
Copy link

Choose a reason for hiding this comment

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

Only tangentially related to this PR, but the error printed by pkg_check_modules is kind of ugly for a user facing message:

CMake Error at /usr/share/cmake-3.28/Modules/FindPkgConfig.cmake:619 (message):
  The following required packages were not found:

   - libqrencode

Call Stack (most recent call first):
  /usr/share/cmake-3.28/Modules/FindPkgConfig.cmake:841 (_pkg_check_modules_internal)
  CMakeLists.txt:123 (pkg_check_modules)

Would be nice to have something along the lines of what configure would print in a similar case:

configure: error: QR support requested but cannot be built. Use --without-qrencode

target_compile_definitions(PkgConfig::libqrencode INTERFACE
USE_QRCODE
)
endif()

option(MULTIPROCESS "Build multiprocess bitcoin-node, bitcoin-wallet, and bitcoin-gui executables in addition to monolithic bitcoind and bitcoin-qt executables. Requires libmultiprocess library. Experimental." OFF)
if(MULTIPROCESS)
Expand Down Expand Up @@ -440,8 +449,6 @@ else()
unset(c_flags_debug_overridden)
endif()

include(cmake/optional_qt.cmake)

include(cmake/optional.cmake)

# Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review.
Expand Down
4 changes: 4 additions & 0 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
"toolchainFile": "$env{VCPKG_ROOT}\\scripts\\buildsystems\\vcpkg.cmake",
"cacheVariables": {
"VCPKG_TARGET_TRIPLET": "x64-windows",
"WITH_GUI": "Qt5",
"WITH_QRENCODE": "OFF",
"WITH_NATPMP": "OFF"
}
},
Expand All @@ -73,6 +75,8 @@
"toolchainFile": "$env{VCPKG_ROOT}\\scripts\\buildsystems\\vcpkg.cmake",
"cacheVariables": {
"VCPKG_TARGET_TRIPLET": "x64-windows-static",
"WITH_GUI": "Qt5",
"WITH_QRENCODE": "OFF",
"WITH_NATPMP": "OFF"
}
}
Expand Down
4 changes: 0 additions & 4 deletions cmake/introspection.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,3 @@ check_cxx_source_compiles("
int main(){}
" HAVE_DLLEXPORT_ATTRIBUTE
)

if(CMAKE_HOST_APPLE)
find_program(BREW_COMMAND brew)
endif()
26 changes: 15 additions & 11 deletions cmake/module/FindBerkeleyDB.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,21 @@ This module defines the following variables:

#]=======================================================================]

if(BREW_COMMAND)
# The Homebrew package manager installs the berkeley-db* packages as
# "keg-only", which means they are not symlinked into the default prefix.
# To find such a package, the find_path() and find_library() commands
# need additional path hints that are computed by Homebrew itself.
execute_process(
COMMAND ${BREW_COMMAND} --prefix berkeley-db@4
OUTPUT_VARIABLE _BerkeleyDB_homebrew_prefix
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE
)
set(_BerkeleyDB_homebrew_prefix)
if(CMAKE_HOST_APPLE)
find_program(HOMEBREW_EXECUTABLE brew)
if(HOMEBREW_EXECUTABLE)
# The Homebrew package manager installs the berkeley-db* packages as
# "keg-only", which means they are not symlinked into the default prefix.
# To find such a package, the find_path() and find_library() commands
# need additional path hints that are computed by Homebrew itself.
execute_process(
COMMAND ${HOMEBREW_EXECUTABLE} --prefix berkeley-db@4
OUTPUT_VARIABLE _BerkeleyDB_homebrew_prefix
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE
)
endif()
endif()

find_path(BerkeleyDB_INCLUDE_DIR
Expand Down
21 changes: 0 additions & 21 deletions cmake/optional.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,3 @@ else()
set(WITH_SQLITE OFF)
set(WITH_BDB OFF)
endif()

if(WITH_GUI AND WITH_QRENCODE)
if(VCPKG_TARGET_TRIPLET)
# TODO: vcpkg fails to build libqrencode package due to
# the build error in its libiconv dependency.
# See: https://github.com/microsoft/vcpkg/issues/36924.
else()
find_package(PkgConfig REQUIRED)
pkg_check_modules(libqrencode IMPORTED_TARGET libqrencode)
endif()
if(TARGET PkgConfig::libqrencode)
set_target_properties(PkgConfig::libqrencode PROPERTIES
INTERFACE_COMPILE_DEFINITIONS USE_QRCODE
)
set(WITH_QRENCODE ON)
elseif(WITH_QRENCODE STREQUAL "AUTO")
set(WITH_QRENCODE OFF)
else()
message(FATAL_ERROR "libqrencode requested, but not found.")
endif()
endif()
18 changes: 11 additions & 7 deletions cmake/optional_qt.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ if(WITH_GUI)
set(QT_NO_CREATE_VERSIONLESS_FUNCTIONS ON)
set(QT_NO_CREATE_VERSIONLESS_TARGETS ON)

if(BREW_COMMAND)
execute_process(
COMMAND ${BREW_COMMAND} --prefix qt@5
OUTPUT_VARIABLE qt5_brew_prefix
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE
)
set(qt5_brew_prefix)
if(CMAKE_HOST_APPLE)
find_program(HOMEBREW_EXECUTABLE brew)
if(HOMEBREW_EXECUTABLE)
execute_process(
COMMAND ${HOMEBREW_EXECUTABLE} --prefix qt@5
OUTPUT_VARIABLE qt5_brew_prefix
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE
)
endif()
endif()

if(WITH_GUI STREQUAL "AUTO")
Expand Down
Loading