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: Add external signer support #79

Merged
merged 2 commits into from
Mar 7, 2024
Merged

cmake: Add external signer support #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fd39c9
cmake: Add external signer support
hebasto Feb 24, 2024
617ef5b
fixup! ci: Test CMake edge cases
hebasto Jan 29, 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
19 changes: 15 additions & 4 deletions .github/workflows/cmake.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,20 @@ jobs:


ubuntu-focal-native:
name: 'Ubuntu 20.04, CMake 3.16, downloaded Boost'
name: 'Ubuntu 20.04, CMake 3.16, Boost ${{ matrix.conf.boost_version }}'
runs-on: ubuntu-20.04

strategy:
fail-fast: false
matrix:
conf:
- boost_version: '1.73.0'
boost_archive: 'boost_1_73_0'
- boost_version: '1.78.0'
boost_archive: 'boost_1_78_0'
- boost_version: '1.84.0'
boost_archive: 'boost_1_84_0'

steps:
- name: Checkout
uses: actions/checkout@v4
Expand All @@ -203,8 +214,8 @@ jobs:

- name: Download Boost
run: |
curl --location --remote-name https://boostorg.jfrog.io/artifactory/main/release/1.84.0/source/boost_1_84_0.tar.gz
tar -xf boost_1_84_0.tar.gz
curl --location --remote-name https://boostorg.jfrog.io/artifactory/main/release/${{ matrix.conf.boost_version }}/source/${{ matrix.conf.boost_archive }}.tar.gz
tar -xf ${{ matrix.conf.boost_archive }}.tar.gz

- name: Restore Ccache cache
uses: actions/cache/restore@v4
Expand All @@ -216,7 +227,7 @@ jobs:

- name: Generate build system
run: |
cmake -B build -DCMAKE_C_COMPILER=gcc-10 -DCMAKE_CXX_COMPILER=g++-10 -DBoost_INCLUDE_DIR="${PWD}/boost_1_84_0" -DENABLE_WALLET=ON -DWITH_MINIUPNPC=ON -DWITH_NATPMP=ON -DWITH_ZMQ=ON -DWITH_USDT=ON -DWERROR=ON
cmake -B build -DCMAKE_C_COMPILER=gcc-10 -DCMAKE_CXX_COMPILER=g++-10 -DBoost_INCLUDE_DIR="${PWD}/${{ matrix.conf.boost_archive }}" -DENABLE_WALLET=ON -DWITH_EXTERNAL_SIGNER=ON -DWITH_MINIUPNPC=ON -DWITH_NATPMP=ON -DWITH_ZMQ=ON -DWITH_USDT=ON -DWERROR=ON

- name: Build
working-directory: build
Expand Down
6 changes: 6 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ tristate_option(WITH_USDT
"if sys/sdt.h is found."
AUTO
)
tristate_option(WITH_EXTERNAL_SIGNER
"Enable external signer support."
"if Boost.Process is found."
AUTO
)

option(BUILD_TESTS "Build test_bitcoin executable." ON)
option(BUILD_BENCH "Build bench_bitcoin executable." ON)
Expand Down Expand Up @@ -413,6 +418,7 @@ message(" SQLite, descriptor wallets .......... ${WITH_SQLITE}")
message(" Berkeley DB, legacy wallets ......... ${WITH_BDB}")
message("Optional packages:")
message(" GUI ................................. ${WITH_GUI}")
message(" external signer ..................... ${WITH_EXTERNAL_SIGNER}")
message(" NAT-PMP ............................. ${WITH_NATPMP}")
message(" UPnP ................................ ${WITH_MINIUPNPC}")
message(" ZeroMQ .............................. ${WITH_ZMQ}")
Expand Down
3 changes: 3 additions & 0 deletions cmake/bitcoin-config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,7 @@
/* Define if Berkeley DB (BDB) support should be compiled in. */
#cmakedefine USE_BDB

/* Define if external signer support is enabled. */
#cmakedefine ENABLE_EXTERNAL_SIGNER

#endif //BITCOIN_CONFIG_H
52 changes: 51 additions & 1 deletion cmake/module/AddBoostIfNeeded.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ function(add_boost_if_needed)

set(Boost_NO_BOOST_CMAKE ON)
find_package(Boost 1.73.0 REQUIRED)
mark_as_advanced(Boost_INCLUDE_DIR)
set_target_properties(Boost::headers PROPERTIES IMPORTED_GLOBAL TRUE)
target_compile_definitions(Boost::headers INTERFACE
# We don't use multi_index serialization.
Expand Down Expand Up @@ -54,5 +55,54 @@ function(add_boost_if_needed)
endif()
endif()

mark_as_advanced(Boost_INCLUDE_DIR)
if(WITH_EXTERNAL_SIGNER)
include(CheckCXXSourceCompiles)
set(CMAKE_REQUIRED_INCLUDES ${Boost_INCLUDE_DIR})
# Boost 1.78 requires the following workaround.
# See: https://github.com/boostorg/process/issues/235
set(CMAKE_REQUIRED_FLAGS "-Wno-error=narrowing")
# Boost 1.73 requires the following workaround on systems with
# libc<2.34 only, as newer libc has integrated the functionality
# of the libpthread library.
set(CMAKE_REQUIRED_LIBRARIES Threads::Threads)
check_cxx_source_compiles("
#define BOOST_PROCESS_USE_STD_FS
#include <boost/process.hpp>
#include <string>

int main()
{
namespace bp = boost::process;
bp::opstream stdin_stream;
bp::ipstream stdout_stream;
bp::child c(\"dummy\", bp::std_out > stdout_stream, bp::std_err > stdout_stream, bp::std_in < stdin_stream);
stdin_stream << std::string{\"test\"} << std::endl;
if (c.running()) c.terminate();
c.wait();
c.exit_code();
}
" HAVE_BOOST_PROCESS
)
if(HAVE_BOOST_PROCESS)
if(WIN32)
Copy link

Choose a reason for hiding this comment

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

NIT: if it's WIN32 couldn't you just return early with external signer OFF and not bother checking if boost supports boost process? Would cleanup one nested if.

Copy link
Owner Author

@hebasto hebasto Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You are right. However, I've chosen this clumsy approach to mimic the Autotools' one for the sake of easier reviewing,

Anyway, this code will be gone after bitcoin#28981.

# Boost Process for Windows uses Boost ASIO. Boost ASIO performs
# pre-main init of Windows networking libraries, which we do not
# want.
set(WITH_EXTERNAL_SIGNER OFF PARENT_SCOPE)
set(ENABLE_EXTERNAL_SIGNER OFF PARENT_SCOPE)
else()
set(WITH_EXTERNAL_SIGNER ON PARENT_SCOPE)
set(ENABLE_EXTERNAL_SIGNER ON PARENT_SCOPE)
target_compile_definitions(Boost::headers INTERFACE
BOOST_PROCESS_USE_STD_FS
)
endif()
elseif(WITH_EXTERNAL_SIGNER STREQUAL "AUTO")
set(WITH_EXTERNAL_SIGNER OFF PARENT_SCOPE)
set(ENABLE_EXTERNAL_SIGNER OFF PARENT_SCOPE)
else()
message(FATAL_ERROR "External signing is not supported for this Boost version.")
endif()
endif()

endfunction()
Loading