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

cmake: Build bitcoin-qt executable #77

Merged
merged 6 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 7 additions & 4 deletions .github/workflows/cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -259,17 +259,17 @@ jobs:
packages: 'clang-14 g++-multilib'
c_compiler: 'clang-14 -m32'
cxx_compiler: 'clang++-14 -m32'
depends_options: 'NO_QT=1'
depends_options: ''
configure_options: '-DWERROR=ON'
- name: 'MinGW-w64'
triplet: 'x86_64-w64-mingw32'
packages: 'g++-mingw-w64-x86-64-posix'
depends_options: 'NO_QT=1'
depends_options: ''
exe_extension: '.exe'
- name: 'MinGW-w64, debug'
triplet: 'x86_64-w64-mingw32'
packages: 'g++-mingw-w64-x86-64-posix'
depends_options: 'NO_QT=1 DEBUG=1'
depends_options: 'DEBUG=1'
configure_options: '-DCMAKE_BUILD_TYPE=Debug'
cache_suffix: '-debug'
exe_extension: '.exe'
Expand Down Expand Up @@ -392,6 +392,9 @@ jobs:
# to avoid linker errors when using vcpkg in the manifest mode.
# See: https://github.com/bitcoin/bitcoin/pull/28934
Add-Content -Path "$env:VCPKG_ROOT\triplets\x64-windows-static.cmake" -Value "set(VCPKG_PLATFORM_TOOLSET_VERSION $env:VCToolsVersion)"
# Skip debug configuration to speed up build and minimize cache size.
Add-Content -Path "$env:VCPKG_ROOT\triplets\x64-windows.cmake" -Value "set(VCPKG_BUILD_TYPE release)"
Add-Content -Path "$env:VCPKG_ROOT\triplets\x64-windows-static.cmake" -Value "set(VCPKG_BUILD_TYPE release)"

- name: Restore vcpkg binary cache
uses: actions/cache/restore@v3
Expand Down Expand Up @@ -453,7 +456,7 @@ jobs:
env:
HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK: 1
run: |
brew install ccache cmake pkg-config boost libevent berkeley-db@4 libnatpmp miniupnpc zeromq tree
brew install ccache cmake pkg-config boost libevent berkeley-db@4 qt@5 libnatpmp miniupnpc zeromq tree
echo "CCACHE_DIR=${{ runner.temp }}/ccache" >> "$GITHUB_ENV"

- name: CMake version
Expand Down
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ else()
unset(debug_flags)
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 Expand Up @@ -342,6 +344,7 @@ message("Wallet support:")
message(" SQLite, descriptor wallets .......... ${WITH_SQLITE}")
message(" Berkeley DB, legacy wallets ......... ${WITH_BDB}")
message("Optional packages:")
message(" GUI ................................. ${WITH_GUI}")
message(" NAT-PMP ............................. ${WITH_NATPMP}")
message(" UPnP ................................ ${WITH_MINIUPNPC}")
message(" ZeroMQ .............................. ${WITH_ZMQ}")
Expand Down
46 changes: 46 additions & 0 deletions cmake/optional_qt.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

set(WITH_GUI "AUTO" CACHE STRING "Build GUI ([AUTO], Qt5, OFF)")
set(with_gui_values AUTO Qt5 OFF)
if(NOT WITH_GUI IN_LIST with_gui_values)
message(FATAL_ERROR "WITH_GUI value is \"${WITH_GUI}\", but must be one of \"AUTO\", \"Qt5\" or \"OFF\".")
endif()

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
)
endif()

if(WITH_GUI STREQUAL "AUTO")
# The PATH_SUFFIXES option is required on OpenBSD systems.
find_package(QT NAMES Qt5
COMPONENTS Core
HINTS ${qt5_brew_prefix}
PATH_SUFFIXES Qt5
)
if(QT_FOUND)
set(WITH_GUI Qt${QT_VERSION_MAJOR})
if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
enable_language(OBJCXX)
set(CMAKE_OBJCXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}")
set(CMAKE_OBJCXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}")
set(CMAKE_OBJCXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
set(CMAKE_OBJCXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL}")
endif()
else()
message(WARNING "Qt not found, disabling.\n"
"To skip this warning check, use \"-DWITH_GUI=OFF\".\n")
set(WITH_GUI OFF)
endif()
endif()
endif()
1 change: 1 addition & 0 deletions depends/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ $(host_prefix)/share/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_
-e 's|@host_arch@|$(host_arch)|' \
-e 's|@CC@|$(host_CC)|' \
-e 's|@CXX@|$(host_CXX)|' \
-e 's|@OSX_SDK@|$(OSX_SDK)|' \
-e 's|@AR@|$(host_AR)|' \
-e 's|@RANLIB@|$(host_RANLIB)|' \
-e 's|@STRIP@|$(host_STRIP)|' \
Expand Down
7 changes: 4 additions & 3 deletions depends/packages/qt.mk
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,14 @@ define $(package)_build_cmds
$(MAKE)
endef

# TODO: Investigate whether specific targets can be used here to minimize the amount of files/components installed.
define $(package)_stage_cmds
$(MAKE) -C qtbase/src INSTALL_ROOT=$($(package)_staging_dir) $(addsuffix -install_subtargets,$(addprefix sub-,$($(package)_qt_libs))) && \
$(MAKE) -C qttools/src/linguist INSTALL_ROOT=$($(package)_staging_dir) $(addsuffix -install_subtargets,$(addprefix sub-,$($(package)_linguist_tools))) && \
$(MAKE) -C qtbase INSTALL_ROOT=$($(package)_staging_dir) install && \
Copy link

Choose a reason for hiding this comment

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

Can you explain this change? It's been ages since I've messed with the qt build, but IIRC this was done to keep the installed libs to a minimum. Now we just install everything?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Otherwise, the CMake-driven configuration fails. I cannot point at the exact error as I did that change a year ago and just don't remember the exact problem.

Copy link

Choose a reason for hiding this comment

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

Ok. Sounds like maybe it just added some additional cmake targets added to our list? Otherwise I believe this means $(package)_qt_libs is now unused.

Mind adding a comment?

"TODO: Investigate whether specific targets can be used here to minimize the amount of files/components installed.".

Copy link
Owner Author

Choose a reason for hiding this comment

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

Mind adding a comment?

"TODO: Investigate whether specific targets can be used here to minimize the amount of files/components installed.".

Added.

$(MAKE) -C qttools INSTALL_ROOT=$($(package)_staging_dir) install && \
$(MAKE) -C qttranslations INSTALL_ROOT=$($(package)_staging_dir) install_subtargets
endef

define $(package)_postprocess_cmds
rm -rf native/mkspecs/ native/lib/ lib/cmake/ && \
rm -rf doc/ native/lib/ && \
hebasto marked this conversation as resolved.
Show resolved Hide resolved
rm -f lib/lib*.la
endef
4 changes: 0 additions & 4 deletions depends/patches/qt/qt.pro
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ cache(, super)

!QTDIR_build: cache(CONFIG, add, $$list(QTDIR_build))

prl = no_install_prl
CONFIG += $$prl
cache(CONFIG, add stash, prl)
hebasto marked this conversation as resolved.
Show resolved Hide resolved

TEMPLATE = subdirs
SUBDIRS = qtbase qttools qttranslations

Expand Down
10 changes: 9 additions & 1 deletion depends/toolchain.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,16 @@ set(PKG_CONFIG_PATH "@depends_prefix@/lib/pkgconfig")
set(PKG_CONFIG_LIBDIR "${PKG_CONFIG_PATH}")
set(QT_TRANSLATIONS_DIR "@depends_prefix@/translations")

if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND NOT CMAKE_HOST_APPLE)
# The find_package(Qt ...) function internally uses find_library()
# calls for all dependencies to ensure their availability.
# In turn, the find_library() inspects the well-known locations
# on the file system; therefore, a hint is required.
set(CMAKE_FRAMEWORK_PATH "@OSX_SDK@/System/Library/Frameworks")

Choose a reason for hiding this comment

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

Can't remember the exact comments from yesterday, but I think this could use some docs to explain why the SDK is leaking out of the compiler invocation.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I've added an explanation comment:

# The find_package(Qt ...) function internally uses find_library()
# calls for all dependencies to ensure their availability.
# In turn, the find_library() inspects the well-known locations
# on the file system; therefore, a hint is required.
set(CMAKE_FRAMEWORK_PATH "@OSX_SDK@/System/Library/Frameworks")

Choose a reason for hiding this comment

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

I'm still confused by this. Does this relate to the previous commit? If I unset CMAKE_FRAMEWORK_PATH and drop the previous commit, it still configures, but then fails to link bitcoin-qt.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm still confused by this.

What exactly is the source of your confusion: the code or the comment?

Choose a reason for hiding this comment

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

The code, but I guess I should just look at what cmake does here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

https://cmake.org/cmake/help/latest/command/find_library.html describes the role of the CMAKE_FRAMEWORK_PATH variable.

Copy link
Owner Author

@hebasto hebasto Jan 22, 2024

Choose a reason for hiding this comment

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

For Qt's part, please see https://github.com/qt/qtbase/blob/da6e958319e95fe564d3b30c931492dd666bfaff/mkspecs/features/data/cmake/Qt5BasicConfig.cmake.in#L77-L134

The second invocation:

find_library(_Qt5$${CMAKE_MODULE_NAME}_${Configuration}_${_lib}_PATH ${_lib} HINTS ${CMAKE_CXX_IMPLICIT_LINK_DIRECTORIES})

actually uses the CMAKE_FRAMEWORK_PATH variable.

Copy link
Owner Author

@hebasto hebasto Jan 22, 2024

Choose a reason for hiding this comment

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

Does this relate to the previous commit? If I unset CMAKE_FRAMEWORK_PATH and drop the previous commit, it still configures, but then fails to link bitcoin-qt.

No, it doesn't. The Revert "build, qt: Do not install *.prl files" commit does not actually made the "prl" files available, which is required for a proper configuration. This is done in the cmake: Build bitcoin-qt executable commit. The first commit is supposed to make further changes more meaningful and clear.

Unsetting CMAKE_FRAMEWORK_PATH would result in a cross-compiling for macOS failure regardless of the commit.

endif()

if(NOT WITH_GUI AND "@no_qt@" STREQUAL "1")
set(WITH_GUI "no" CACHE STRING "")
set(WITH_GUI OFF CACHE STRING "")
endif()

if(NOT WITH_QRENCODE AND "@no_qr@" STREQUAL "1")
Expand Down
5 changes: 5 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,11 @@ if(BUILD_UTIL)
endif()


if(WITH_GUI)
add_subdirectory(qt)
endif()


add_subdirectory(test/util)
if(BUILD_BENCH)
add_subdirectory(bench)
Expand Down
172 changes: 172 additions & 0 deletions src/qt/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

# See:
# - https://cmake.org/cmake/help/latest/manual/cmake-qt.7.html
# - https://doc.qt.io/qt-5/cmake-manual.html

set(CMAKE_AUTOMOC ON)
set(CMAKE_AUTORCC ON)
set(CMAKE_AUTOUIC ON)
set(CMAKE_AUTOUIC_SEARCH_PATHS forms)

set(qt_minimum_required_version 5.11.3)

set(qt_components Core Gui Widgets Network LinguistTools)

if(CMAKE_CROSSCOMPILING)
# The find_package(Qt ...) function internally uses find_library()
# calls for all dependencies to ensure their availability.
# In turn, the find_library() inspects the well-known locations
# on the file system; therefore, it must be able to find
# platform-specific system libraries, for example:
# /usr/x86_64-w64-mingw32/lib/libm.a or /usr/arm-linux-gnueabihf/lib/libm.a.
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY BOTH)
endif()

find_package(Qt5 ${qt_minimum_required_version} REQUIRED
COMPONENTS ${qt_components}
HINTS ${qt5_brew_prefix}
PATH_SUFFIXES Qt5 # Required on OpenBSD systems.
)
unset(qt_components)
message(STATUS "Found Qt: ${Qt5_DIR} (found suitable version \"${Qt5_VERSION}\", minimum required is \"${qt_minimum_required_version}\")")
unset(qt_minimum_required_version)

# TODO: The file(GLOB ...) command should be replaced with an explicit
# file list. Such a change must be synced with the corresponding change
# to https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py
file(GLOB ts_files RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} locale/*.ts)
Copy link

Choose a reason for hiding this comment

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

Why glob rather than listing explicitly? This introduces potential non-determinism based on the builder's environment.

Copy link
Owner Author

@hebasto hebasto Jan 24, 2024

Choose a reason for hiding this comment

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

You are right. I would consider this approach as a temporary one until the https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py script is updated. Specifically, its update_build_systems function.

Also the bitcoin_locale.qrc file could be a build artifact rather than a source tree file.

Choose a reason for hiding this comment

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

Maybe add a comment to make this clear?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Maybe add a comment to make this clear?

Added.

set_source_files_properties(${ts_files} PROPERTIES OUTPUT_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/locale)
qt5_add_translation(qm_files ${ts_files})

configure_file(bitcoin_locale.qrc bitcoin_locale.qrc COPYONLY)

add_library(bitcoinqt STATIC EXCLUDE_FROM_ALL
bantablemodel.cpp
bitcoin.cpp
bitcoinaddressvalidator.cpp
bitcoinamountfield.cpp
bitcoingui.cpp
bitcoinunits.cpp
clientmodel.cpp
csvmodelwriter.cpp
guiutil.cpp
initexecutor.cpp
intro.cpp
modaloverlay.cpp
networkstyle.cpp
notificator.cpp
optionsdialog.cpp
optionsmodel.cpp
peertablemodel.cpp
peertablesortproxy.cpp
platformstyle.cpp
qvalidatedlineedit.cpp
qvaluecombobox.cpp
rpcconsole.cpp
splashscreen.cpp
trafficgraphwidget.cpp
utilitydialog.cpp
$<$<PLATFORM_ID:Windows>:winshutdownmonitor.cpp>
$<$<PLATFORM_ID:Darwin>:macdockiconhandler.mm>
$<$<PLATFORM_ID:Darwin>:macnotificationhandler.mm>
$<$<PLATFORM_ID:Darwin>:macos_appnap.mm>
bitcoin.qrc
${CMAKE_CURRENT_BINARY_DIR}/bitcoin_locale.qrc
)
target_compile_definitions(bitcoinqt
PUBLIC
QT_NO_KEYWORDS
QT_USE_QSTRINGBUILDER
)
target_include_directories(bitcoinqt
PUBLIC
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>
)
target_link_libraries(bitcoinqt
PUBLIC
Qt5::Widgets
PRIVATE
core_interface
bitcoin_cli
leveldb
Boost::headers
$<TARGET_NAME_IF_EXISTS:NATPMP::NATPMP>
$<TARGET_NAME_IF_EXISTS:MiniUPnPc::MiniUPnPc>
$<$<PLATFORM_ID:Darwin>:-framework\ AppKit>
$<$<CXX_COMPILER_ID:MSVC>:shlwapi>
)

if(ENABLE_WALLET)
target_sources(bitcoinqt
PRIVATE
addressbookpage.cpp
addresstablemodel.cpp
askpassphrasedialog.cpp
coincontroldialog.cpp
coincontroltreewidget.cpp
createwalletdialog.cpp
editaddressdialog.cpp
openuridialog.cpp
overviewpage.cpp
paymentserver.cpp
psbtoperationsdialog.cpp
qrimagewidget.cpp
receivecoinsdialog.cpp
receiverequestdialog.cpp
recentrequeststablemodel.cpp
sendcoinsdialog.cpp
sendcoinsentry.cpp
signverifymessagedialog.cpp
transactiondesc.cpp
transactiondescdialog.cpp
transactionfilterproxy.cpp
transactionoverviewwidget.cpp
transactionrecord.cpp
transactiontablemodel.cpp
transactionview.cpp
walletcontroller.cpp
walletframe.cpp
walletmodel.cpp
walletmodeltransaction.cpp
walletview.cpp
)
target_link_libraries(bitcoinqt
PRIVATE
bitcoin_wallet
Qt5::Network
)
endif()

if(CMAKE_CROSSCOMPILING)
target_compile_definitions(bitcoinqt PRIVATE QT_STATICPLUGIN)
if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND TARGET Qt5::QXcbIntegrationPlugin)
target_compile_definitions(bitcoinqt PRIVATE QT_QPA_PLATFORM_XCB)
elseif(WIN32 AND TARGET Qt5::QWindowsIntegrationPlugin AND TARGET Qt5::QWindowsVistaStylePlugin)
target_compile_definitions(bitcoinqt PRIVATE QT_QPA_PLATFORM_WINDOWS)
elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND TARGET Qt5::QCocoaIntegrationPlugin AND TARGET Qt5::QMacStylePlugin)
target_compile_definitions(bitcoinqt PRIVATE QT_QPA_PLATFORM_COCOA)
endif()
endif()

add_executable(bitcoin-qt
main.cpp
../init/bitcoin-qt.cpp
)

target_link_libraries(bitcoin-qt
core_interface
bitcoinqt
bitcoin_node
)

if(WIN32)
set_target_properties(bitcoin-qt PROPERTIES WIN32_EXECUTABLE TRUE)
endif()

install(TARGETS bitcoin-qt
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
COMPONENT GUI
)
2 changes: 1 addition & 1 deletion src/qt/winshutdownmonitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include <QString>
#include <functional>

#include <windef.h> // for HWND
#include <windows.h>

#include <QAbstractNativeEventFilter>

Expand Down
2 changes: 2 additions & 0 deletions vcpkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"libevent",
"miniupnpc",
"sqlite3",
"qt5-base",
"qt5-tools",
"zeromq"
]
}
Loading