Skip to content

Commit

Permalink
Merge bitcoin#24065: build: explicitly disable support for external s…
Browse files Browse the repository at this point in the history
…igning on Windows

e2ab9f8 build: disable external signer on Windows (fanquake)

Pull request description:

  This change explicitly disables support for external signing when targeting Windows and OpenBSD. The driver for this is that Boost Process uses boost::filesystem internally, when targeting Windows, which gets in the way of removing our usage of it (bitcoin#20744). While we could adjust bitcoin#20744 to still link against the Boost libs when building for Windows, that would be disappointing, as we wouldn't have cleanly removed the Boost usage we're trying too (including the build infrastructure), and, we'd be in a position where we would be building releases differently depending on the platform, which is something I want to avoid.

  After discussion with Sjors, Achow and Hebasto, this seemed like a reasonable step to move bitcoin#20744 forward (as-is). Note that support for external signing ([while already being experimental](https://github.com/bitcoin/bitcoin/blob/master/doc/external-signer.md#example-usage)), could be considered even more experimental on Windows. Also, oddly, we have external-signing [explicitly disabled in our Windows (cross-compile) CI](https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/ci/test/00_setup_env_win64.sh#L16), it's not clear why this is the case, as, if it's a feature being built into releases, it should be being built and tested in the CI which is most-like the release process.

  There is an [issue open upstream](boostorg/process#207), in regards to migrating Boost Process to std::filesystem, or having an option to use it. However there hasn't been much discussion since it was opened ~9 months ago. There is another related issue here: klemens-morgenstern/boost-process#164.

  Resolves bitcoin#24036.

ACKs for top commit:
  Sjors:
    utACK e2ab9f8
  achow101:
    ACK e2ab9f8
  kallewoof:
    utACK e2ab9f8
  hebasto:
    ACK e2ab9f8, tested on Linux Mint 20.2 (x86_64).

Tree-SHA512: 36fcfc0e1a008a8271dc76b8e12e93d3e1d1e528bf668e95a559e9f6fd7d5f031bd7a6a6bc8b9fa9d057b2cd56f9ec8838c7f74e87899bf9a6aeb787afbd112c
  • Loading branch information
fanquake committed Jan 20, 2022
2 parents a541e5d + e2ab9f8 commit 63fc2f5
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
2 changes: 1 addition & 1 deletion ci/test/00_setup_env_win64.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ export DPKG_ADD_ARCH="i386"
export PACKAGES="python3 nsis g++-mingw-w64-x86-64 wine-binfmt wine64 wine32 file"
export RUN_FUNCTIONAL_TESTS=false
export GOAL="deploy"
export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests --disable-external-signer"
export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests"
18 changes: 16 additions & 2 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ AC_ARG_ENABLE([werror],
AC_ARG_ENABLE([external-signer],
[AS_HELP_STRING([--enable-external-signer],[compile external signer support (default is yes, requires Boost::Process)])],
[use_external_signer=$enableval],
[use_external_signer=yes])
[use_external_signer=auto])

AC_ARG_ENABLE([lto],
[AS_HELP_STRING([--enable-lto],[build using LTO (default is no)])],
Expand Down Expand Up @@ -1415,7 +1415,21 @@ if test "$use_boost" = "yes"; then
fi

if test "$use_external_signer" != "no"; then
AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [], [Define if external signer support is enabled])
case $host in
*mingw*)
dnl Boost Process uses Boost Filesystem when targeting Windows. Also,
dnl since Boost 1.71.0, Process does not work with mingw-w64 without
dnl workarounds. See 67669ab425b52a2b6be3d2f3b3b7e3939b676a2c.
if test "$use_external_signer" = "yes"; then
AC_MSG_ERROR([External signing is not supported on Windows])
fi
use_external_signer="no";
;;
*)
use_external_signer="yes"
AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled])
;;
esac
fi
AM_CONDITIONAL([ENABLE_EXTERNAL_SIGNER], [test "$use_external_signer" = "yes"])

Expand Down
5 changes: 0 additions & 5 deletions src/util/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@
#include <util/system.h>

#ifdef ENABLE_EXTERNAL_SIGNER
#if defined(WIN32) && !defined(__kernel_entry)
// A workaround for boost 1.71 incompatibility with mingw-w64 compiler.
// For details see https://github.com/bitcoin/bitcoin/pull/22348.
#define __kernel_entry
#endif
#include <boost/process.hpp>
#endif // ENABLE_EXTERNAL_SIGNER

Expand Down

0 comments on commit 63fc2f5

Please sign in to comment.