From a67a9b6ae6823a98b96f08d592ee49b2c623931f Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 14 Feb 2022 20:20:21 +0000 Subject: [PATCH 01/13] Merge bitcoin/bitcoin#24254: build: Add Boost.Process usage check abc057c6030b2a0ddab46835a7801054da677781 build: Add Boost.Process usage check (Hennadii Stepanov) Pull request description: This PR adds a check that Boost.Process can be used without linking any libraries (header-only). Disable the functionality if that is not the case. Fixes bitcoin/bitcoin#24314. ACKs for top commit: fanquake: ACK abc057c6030b2a0ddab46835a7801054da677781 Tree-SHA512: ed2a32b1f751ec6f88cc7220766edd4cdab93c1d0515c892aa3094ee8d5b13ef569830d6e7a7a00c0197b117585dc526d00d943cc99a1f8c8a66ac4e20fe2061 --- configure.ac | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c2723841e083..eb61af751acf 100644 --- a/configure.ac +++ b/configure.ac @@ -1623,8 +1623,20 @@ if test "$use_external_signer" != "no"; then use_external_signer="no"; ;; *) - use_external_signer="yes" - AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) + AC_MSG_CHECKING([whether Boost.Process can be used]) + AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]])], + [have_boost_process="yes"], + [have_boost_process="no"]) + AC_MSG_RESULT([$have_boost_process]) + if test "$have_boost_process" == "yes"; then + use_external_signer="yes" + AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) + else + if test "$use_external_signer" = "yes"; then + AC_MSG_ERROR([External signing is not supported for this Boost version]) + fi + use_external_signer="no"; + fi ;; esac fi From 8abdbcb3c888e682fee2ee2d82b98b54122476a1 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Mon, 28 Feb 2022 13:47:14 +0100 Subject: [PATCH 02/13] Merge bitcoin/bitcoin#24397: build: Fix Boost.Process check for Boost 1.73 and older 774323e378acea998069980edf50e1a26f4b6b9e ci: Force `--enable-external-signer` to prevent future regressions (Hennadii Stepanov) 69978858a4d8a9c34070f96133b8f95737a6505b build: Fix Boost.Process check for Boost 1.73 and older (Hennadii Stepanov) 2199ef79cba865af7949c3a55938ad54519ca5b8 build: Fix a non-portable use of `test` (Hennadii Stepanov) d436c488d4216298016b09f8f97e10eada489bda build, refactor: Replace tabs with spaces (Hennadii Stepanov) Pull request description: On master (5f44c5c428b696af4214b2519cb2bbeb0e4a1027) Boost.Process check false fails without the `-lpthread` flag. ``` $ grep -C 2 pthread_detach config.log /usr/bin/ld: /tmp/cczCQfQv.o: in function `boost::asio::detail::posix_global_impl::~posix_global_impl()': conftest.cpp:(.text._ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED2Ev[_ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED5Ev]+0xa3): undefined reference to `pthread_join' /usr/bin/ld: conftest.cpp:(.text._ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED2Ev[_ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED5Ev]+0xc4): undefined reference to `pthread_detach' collect2: error: ld returned 1 exit status configure:26674: $? = 1 ``` Not required for Boost 1.74+. ACKs for top commit: laanwj: Code review ACK 774323e378acea998069980edf50e1a26f4b6b9e, is a bugfix/workaround, seems fine to merge last minute for 23.0. Tree-SHA512: 2a9d4b67fd8910e107af972d8c223286b7c933bc310616f86c8b6d8c903438916980fc76bd7e37f2698f6ce5361dc706cbf2933d1ac2c081bcabe1b83ca7d6b6 --- ci/dash/build_src.sh | 2 +- ci/test/00_setup_env_win64.sh | 2 +- configure.ac | 8 ++++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ci/dash/build_src.sh b/ci/dash/build_src.sh index ad1201e4599d..7014d51328d0 100755 --- a/ci/dash/build_src.sh +++ b/ci/dash/build_src.sh @@ -33,7 +33,7 @@ if [ -n "$CONFIG_SHELL" ]; then export CONFIG_SHELL="$CONFIG_SHELL" fi -BITCOIN_CONFIG_ALL="--disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib" +BITCOIN_CONFIG_ALL="--enable-external-signer --disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib" if [ -z "$NO_WERROR" ]; then BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} --enable-werror" fi diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index 2010c111818d..215a2db8269b 100755 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -16,5 +16,5 @@ export GOAL="deploy" # Prior to 11.0.0, the mingw-w64 headers were missing noreturn attributes, causing warnings when # cross-compiling for Windows. https://sourceforge.net/p/mingw-w64/bugs/306/ # https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe -export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" +export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner --disable-external-signer CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" export DIRECT_WINE_EXEC_TESTS=true diff --git a/configure.ac b/configure.ac index eb61af751acf..f7cfdff52f4a 100644 --- a/configure.ac +++ b/configure.ac @@ -354,7 +354,7 @@ if test "$enable_debug" = "yes"; then dnl them, to prevent autoconfs "-g -O2" being added. Otherwise we'd end up dnl with "-O0 -g3 -g -O2". if test "$CXXFLAGS_overridden" = "no"; then - CXXFLAGS="" + CXXFLAGS="" fi dnl Disable all optimizations @@ -1624,11 +1624,15 @@ if test "$use_external_signer" != "no"; then ;; *) AC_MSG_CHECKING([whether Boost.Process can be used]) + TEMP_LDFLAGS="$LDFLAGS" + dnl Boost 1.73 and older require the following workaround. + LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS" AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]])], [have_boost_process="yes"], [have_boost_process="no"]) + LDFLAGS="$TEMP_LDFLAGS" AC_MSG_RESULT([$have_boost_process]) - if test "$have_boost_process" == "yes"; then + if test "$have_boost_process" = "yes"; then use_external_signer="yes" AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) else From db6f85dd8b90bef4486313cad6abf4904bc883fe Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 11 Mar 2022 10:23:20 +0000 Subject: [PATCH 03/13] Merge bitcoin/bitcoin#24521: build: Fix Boost.Process detection on macOS arm64 1d4157a42b77411579eb721b5c6fb533a357959d build: Fix Boost.Process detection on macOS arm64 (Hennadii Stepanov) Pull request description: Could be tested as follows: ``` % brew install boost@1.76 % ./autogen.sh % ./configure --with-boost='/opt/homebrew/opt/boost@1.76' ``` ACKs for top commit: promag: Tested ACK on 1d4157a42b77411579eb721b5c6fb533a357959d with boost 1.76 on macOS arm64. #24523 is required for boost 1.78. Tree-SHA512: 7abd39a78e970ecc051e53b5923dfc31d3f0576cf4ff7fcfb9c8708c857c46a7a595ec36238def83f41158970eeee209980da4b8b70f0ff68f940a38ac9a0471 --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index f7cfdff52f4a..87d3cdcda612 100644 --- a/configure.ac +++ b/configure.ac @@ -1624,6 +1624,8 @@ if test "$use_external_signer" != "no"; then ;; *) AC_MSG_CHECKING([whether Boost.Process can be used]) + TEMP_CPPFLAGS="$CPPFLAGS" + CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" TEMP_LDFLAGS="$LDFLAGS" dnl Boost 1.73 and older require the following workaround. LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS" @@ -1631,6 +1633,7 @@ if test "$use_external_signer" != "no"; then [have_boost_process="yes"], [have_boost_process="no"]) LDFLAGS="$TEMP_LDFLAGS" + CPPFLAGS="$TEMP_CPPFLAGS" AC_MSG_RESULT([$have_boost_process]) if test "$have_boost_process" = "yes"; then use_external_signer="yes" From 6f34964a45c75041e1a6bcb36d7632afcf422f7a Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Tue, 29 Mar 2022 13:31:44 +0200 Subject: [PATCH 04/13] Merge bitcoin/bitcoin#24523: build: Fix Boost.Process test for Boost 1.78 532c64a7264dd3c7329e8839547837c57da7dbe8 build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov) Pull request description: Rebased #24415 with Luke's suggestion. Fixes #24413. ACKs for top commit: hebasto: ACK 532c64a7264dd3c7329e8839547837c57da7dbe8, tested on Mac mini (M1, 2020) + macOS Monterey 12.3 (21E230). Tree-SHA512: 74f779695f6bbc45a2b7341a1402f747cc0d433d74825c7196cb9f156db0c0299895365f01665bd0bff12a8ebb5ea33a29b9a52f5eac0007ec35d1dca6544705 --- configure.ac | 5 +++++ src/test/system_tests.cpp | 9 +++++++++ src/util/system.cpp | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/configure.ac b/configure.ac index 87d3cdcda612..08d26d2727d3 100644 --- a/configure.ac +++ b/configure.ac @@ -1624,6 +1624,10 @@ if test "$use_external_signer" != "no"; then ;; *) AC_MSG_CHECKING([whether Boost.Process can be used]) + TEMP_CXXFLAGS="$CXXFLAGS" + dnl Boost 1.78 requires the following workaround. + dnl See: https://github.com/boostorg/process/issues/235 + CXXFLAGS="$CXXFLAGS -Wno-error=narrowing" TEMP_CPPFLAGS="$CPPFLAGS" CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" TEMP_LDFLAGS="$LDFLAGS" @@ -1634,6 +1638,7 @@ if test "$use_external_signer" != "no"; then [have_boost_process="no"]) LDFLAGS="$TEMP_LDFLAGS" CPPFLAGS="$TEMP_CPPFLAGS" + CXXFLAGS="$TEMP_CXXFLAGS" AC_MSG_RESULT([$have_boost_process]) if test "$have_boost_process" = "yes"; then use_external_signer="yes" diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index c2e7a77e73a8..f5f94d442626 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -12,7 +12,16 @@ // For details see https://github.com/bitcoin/bitcoin/pull/22348. #define __kernel_entry #endif +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif #include +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif #endif // ENABLE_EXTERNAL_SIGNER #include diff --git a/src/util/system.cpp b/src/util/system.cpp index 50c761b8418a..5fca3e24bd2e 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -9,7 +9,16 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif #include +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif #endif // ENABLE_EXTERNAL_SIGNER #include From 096612988ba3257d7d262b0b13107982a6d434ba Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 28 Jul 2022 11:02:04 +0100 Subject: [PATCH 05/13] Merge bitcoin/bitcoin#25723: test: Drop unused boost workaround ba9a8e6cc1d1e9746f74e8c75af5c6c0a49f25c3 test: Drop unused boost workaround (Hennadii Stepanov) Pull request description: This PR is a follow up of bitcoin/bitcoin#24065 and removes the workaround which has already been removed in other [places](https://github.com/bitcoin/bitcoin/pull/24065/files#diff-19427b0dd1a791adc728c82e88f267751ba4f1c751e19262cac03cccd2822216). Moreover, this workaround won't be required even if bitcoin/bitcoin#25696 is ever merged. ACKs for top commit: fanquake: ACK ba9a8e6cc1d1e9746f74e8c75af5c6c0a49f25c3 Tree-SHA512: db19fc1550252d7a82a08f388ff6078c78452365e74b41e7bc36cbbc4d0fed9342636e8f2371bb8e78c9d11ee721d6363bcc21d11787f3aac967a6c4a9cc346f --- src/test/system_tests.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index f5f94d442626..383a6496938c 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -7,11 +7,6 @@ #include #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 #if defined(__GNUC__) // Boost 1.78 requires the following workaround. // See: https://github.com/boostorg/process/issues/235 From e5497458966a25deda926f1aa1f2a4b429772290 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 13 Aug 2022 16:04:59 +0100 Subject: [PATCH 06/13] Merge bitcoin/bitcoin#25235: GetExternalSigner(): fail if multiple signers are found 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 GetExternalSigner(): fail if multiple signers are found (amadeuszpawlik) Pull request description: If there are multiple external signers, `GetExternalSigner()` will just pick the first one in the list. If the user has two or more hardware wallets connected at the same time, he might not notice this. This PR adds a check and fails with suitable message, forcing the user to disconnect all but one external signer, so that there is no ambiguity as to which external signer was used. ACKs for top commit: Sjors: tACK 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 achow101: ACK 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 Tree-SHA512: e2a41d3eecc607d4f94e708614bed0f3545f7abba85f300c5a5f0d3d17d72c815259734accc5ca370953eacd290f27894ba2c18016f5e9584cd50fa1ec2fbb0b --- src/qt/walletcontroller.cpp | 4 +++ .../external_signer_scriptpubkeyman.cpp | 3 +- test/functional/mocks/invalid_signer.py | 2 +- test/functional/mocks/multi_signers.py | 30 +++++++++++++++++++ test/functional/mocks/signer.py | 2 +- test/functional/rpc_signer.py | 5 +--- test/functional/wallet_signer.py | 15 ++++++++++ 7 files changed, 54 insertions(+), 7 deletions(-) create mode 100755 test/functional/mocks/multi_signers.py diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index a6b071ef1b2f..f694dbfde523 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -376,6 +376,10 @@ void CreateWalletActivity::create() } catch (const std::runtime_error& e) { QMessageBox::critical(nullptr, tr("Can't list signers"), e.what()); } + if (signers.size() > 1) { + QMessageBox::critical(nullptr, tr("Too many external signers found"), QString::fromStdString("More than one external signer found. Please connect only one at a time.")); + signers.clear(); + } m_create_wallet_dialog->setSigners(signers); m_create_wallet_dialog->setWindowModality(Qt::ApplicationModal); diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 2d50c09b3574..14523eb87f59 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -45,7 +45,8 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { std::vector signers; ExternalSigner::Enumerate(command, signers, Params().NetworkIDString()); if (signers.empty()) throw std::runtime_error(std::string(__func__) + ": No external signers found"); - // TODO: add fingerprint argument in case of multiple signers + // TODO: add fingerprint argument instead of failing in case of multiple signers. + if (signers.size() > 1) throw std::runtime_error(std::string(__func__) + ": More than one external signer found. Please connect only one at a time."); return signers[0]; } diff --git a/test/functional/mocks/invalid_signer.py b/test/functional/mocks/invalid_signer.py index 4dd213cc3c0b..d99f33b297d9 100755 --- a/test/functional/mocks/invalid_signer.py +++ b/test/functional/mocks/invalid_signer.py @@ -18,7 +18,7 @@ def perform_pre_checks(): sys.exit(int(mock_result[0])) def enumerate(args): - sys.stdout.write(json.dumps([{"fingerprint": "b3c19bfc", "type": "trezor", "model": "trezor_t"}, {"fingerprint": "00000002"}])) + sys.stdout.write(json.dumps([{"fingerprint": "b3c19bfc", "type": "trezor", "model": "trezor_t"}])) def getdescriptors(args): xpub_pkh = "xpub6CRhJvXV8x2AKWvqi1ZSMFU6cbxzQiYrv3dxSUXCawjMJ1JzpqVsveH4way1yCmJm29KzH1zrVZmVwes4Qo6oXVE1HFn4fdiKrYJngqFFc6" diff --git a/test/functional/mocks/multi_signers.py b/test/functional/mocks/multi_signers.py new file mode 100755 index 000000000000..88f93e23dea0 --- /dev/null +++ b/test/functional/mocks/multi_signers.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +import argparse +import json +import sys + +def enumerate(args): + sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}, + {"fingerprint": "00000002", "type": "trezor", "model": "trezor_one"}])) + +parser = argparse.ArgumentParser(prog='./multi_signers.py', description='External multi-signer mock') + +subparsers = parser.add_subparsers(description='Commands', dest='command') +subparsers.required = True + +parser_enumerate = subparsers.add_parser('enumerate', help='list available signers') +parser_enumerate.set_defaults(func=enumerate) + + +if not sys.stdin.isatty(): + buffer = sys.stdin.read() + if buffer and buffer.rstrip() != "": + sys.argv.extend(buffer.rstrip().split(" ")) + +args = parser.parse_args() + +args.func(args) diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py index 0d5014a833b1..432de515c88d 100755 --- a/test/functional/mocks/signer.py +++ b/test/functional/mocks/signer.py @@ -18,7 +18,7 @@ def perform_pre_checks(): sys.exit(int(mock_result[0])) def enumerate(args): - sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}, {"fingerprint": "00000002"}])) + sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}])) def getdescriptors(args): xpub = "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B" diff --git a/test/functional/rpc_signer.py b/test/functional/rpc_signer.py index 2c17bc6a16e3..2d8ae0426652 100755 --- a/test/functional/rpc_signer.py +++ b/test/functional/rpc_signer.py @@ -70,10 +70,7 @@ def run_test(self): ) self.clear_mock_result(self.nodes[1]) - result = self.nodes[1].enumeratesigners() - assert_equal(len(result['signers']), 2) - assert_equal(result['signers'][0]["fingerprint"], "00000001") - assert_equal(result['signers'][0]["name"], "trezor_t") + assert_equal({'fingerprint': '00000001', 'name': 'trezor_t'} in self.nodes[1].enumeratesigners()['signers'], True) if __name__ == '__main__': RPCSignerTest().main() diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 19ac88527ef1..cbb59bede125 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -32,6 +32,13 @@ def mock_invalid_signer_path(self): else: return path + def mock_multi_signers_path(self): + path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'multi_signers.py') + if platform.system() == "Windows": + return "py " + path + else: + return path + def set_test_params(self): self.num_nodes = 2 @@ -55,6 +62,8 @@ def run_test(self): self.test_valid_signer() self.restart_node(1, [f"-signer={self.mock_invalid_signer_path()}", "-keypool=10"]) self.test_invalid_signer() + self.restart_node(1, [f"-signer={self.mock_multi_signers_path()}", "-keypool=10"]) + self.test_multiple_signers() def test_valid_signer(self): self.log.debug(f"-signer={self.mock_signer_path()}") @@ -190,5 +199,11 @@ def test_invalid_signer(self): self.log.info('Test invalid external signer') assert_raises_rpc_error(-1, "Invalid descriptor", self.nodes[1].createwallet, wallet_name='hww_invalid', disable_private_keys=True, descriptors=True, external_signer=True) + def test_multiple_signers(self): + self.log.debug(f"-signer={self.mock_multi_signers_path()}") + self.log.info('Test multiple external signers') + + assert_raises_rpc_error(-1, "GetExternalSigner: More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', disable_private_keys=True, descriptors=True, external_signer=True) + if __name__ == '__main__': WalletSignerTest().main() From bf70ffbd31a6680c4ffced8246633a5252741a8c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 8 Mar 2023 21:00:14 -0500 Subject: [PATCH 07/13] Merge bitcoin/bitcoin#25696: build: Re-enable external signer on Windows 1a0d8e178c7b9a3e94d14f94b77305802ebdc93b build: Re-enable external signer on Windows (Hennadii Stepanov) 989451d0689543b25ee6bf1d5b82c863d583597b configure: Detect compatibility of Boost.Process rather than hardcode non-Windows (Luke Dashjr) Pull request description: As https://github.com/boostorg/process/issues/207 has been resolved, it is possible now to re-enable external signer on Windows when cross-compiling. Guix build hashes: ``` 78f69ea7e0dbc8338981a92c0352220ccd7c2272d8cbff6a3b082a1412a935c5 guix-build-1a0d8e178c7b/output/aarch64-linux-gnu/SHA256SUMS.part ee17456ec818ddf5a175182508966e622573ccb518807cca43a40fa1dceda092 guix-build-1a0d8e178c7b/output/aarch64-linux-gnu/bitcoin-1a0d8e178c7b-aarch64-linux-gnu-debug.tar.gz 5080551bde379c746cc67b10429aef33b9f9e49d2d4e21ee1c3bfd9c1c845d46 guix-build-1a0d8e178c7b/output/aarch64-linux-gnu/bitcoin-1a0d8e178c7b-aarch64-linux-gnu.tar.gz dfab220ce76a40bf7dcf07aab352a616a91b516503639455fe7e1b137bad3e85 guix-build-1a0d8e178c7b/output/arm-linux-gnueabihf/SHA256SUMS.part 516ceb822571a8bd88fe107dca434ef596b1e4328ccbda1d51e1d482d3050396 guix-build-1a0d8e178c7b/output/arm-linux-gnueabihf/bitcoin-1a0d8e178c7b-arm-linux-gnueabihf-debug.tar.gz 21325380638f817107c203b9a1aedb808d1a4a2b4041493753ca4cbf19aa4f2c guix-build-1a0d8e178c7b/output/arm-linux-gnueabihf/bitcoin-1a0d8e178c7b-arm-linux-gnueabihf.tar.gz cf48ed78fcfceaeb3610ccf22326d735a129dcbf9d50b557b3de359169aefdfd guix-build-1a0d8e178c7b/output/arm64-apple-darwin/SHA256SUMS.part d4d51e136148bac6a20bb3adb402c499967647736acb420bfdeb71603aba57da guix-build-1a0d8e178c7b/output/arm64-apple-darwin/bitcoin-1a0d8e178c7b-arm64-apple-darwin-unsigned.dmg 95bb62d24f860e08a392ddb74d5860ccf27e8baa183e6749af877d26a3bd6b0b guix-build-1a0d8e178c7b/output/arm64-apple-darwin/bitcoin-1a0d8e178c7b-arm64-apple-darwin-unsigned.tar.gz 68da4c92f37bb802df37141af194f47c16da1d84f77a0fbb1016013ae0338502 guix-build-1a0d8e178c7b/output/arm64-apple-darwin/bitcoin-1a0d8e178c7b-arm64-apple-darwin.tar.gz 6704e38c2d3f11321403797598d05f062648fec6f2d76900ba250dab481e29da guix-build-1a0d8e178c7b/output/dist-archive/bitcoin-1a0d8e178c7b.tar.gz 64b936bc90d1e01fe8f276511edc9bb945dcebe70332aa37d3a786348443b8e7 guix-build-1a0d8e178c7b/output/powerpc64-linux-gnu/SHA256SUMS.part 3d03532e54b6e42498ea240c86b8567e94fd462f56087b869c3d6f09e2dde878 guix-build-1a0d8e178c7b/output/powerpc64-linux-gnu/bitcoin-1a0d8e178c7b-powerpc64-linux-gnu-debug.tar.gz c5843d79a58b0a864fe723458dab4eee54ad11f4b1f7960975b086eeedc0d541 guix-build-1a0d8e178c7b/output/powerpc64-linux-gnu/bitcoin-1a0d8e178c7b-powerpc64-linux-gnu.tar.gz f861ff519bd5e3d6d5ce1646ee0a06bcef1288ddb804a4a600e4dbfe5d5be521 guix-build-1a0d8e178c7b/output/powerpc64le-linux-gnu/SHA256SUMS.part 5f477da21980dbcf9696081903dc1ba8a3f79ce3579641d208e69a6f598c8eb9 guix-build-1a0d8e178c7b/output/powerpc64le-linux-gnu/bitcoin-1a0d8e178c7b-powerpc64le-linux-gnu-debug.tar.gz b3757b11c614136934158acea5139e8abd0c5c9cdfda72ae44db436f21716b33 guix-build-1a0d8e178c7b/output/powerpc64le-linux-gnu/bitcoin-1a0d8e178c7b-powerpc64le-linux-gnu.tar.gz 1c21bdb17fe3436e685e88c62423e630fe2b3c41dd00025a99fd80d97817ac2f guix-build-1a0d8e178c7b/output/riscv64-linux-gnu/SHA256SUMS.part f36ae98473f086ae8f0dc66223b5ec407d57dc4d8d45ae284401520ff5c0b273 guix-build-1a0d8e178c7b/output/riscv64-linux-gnu/bitcoin-1a0d8e178c7b-riscv64-linux-gnu-debug.tar.gz 1603e4d0e869eb47a1dc2d26b67772d0016d90f7ba5e50d2009365cc02cb8169 guix-build-1a0d8e178c7b/output/riscv64-linux-gnu/bitcoin-1a0d8e178c7b-riscv64-linux-gnu.tar.gz f86ef652102f022827b70477bffa0a44008c6300cf62ca7b3595146cf2ed91ba guix-build-1a0d8e178c7b/output/x86_64-apple-darwin/SHA256SUMS.part f84d435d8e4709bf29bc7ac7ed8dc6b8af4077cef05e520b468b2896ce10876a guix-build-1a0d8e178c7b/output/x86_64-apple-darwin/bitcoin-1a0d8e178c7b-x86_64-apple-darwin-unsigned.dmg af2aab969b7ed7aeea0e02adbcc9e3b438086bf76b6bfc36146c53e05a27bd57 guix-build-1a0d8e178c7b/output/x86_64-apple-darwin/bitcoin-1a0d8e178c7b-x86_64-apple-darwin-unsigned.tar.gz 32a5109ba28ab74ff66238e6a8f8a04e455ebce382a3be287df92a227818fe72 guix-build-1a0d8e178c7b/output/x86_64-apple-darwin/bitcoin-1a0d8e178c7b-x86_64-apple-darwin.tar.gz 377462e9a96f4aba72c915dd5df5159a4301a1fa8ed0ee48faa6c71573de80c3 guix-build-1a0d8e178c7b/output/x86_64-linux-gnu/SHA256SUMS.part a3bf62e828d2350a483b2d16205014f66e8884597b0b72e178042a958c548336 guix-build-1a0d8e178c7b/output/x86_64-linux-gnu/bitcoin-1a0d8e178c7b-x86_64-linux-gnu-debug.tar.gz 66cda980188ea1941a7d66c8b03c447580af33db55abe3bbe3581823ae0534a3 guix-build-1a0d8e178c7b/output/x86_64-linux-gnu/bitcoin-1a0d8e178c7b-x86_64-linux-gnu.tar.gz 2117f0dd9baeb4d585f841592e94c088f4487bf2008b8f281d0c3ceee92ff6cc guix-build-1a0d8e178c7b/output/x86_64-w64-mingw32/SHA256SUMS.part d40d5dec3287f467c42232c05d82f7fb538cda34bd2e63ff7e1876f471c3a790 guix-build-1a0d8e178c7b/output/x86_64-w64-mingw32/bitcoin-1a0d8e178c7b-win64-debug.zip 92dcc92765fbc07b1cc8258bfa69280541e1b4553cc41fed18672c2c6931d5c0 guix-build-1a0d8e178c7b/output/x86_64-w64-mingw32/bitcoin-1a0d8e178c7b-win64-setup-unsigned.exe a6dd9b4d29f21d3a18cf64556cb03446ef17bf801eb6ac257b65d27cbd95080f guix-build-1a0d8e178c7b/output/x86_64-w64-mingw32/bitcoin-1a0d8e178c7b-win64-unsigned.tar.gz a4022e595d955198f73530473ef8e90a708746089ee2dd27de794176873330c1 guix-build-1a0d8e178c7b/output/x86_64-w64-mingw32/bitcoin-1a0d8e178c7b-win64.zip ``` ACKs for top commit: Sjors: tACK 1a0d8e178c7b9a3e94d14f94b77305802ebdc93b achow101: ACK 1a0d8e178c7b9a3e94d14f94b77305802ebdc93b Tree-SHA512: db7319259b1e1571cfab4bb3b99ae10a2f744e62757cae5059fd6f4dd6d5586eb09feb63a0c4bb07f7128b283f1dc281ed435224bc8e40da577fd4f04cde489a --- ci/test/00_setup_env_win64.sh | 2 +- configure.ac | 78 +++++++++++++++++------------------ src/test/system_tests.cpp | 21 ++++++++-- 3 files changed, 57 insertions(+), 44 deletions(-) diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index 215a2db8269b..1fa7fce5a8db 100755 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -16,5 +16,5 @@ export GOAL="deploy" # Prior to 11.0.0, the mingw-w64 headers were missing noreturn attributes, causing warnings when # cross-compiling for Windows. https://sourceforge.net/p/mingw-w64/bugs/306/ # https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe -export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner --disable-external-signer CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" +export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner --enable-external-signer CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" export DIRECT_WINE_EXEC_TESTS=true diff --git a/configure.ac b/configure.ac index 08d26d2727d3..78ae9308679b 100644 --- a/configure.ac +++ b/configure.ac @@ -1612,45 +1612,45 @@ if test "$use_boost" = "yes"; then fi if test "$use_external_signer" != "no"; then - 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"; - ;; - *) - AC_MSG_CHECKING([whether Boost.Process can be used]) - TEMP_CXXFLAGS="$CXXFLAGS" - dnl Boost 1.78 requires the following workaround. - dnl See: https://github.com/boostorg/process/issues/235 - CXXFLAGS="$CXXFLAGS -Wno-error=narrowing" - TEMP_CPPFLAGS="$CPPFLAGS" - CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" - TEMP_LDFLAGS="$LDFLAGS" - dnl Boost 1.73 and older require the following workaround. - LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS" - AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]])], - [have_boost_process="yes"], - [have_boost_process="no"]) - LDFLAGS="$TEMP_LDFLAGS" - CPPFLAGS="$TEMP_CPPFLAGS" - CXXFLAGS="$TEMP_CXXFLAGS" - AC_MSG_RESULT([$have_boost_process]) - if test "$have_boost_process" = "yes"; then - use_external_signer="yes" - AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) - else - if test "$use_external_signer" = "yes"; then - AC_MSG_ERROR([External signing is not supported for this Boost version]) - fi - use_external_signer="no"; - fi - ;; - esac + AC_MSG_CHECKING([whether Boost.Process can be used]) + TEMP_CXXFLAGS="$CXXFLAGS" + dnl Boost 1.78 requires the following workaround. + dnl See: https://github.com/boostorg/process/issues/235 + CXXFLAGS="$CXXFLAGS -Wno-error=narrowing" + TEMP_CPPFLAGS="$CPPFLAGS" + CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" + TEMP_LDFLAGS="$LDFLAGS" + dnl Boost 1.73 and older require the following workaround. + LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS" + AC_LINK_IFELSE([AC_LANG_PROGRAM([[ + #define BOOST_PROCESS_USE_STD_FS + #include + ]],[[ + 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="yes"], + [have_boost_process="no"]) + LDFLAGS="$TEMP_LDFLAGS" + CPPFLAGS="$TEMP_CPPFLAGS" + CXXFLAGS="$TEMP_CXXFLAGS" + AC_MSG_RESULT([$have_boost_process]) + if test "$have_boost_process" = "yes"; then + use_external_signer="yes" + AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) + AC_DEFINE([BOOST_PROCESS_USE_STD_FS], [1], [Defined to avoid Boost::Process trying to use Boost Filesystem]) + else + if test "$use_external_signer" = "yes"; then + AC_MSG_ERROR([External signing is not supported for this Boost version]) + fi + use_external_signer="no"; + fi fi AM_CONDITIONAL([ENABLE_EXTERNAL_SIGNER], [test "$use_external_signer" = "yes"]) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 383a6496938c..735aa93874e8 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -34,6 +34,13 @@ BOOST_AUTO_TEST_CASE(dummy) BOOST_AUTO_TEST_CASE(run_command) { +#ifdef WIN32 + // https://www.winehq.org/pipermail/wine-devel/2008-September/069387.html + auto hntdll = GetModuleHandleA("ntdll.dll"); + assert(hntdll); + const bool wine_runtime = GetProcAddress(hntdll, "wine_get_version"); +#endif + { const UniValue result = RunCommandParseJSON(""); BOOST_CHECK(result.isNull()); @@ -51,21 +58,27 @@ BOOST_AUTO_TEST_CASE(run_command) } { // An invalid command is handled by Boost +#ifdef WIN32 + const int expected_error{wine_runtime ? 6 : 2}; +#else + const int expected_error{2}; +#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), boost::process::process_error, [&](const boost::process::process_error& e) { BOOST_CHECK(std::string(e.what()).find("RunCommandParseJSON error:") == std::string::npos); - BOOST_CHECK_EQUAL(e.code().value(), 2); + BOOST_CHECK_EQUAL(e.code().value(), expected_error); return true; }); } { // Return non-zero exit code, no output to stderr #ifdef WIN32 - const std::string command{"cmd.exe /c call"}; + const std::string command{"cmd.exe /c exit 1"}; #else const std::string command{"false"}; #endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const std::runtime_error& e) { - BOOST_CHECK(std::string(e.what()).find(strprintf("RunCommandParseJSON error: process(%s) returned 1: \n", command)) != std::string::npos); + const std::string what{e.what()}; + BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned 1: \n", command)) != std::string::npos); return true; }); } @@ -73,7 +86,7 @@ BOOST_AUTO_TEST_CASE(run_command) // Return non-zero exit code, with error message for stderr #ifdef WIN32 const std::string command{"cmd.exe /c dir nosuchfile"}; - const std::string expected{"File Not Found"}; + const std::string expected{wine_runtime ? "File not found." : "File Not Found"}; #else const std::string command{"ls nosuchfile"}; const std::string expected{"No such file or directory"}; From e8888d782e2324c2905e47ee39bdfa3ebb22eb1b Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 12 Apr 2023 15:49:47 +0100 Subject: [PATCH 08/13] Merge bitcoin/bitcoin#27449: doc: update OpenBSD build docs for 7.3 (external signer support available) 6b17994ede6fe1961667d2e96127291b2a8b4f9d doc: update OpenBSD build docs for 7.3 (external signer support available) (Sebastian Falbesoner) Pull request description: With OpenBSD 7.3, the waitid(2) system call is implemented (see https://github.com/openbsd/src/commit/8112871f19bbd25e86c93d0f901071ca2335a352, first mentioned kernel improvement at https://www.openbsd.org/73.html). This means Boost.Process finally doesn't fail to compile anymore and we can remove the build hint about missing external signer support. Tested on my amd64 machine by reconfiguring / rebuilding master branch and successfully running the functional test wallet_signer.py. :heavy_check_mark: ACKs for top commit: fanquake: ACK 6b17994ede6fe1961667d2e96127291b2a8b4f9d - haven't tested, but looks good to me. Tree-SHA512: 5bbcecce4ced38d8221f2c906a54667e50317e9ded182554cf73bb7f2fce55a38e53730eca25f813cff1d2d65c94141eb158d40f83228d12dcf859c16a1798b9 --- doc/build-openbsd.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index dfdff14e6f93..8b0ee196eb46 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -1,6 +1,6 @@ # OpenBSD Build Guide -**Updated for OpenBSD [7.1](https://www.openbsd.org/71.html)** +**Updated for OpenBSD [7.3](https://www.openbsd.org/73.html)** This guide describes how to build dashd, command-line utilities, and GUI on OpenBSD. @@ -82,10 +82,6 @@ export AUTOMAKE_VERSION=1.16 ### 1. Configuration -Note that external signer support is currently not available on OpenBSD, since -the used header-only library Boost.Process fails to compile (certain system -calls and preprocessor defines like `waitid()` and `WEXITED` are missing). - There are many ways to configure Dash Core, here are a few common examples: ##### Descriptor Wallet and GUI: From b881735ab2476a312bad91c99ad718183be40688 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 13 Dec 2023 11:27:02 +0000 Subject: [PATCH 09/13] Merge bitcoin/bitcoin#28967: build: disable external-signer for Windows 308aec3e5655327d98e0428d8205d246f24d6af5 build: disable external-signer for Windows (fanquake) 35537318a19360ddf1ea8f0c1e6d8ad49e635516 ci: remove --enable-external-signer from win64 job (fanquake) Pull request description: It's come to light that Boost ASIO (a Boost Process sub dep) has in some instances, been quietly initialising our network stack on Windows (see PR https://github.com/bitcoin/bitcoin/pull/28486 and discussion in https://github.com/bitcoin/bitcoin/issues/28940). This has been shielding a bug in our own code, but the larger issue is that Boost Process/ASIO is running code before main, and doing things like setting up networking. This undermines our own assumptions about how our binary works, happens before we run any sanity checks, and before we call our own code to setup networking. Note that ASIO also calls WSAStartup with version `2.0`, whereas we call with `2.2`. It's also not clear why a feature like external signer would have a dependency that would be doing anything network/socket related, given it only exists to spawn a local process. See also the discussion in https://github.com/bitcoin/bitcoin/issues/24907. Note that the maintaince of Boost Process in general, has not really improved. For example, rather than fixing bugs like https://github.com/boostorg/process/issues/111, i.e, https://github.com/boostorg/process/pull/317, the maintainer chooses to just wrap exception causing overflows in try-catch blocks: https://github.com/boostorg/process/commit/0c42a58eacab6a96b19196e399307bad8a938a27. These changes get merged in large, unreviewed PRs, i.e https://github.com/boostorg/process/pull/319. This PR disables external-signer on Windows for now. If, in future, someone changes how Boost Process works, or replaces it entirely with some properly reviewed and maintained code, we could reenable this feature on Windows. ACKs for top commit: hebasto: re-ACK 308aec3e5655327d98e0428d8205d246f24d6af5. TheCharlatan: ACK 308aec3e5655327d98e0428d8205d246f24d6af5 Tree-SHA512: 7405f7fc9833eeaacd6836c4e5b1c1a7845a40c1fdd55c1060152f8d8189e4777464fde650e11eb1539556a75dddf49667105987078b1457493ee772945da66e --- ci/test/00_setup_env_win64.sh | 2 +- configure.ac | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index 1fa7fce5a8db..2010c111818d 100755 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -16,5 +16,5 @@ export GOAL="deploy" # Prior to 11.0.0, the mingw-w64 headers were missing noreturn attributes, causing warnings when # cross-compiling for Windows. https://sourceforge.net/p/mingw-w64/bugs/306/ # https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe -export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner --enable-external-signer CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" +export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" export DIRECT_WINE_EXEC_TESTS=true diff --git a/configure.ac b/configure.ac index 78ae9308679b..ff4184f1a94d 100644 --- a/configure.ac +++ b/configure.ac @@ -1642,9 +1642,19 @@ if test "$use_external_signer" != "no"; then CXXFLAGS="$TEMP_CXXFLAGS" AC_MSG_RESULT([$have_boost_process]) if test "$have_boost_process" = "yes"; then - use_external_signer="yes" - AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) - AC_DEFINE([BOOST_PROCESS_USE_STD_FS], [1], [Defined to avoid Boost::Process trying to use Boost Filesystem]) + case $host in + dnl Boost Process for Windows uses Boost ASIO. Boost ASIO performs + dnl pre-main init of Windows networking libraries, which we do not + dnl want. + *mingw*) + use_external_signer="no" + ;; + *) + use_external_signer="yes" + AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) + AC_DEFINE([BOOST_PROCESS_USE_STD_FS], [1], [Defined to avoid Boost::Process trying to use Boost Filesystem]) + ;; + esac else if test "$use_external_signer" = "yes"; then AC_MSG_ERROR([External signing is not supported for this Boost version]) From 81f21fde127e342b3524ade5a3e8429986a59e00 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 28 Feb 2024 17:24:11 -0500 Subject: [PATCH 10/13] Merge bitcoin/bitcoin#29489: test: Remove Windows-specific code from `system_tests/run_command` 51bc1c7126d6e130bc40c529fb71ae6486da0492 test: Remove Windows-specific code from `system_tests/run_command` (Hennadii Stepanov) Pull request description: The removed code has been dead since https://github.com/bitcoin/bitcoin/pull/28967. Required as a precondition for replacing Boost.Process with [cpp-subprocess](https://github.com/bitcoin/bitcoin/pull/28981) to make diff for this code meaningful and reviewable. The plan is to reintroduce Windows-specific code in this test simultaneously with enabling Windows support in cpp-subprocess. ACKs for top commit: Sjors: utACK 51bc1c7126d6e130bc40c529fb71ae6486da0492 theStack: Code-review ACK 51bc1c7126d6e130bc40c529fb71ae6486da0492 Tree-SHA512: 0e3875c4dc20564332555633daf2227223b10dc3d052557635eced2734575d1e0252fb19e46ea6e6c47a15c51c345f70b6d437e33435abcd0e4fcf29edb50887 --- src/test/system_tests.cpp | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 735aa93874e8..4111aa8488b0 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -34,23 +34,12 @@ BOOST_AUTO_TEST_CASE(dummy) BOOST_AUTO_TEST_CASE(run_command) { -#ifdef WIN32 - // https://www.winehq.org/pipermail/wine-devel/2008-September/069387.html - auto hntdll = GetModuleHandleA("ntdll.dll"); - assert(hntdll); - const bool wine_runtime = GetProcAddress(hntdll, "wine_get_version"); -#endif - { const UniValue result = RunCommandParseJSON(""); BOOST_CHECK(result.isNull()); } { -#ifdef WIN32 - const UniValue result = RunCommandParseJSON("cmd.exe /c echo {\"success\": true}"); -#else const UniValue result = RunCommandParseJSON("echo \"{\"success\": true}\""); -#endif BOOST_CHECK(result.isObject()); const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); @@ -58,11 +47,7 @@ BOOST_AUTO_TEST_CASE(run_command) } { // An invalid command is handled by Boost -#ifdef WIN32 - const int expected_error{wine_runtime ? 6 : 2}; -#else const int expected_error{2}; -#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), boost::process::process_error, [&](const boost::process::process_error& e) { BOOST_CHECK(std::string(e.what()).find("RunCommandParseJSON error:") == std::string::npos); BOOST_CHECK_EQUAL(e.code().value(), expected_error); @@ -71,11 +56,7 @@ BOOST_AUTO_TEST_CASE(run_command) } { // Return non-zero exit code, no output to stderr -#ifdef WIN32 - const std::string command{"cmd.exe /c exit 1"}; -#else const std::string command{"false"}; -#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const std::runtime_error& e) { const std::string what{e.what()}; BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned 1: \n", command)) != std::string::npos); @@ -84,13 +65,8 @@ BOOST_AUTO_TEST_CASE(run_command) } { // Return non-zero exit code, with error message for stderr -#ifdef WIN32 - const std::string command{"cmd.exe /c dir nosuchfile"}; - const std::string expected{wine_runtime ? "File not found." : "File Not Found"}; -#else const std::string command{"ls nosuchfile"}; const std::string expected{"No such file or directory"}; -#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const std::runtime_error& e) { const std::string what(e.what()); BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned", command)) != std::string::npos); @@ -100,15 +76,10 @@ BOOST_AUTO_TEST_CASE(run_command) } { // Unable to parse JSON -#ifdef WIN32 - const std::string command{"cmd.exe /c echo {"}; -#else const std::string command{"echo {"}; -#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, HasReason("Unable to parse JSON: {")); } - // Test std::in, except for Windows -#ifndef WIN32 + // Test std::in { const UniValue result = RunCommandParseJSON("cat", "{\"success\": true}"); BOOST_CHECK(result.isObject()); @@ -116,7 +87,6 @@ BOOST_AUTO_TEST_CASE(run_command) BOOST_CHECK(!success.isNull()); BOOST_CHECK_EQUAL(success.get_bool(), true); } -#endif } #endif // ENABLE_EXTERNAL_SIGNER From 5a34a79428cec653de16834fb8b3aef6b8e676d7 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 10 Oct 2022 17:41:29 +0800 Subject: [PATCH 11/13] Merge bitcoin/bitcoin#26196: kernel: move RunCommandParseJSON to its own file 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b refactor: move run_command from util to common (Cory Fields) 192325a77d593e404e74ef5e204aed8801b4e66f kernel: move RunCommandParseJSON to its own file (Cory Fields) Pull request description: Because libbitcoinkernel does not include this new object, this has the side-effect of eliminating its unnecessary `boost::process` dependency. This leaves libbitcoinkernel with 3 remaining boost dependencies: - `boost::date_time` for `util/time.cpp`, which I'll separate out next. Exactly like this PR. - `boost::signals2` for which I have a POC re-implementation here: https://github.com/theuni/bitcoin/commits/replace-boost-signals - `boost::multi_index` which I'm not sure about yet. ACKs for top commit: ryanofsky: Code review ACK 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b. Could consider squashing the two commits, so the code just moves once instead of twice. fanquake: ACK 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b Tree-SHA512: f2a46cac34aaadfb8a1442316152ad354f6990021b82c78d80cae9fd43cd026209ffd62132eaa99d5d0f8cf34e996b6737d318a9d9a3f1d2ff8d17d697abf26d --- src/Makefile.am | 2 ++ src/common/run_command.cpp | 64 ++++++++++++++++++++++++++++++++++++++ src/common/run_command.h | 21 +++++++++++++ src/external_signer.cpp | 2 +- src/test/system_tests.cpp | 2 +- src/util/system.cpp | 52 ------------------------------- src/util/system.h | 8 ----- 7 files changed, 89 insertions(+), 62 deletions(-) create mode 100644 src/common/run_command.cpp create mode 100644 src/common/run_command.h diff --git a/src/Makefile.am b/src/Makefile.am index 2ed2fb3a1943..f939980fabf2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -179,6 +179,7 @@ BITCOIN_CORE_H = \ coinjoin/walletman.h \ coins.h \ common/bloom.h \ + common/run_command.h \ compat/assumptions.h \ compat/byteswap.h \ compat/compat.h \ @@ -899,6 +900,7 @@ libbitcoin_common_a_SOURCES = \ chainparams.cpp \ coins.cpp \ common/bloom.cpp \ + common/run_command.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp new file mode 100644 index 000000000000..6ad9f75b5d99 --- /dev/null +++ b/src/common/run_command.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#if defined(HAVE_CONFIG_H) +#include +#endif + +#include + +#include +#include + +#ifdef ENABLE_EXTERNAL_SIGNER +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif +#include +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif +#endif // ENABLE_EXTERNAL_SIGNER + +UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) +{ +#ifdef ENABLE_EXTERNAL_SIGNER + namespace bp = boost::process; + + UniValue result_json; + bp::opstream stdin_stream; + bp::ipstream stdout_stream; + bp::ipstream stderr_stream; + + if (str_command.empty()) return UniValue::VNULL; + + bp::child c( + str_command, + bp::std_out > stdout_stream, + bp::std_err > stderr_stream, + bp::std_in < stdin_stream + ); + if (!str_std_in.empty()) { + stdin_stream << str_std_in << std::endl; + } + stdin_stream.pipe().close(); + + std::string result; + std::string error; + std::getline(stdout_stream, result); + std::getline(stderr_stream, error); + + c.wait(); + const int n_error = c.exit_code(); + if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); + if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); + + return result_json; +#else + throw std::runtime_error("Compiled without external signing support (required for external signing)."); +#endif // ENABLE_EXTERNAL_SIGNER +} diff --git a/src/common/run_command.h b/src/common/run_command.h new file mode 100644 index 000000000000..2fbdc071eeca --- /dev/null +++ b/src/common/run_command.h @@ -0,0 +1,21 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_COMMON_RUN_COMMAND_H +#define BITCOIN_COMMON_RUN_COMMAND_H + +#include + +class UniValue; + +/** + * Execute a command which returns JSON, and parse the result. + * + * @param str_command The command to execute, including any arguments + * @param str_std_in string to pass to stdin + * @return parsed JSON + */ +UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); + +#endif // BITCOIN_COMMON_RUN_COMMAND_H diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 3057fa687e0c..8f2d6601879c 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -3,10 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include -#include #include #include diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 4111aa8488b0..6757f477a385 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // #include -#include +#include #include #ifdef ENABLE_EXTERNAL_SIGNER diff --git a/src/util/system.cpp b/src/util/system.cpp index 5fca3e24bd2e..c9e849b69229 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -8,19 +8,6 @@ #include -#ifdef ENABLE_EXTERNAL_SIGNER -#if defined(__GNUC__) -// Boost 1.78 requires the following workaround. -// See: https://github.com/boostorg/process/issues/235 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wnarrowing" -#endif -#include -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif -#endif // ENABLE_EXTERNAL_SIGNER - #include #include #include @@ -1425,45 +1412,6 @@ void RenameThreadPool(ctpl::thread_pool& tp, const char* baseName) } } -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) -{ -#ifdef ENABLE_EXTERNAL_SIGNER - namespace bp = boost::process; - - UniValue result_json; - bp::opstream stdin_stream; - bp::ipstream stdout_stream; - bp::ipstream stderr_stream; - - if (str_command.empty()) return UniValue::VNULL; - - bp::child c( - str_command, - bp::std_out > stdout_stream, - bp::std_err > stderr_stream, - bp::std_in < stdin_stream - ); - if (!str_std_in.empty()) { - stdin_stream << str_std_in << std::endl; - } - stdin_stream.pipe().close(); - - std::string result; - std::string error; - std::getline(stdout_stream, result); - std::getline(stderr_stream, error); - - c.wait(); - const int n_error = c.exit_code(); - if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); - if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); - - return result_json; -#else - throw std::runtime_error("Compiled without external signing support (required for external signing)."); -#endif // ENABLE_EXTERNAL_SIGNER -} - void SetupEnvironment() { #ifdef HAVE_MALLOPT_ARENA_MAX diff --git a/src/util/system.h b/src/util/system.h index 51bae62d70fe..0b228858047f 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -113,14 +113,6 @@ std::string ShellEscape(const std::string& arg); #if HAVE_SYSTEM void runCommand(const std::string& strCommand); #endif -/** - * Execute a command which returns JSON, and parse the result. - * - * @param str_command The command to execute, including any arguments - * @param str_std_in string to pass to stdin - * @return parsed JSON - */ -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); /** * Most paths passed as configuration arguments are treated as relative to From 00e66c7c279289beec84ca92bcaf1bfe3c352383 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 30 Jun 2023 09:27:14 +0100 Subject: [PATCH 12/13] Merge bitcoin/bitcoin#28002: refactor: remove in-code warning suppression 3210f224dbeaf0f9de09d4e83c3e6666128a6717 refactor: remove in-code warning suppression (fanquake) Pull request description: Should no-longer be needed post #27872. If it is, then suppress-external-warnings should be fixed. ACKs for top commit: hebasto: ACK 3210f224dbeaf0f9de09d4e83c3e6666128a6717 Tree-SHA512: 2405250b7308779d576f13ce9144944abd5b2293499a0c0fe940398dae951cb871246a55c0e644a038ee238f9510b5845c3e39f9658d9f10225a076d8122f078 --- src/common/run_command.cpp | 9 --------- src/test/system_tests.cpp | 9 --------- src/wallet/bdb.h | 7 ------- 3 files changed, 25 deletions(-) diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index 6ad9f75b5d99..8bd5febd5386 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -12,16 +12,7 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#if defined(__GNUC__) -// Boost 1.78 requires the following workaround. -// See: https://github.com/boostorg/process/issues/235 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wnarrowing" -#endif #include -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif #endif // ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 6757f477a385..55cbe3fe64a8 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -7,16 +7,7 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#if defined(__GNUC__) -// Boost 1.78 requires the following workaround. -// See: https://github.com/boostorg/process/issues/235 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wnarrowing" -#endif #include -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif #endif // ENABLE_EXTERNAL_SIGNER #include diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 8a4feb696bc8..1d5f21e88d23 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -21,14 +21,7 @@ struct bilingual_str; -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wsuggest-override" -#endif #include -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic pop -#endif namespace wallet { struct WalletDatabaseFileId { From b7fe867177437c664f140010ee87a608a3bb8bca Mon Sep 17 00:00:00 2001 From: merge-script Date: Wed, 10 Apr 2024 12:03:06 +0200 Subject: [PATCH 13/13] Merge bitcoin/bitcoin#28981: Replace Boost.Process with cpp-subprocess d5a715536e497c160a2520f81334aab6c7490213 build: remove boost::process dependency for building external signer support (Sebastian Falbesoner) 70434b1c443d9251a880d0193af771f574c40617 external_signer: replace boost::process with cpp-subprocess (Sebastian Falbesoner) cc8b9875b104c31f0a5b5e4195a8278ec55f35f7 Add `cpp-subprocess` header-only library (Hennadii Stepanov) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/24907. This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049). The `subprocess.hpp` header has been sourced from the [upstream repo](https://github.com/arun11299/cpp-subprocess) with the only modification being the removal of convenience functions, which are not utilized in our codebase. Windows-related changes will be addressed in subsequent follow-ups. ACKs for top commit: achow101: reACK d5a715536e497c160a2520f81334aab6c7490213 Sjors: re-tACK d5a715536e497c160a2520f81334aab6c7490213 theStack: Light re-ACK d5a715536e497c160a2520f81334aab6c7490213 fanquake: ACK d5a715536e497c160a2520f81334aab6c7490213 - with the expectation that this code is going to be maintained as our own. Next PRs should: Tree-SHA512: d7fb6fecc3f5792496204190afb7d85b3e207b858fb1a75efe483c05260843b81b27d14b299323bb667c990e87a07197059afea3796cf218ed8b614086bd3611 --- configure.ac | 62 +- src/Makefile.am | 1 + src/common/run_command.cpp | 25 +- src/external_signer.cpp | 8 +- src/test/system_tests.cpp | 14 +- src/util/subprocess.hpp | 1986 ++++++++++++++++++++++++++++++++++++ test/lint/lint-includes.py | 1 - test/lint/lint-spelling.py | 2 +- 8 files changed, 2017 insertions(+), 82 deletions(-) create mode 100644 src/util/subprocess.hpp diff --git a/configure.ac b/configure.ac index ff4184f1a94d..d88e4cdd5d4b 100644 --- a/configure.ac +++ b/configure.ac @@ -320,9 +320,9 @@ AC_ARG_ENABLE([werror], [enable_werror=no]) AC_ARG_ENABLE([external-signer], - [AS_HELP_STRING([--enable-external-signer],[compile external signer support (default is auto, requires Boost::Process)])], + [AS_HELP_STRING([--enable-external-signer],[compile external signer support (default is yes)])], [use_external_signer=$enableval], - [use_external_signer=auto]) + [use_external_signer=yes]) AC_LANG_PUSH([C++]) @@ -1611,56 +1611,14 @@ if test "$use_boost" = "yes"; then fi fi -if test "$use_external_signer" != "no"; then - AC_MSG_CHECKING([whether Boost.Process can be used]) - TEMP_CXXFLAGS="$CXXFLAGS" - dnl Boost 1.78 requires the following workaround. - dnl See: https://github.com/boostorg/process/issues/235 - CXXFLAGS="$CXXFLAGS -Wno-error=narrowing" - TEMP_CPPFLAGS="$CPPFLAGS" - CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" - TEMP_LDFLAGS="$LDFLAGS" - dnl Boost 1.73 and older require the following workaround. - LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS" - AC_LINK_IFELSE([AC_LANG_PROGRAM([[ - #define BOOST_PROCESS_USE_STD_FS - #include - ]],[[ - 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="yes"], - [have_boost_process="no"]) - LDFLAGS="$TEMP_LDFLAGS" - CPPFLAGS="$TEMP_CPPFLAGS" - CXXFLAGS="$TEMP_CXXFLAGS" - AC_MSG_RESULT([$have_boost_process]) - if test "$have_boost_process" = "yes"; then - case $host in - dnl Boost Process for Windows uses Boost ASIO. Boost ASIO performs - dnl pre-main init of Windows networking libraries, which we do not - dnl want. - *mingw*) - use_external_signer="no" - ;; - *) - use_external_signer="yes" - AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) - AC_DEFINE([BOOST_PROCESS_USE_STD_FS], [1], [Defined to avoid Boost::Process trying to use Boost Filesystem]) - ;; - esac - else - if test "$use_external_signer" = "yes"; then - AC_MSG_ERROR([External signing is not supported for this Boost version]) - fi - use_external_signer="no"; - fi +case $host in + dnl Re-enable it after enabling Windows support in cpp-subprocess. + *mingw*) + use_external_signer="no" + ;; +esac +if test "$use_external_signer" = "yes"; then + AC_DEFINE([ENABLE_EXTERNAL_SIGNER], [1], [Define if external signer support is enabled]) fi AM_CONDITIONAL([ENABLE_EXTERNAL_SIGNER], [test "$use_external_signer" = "yes"]) diff --git a/src/Makefile.am b/src/Makefile.am index f939980fabf2..f76c844973a4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -405,6 +405,7 @@ BITCOIN_CORE_H = \ util/sock.h \ util/string.h \ util/spanparsing.h \ + util/subprocess.hpp \ util/syserror.h \ util/system.h \ util/time.h \ diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index 8bd5febd5386..e5356490ef7f 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -12,39 +12,34 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#include +#include #endif // ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) { #ifdef ENABLE_EXTERNAL_SIGNER - namespace bp = boost::process; + namespace sp = subprocess; UniValue result_json; - bp::opstream stdin_stream; - bp::ipstream stdout_stream; - bp::ipstream stderr_stream; + std::istringstream stdout_stream; + std::istringstream stderr_stream; if (str_command.empty()) return UniValue::VNULL; - bp::child c( - str_command, - bp::std_out > stdout_stream, - bp::std_err > stderr_stream, - bp::std_in < stdin_stream - ); + auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}); if (!str_std_in.empty()) { - stdin_stream << str_std_in << std::endl; + c.send(str_std_in); } - stdin_stream.pipe().close(); + auto [out_res, err_res] = c.communicate(); + stdout_stream.str(std::string{out_res.buf.begin(), out_res.buf.end()}); + stderr_stream.str(std::string{err_res.buf.begin(), err_res.buf.end()}); std::string result; std::string error; std::getline(stdout_stream, result); std::getline(stderr_stream, error); - c.wait(); - const int n_error = c.exit_code(); + const int n_error = c.retcode(); if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 8f2d6601879c..9602335eea16 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -61,12 +61,12 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector #ifdef ENABLE_EXTERNAL_SIGNER -#include +#include #endif // ENABLE_EXTERNAL_SIGNER #include @@ -30,20 +30,16 @@ BOOST_AUTO_TEST_CASE(run_command) BOOST_CHECK(result.isNull()); } { - const UniValue result = RunCommandParseJSON("echo \"{\"success\": true}\""); + const UniValue result = RunCommandParseJSON("echo {\"success\": true}"); BOOST_CHECK(result.isObject()); const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); BOOST_CHECK_EQUAL(success.get_bool(), true); } { - // An invalid command is handled by Boost - const int expected_error{2}; - BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), boost::process::process_error, [&](const boost::process::process_error& e) { - BOOST_CHECK(std::string(e.what()).find("RunCommandParseJSON error:") == std::string::npos); - BOOST_CHECK_EQUAL(e.code().value(), expected_error); - return true; - }); + // An invalid command is handled by cpp-subprocess + const std::string expected{"execve failed: "}; + BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), subprocess::CalledProcessError, HasReason(expected)); } { // Return non-zero exit code, no output to stderr diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp new file mode 100644 index 000000000000..dd53a8fbb4fb --- /dev/null +++ b/src/util/subprocess.hpp @@ -0,0 +1,1986 @@ +/*! + +Documentation for C++ subprocessing libraray. + +@copyright The code is licensed under the [MIT + License](http://opensource.org/licenses/MIT): +
+ Copyright © 2016-2018 Arun Muralidharan. +
+ Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: +
+ The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. +
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + +@author [Arun Muralidharan] +@see https://github.com/arun11299/cpp-subprocess to download the source code + +@version 1.0.0 +*/ + +#ifndef SUBPROCESS_HPP +#define SUBPROCESS_HPP + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if (defined _MSC_VER) || (defined __MINGW32__) + #define __USING_WINDOWS__ +#endif + +#ifdef __USING_WINDOWS__ + #include +#endif + +extern "C" { +#ifdef __USING_WINDOWS__ + #include + #include + #include + + #define close _close + #define open _open + #define fileno _fileno +#else + #include + #include +#endif + #include + #include + #include +} + +/*! + * Getting started with reading this source code. + * The source is mainly divided into four parts: + * 1. Exception Classes: + * These are very basic exception classes derived from + * runtime_error exception. + * There are two types of exception thrown from subprocess + * library: OSError and CalledProcessError + * + * 2. Popen Class + * This is the main class the users will deal with. It + * provides with all the API's to deal with processes. + * + * 3. Util namespace + * It includes some helper functions to split/join a string, + * reading from file descriptors, waiting on a process, fcntl + * options on file descriptors etc. + * + * 4. Detail namespace + * This includes some metaprogramming and helper classes. + */ + + +namespace subprocess { + +// Max buffer size allocated on stack for read error +// from pipe +static const size_t SP_MAX_ERR_BUF_SIZ = 1024; + +// Default buffer capcity for OutBuffer and ErrBuffer. +// If the data exceeds this capacity, the buffer size is grown +// by 1.5 times its previous capacity +static const size_t DEFAULT_BUF_CAP_BYTES = 8192; + + +/*----------------------------------------------- + * EXCEPTION CLASSES + *----------------------------------------------- + */ + +/*! + * class: CalledProcessError + * Thrown when there was error executing the command. + * Check Popen class API's to know when this exception + * can be thrown. + * + */ +class CalledProcessError: public std::runtime_error +{ +public: + int retcode; + CalledProcessError(const std::string& error_msg, int retcode): + std::runtime_error(error_msg), retcode(retcode) + {} +}; + + +/*! + * class: OSError + * Thrown when some system call fails to execute or give result. + * The exception message contains the name of the failed system call + * with the stringisized errno code. + * Check Popen class API's to know when this exception would be + * thrown. + * Its usual that the API exception specification would have + * this exception together with CalledProcessError. + */ +class OSError: public std::runtime_error +{ +public: + OSError(const std::string& err_msg, int err_code): + std::runtime_error( err_msg + ": " + std::strerror(err_code) ) + {} +}; + +//-------------------------------------------------------------------- + +//Environment Variable types +#ifndef _MSC_VER + using env_string_t = std::string; + using env_char_t = char; +#else + using env_string_t = std::wstring; + using env_char_t = wchar_t; +#endif +using env_map_t = std::map; +using env_vector_t = std::vector; + +//-------------------------------------------------------------------- +namespace util +{ + template + inline bool is_ready(std::shared_future const &f) + { + return f.wait_for(std::chrono::seconds(0)) == std::future_status::ready; + } + + inline void quote_argument(const std::wstring &argument, std::wstring &command_line, + bool force) + { + // + // Unless we're told otherwise, don't quote unless we actually + // need to do so --- hopefully avoid problems if programs won't + // parse quotes properly + // + + if (force == false && argument.empty() == false && + argument.find_first_of(L" \t\n\v\"") == argument.npos) { + command_line.append(argument); + } + else { + command_line.push_back(L'"'); + + for (auto it = argument.begin();; ++it) { + unsigned number_backslashes = 0; + + while (it != argument.end() && *it == L'\\') { + ++it; + ++number_backslashes; + } + + if (it == argument.end()) { + + // + // Escape all backslashes, but let the terminating + // double quotation mark we add below be interpreted + // as a metacharacter. + // + + command_line.append(number_backslashes * 2, L'\\'); + break; + } + else if (*it == L'"') { + + // + // Escape all backslashes and the following + // double quotation mark. + // + + command_line.append(number_backslashes * 2 + 1, L'\\'); + command_line.push_back(*it); + } + else { + + // + // Backslashes aren't special here. + // + + command_line.append(number_backslashes, L'\\'); + command_line.push_back(*it); + } + } + + command_line.push_back(L'"'); + } + } + +#ifdef __USING_WINDOWS__ + inline std::string get_last_error(DWORD errorMessageID) + { + if (errorMessageID == 0) + return std::string(); + + LPSTR messageBuffer = nullptr; + size_t size = FormatMessageA( + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_MAX_WIDTH_MASK, + NULL, errorMessageID, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPSTR)&messageBuffer, 0, NULL); + + std::string message(messageBuffer, size); + + LocalFree(messageBuffer); + + return message; + } + + inline FILE *file_from_handle(HANDLE h, const char *mode) + { + int md; + if (!mode) { + throw OSError("invalid_mode", 0); + } + + if (mode[0] == 'w') { + md = _O_WRONLY; + } + else if (mode[0] == 'r') { + md = _O_RDONLY; + } + else { + throw OSError("file_from_handle", 0); + } + + int os_fhandle = _open_osfhandle((intptr_t)h, md); + if (os_fhandle == -1) { + CloseHandle(h); + throw OSError("_open_osfhandle", 0); + } + + FILE *fp = _fdopen(os_fhandle, mode); + if (fp == 0) { + _close(os_fhandle); + throw OSError("_fdopen", 0); + } + + return fp; + } + + inline void configure_pipe(HANDLE* read_handle, HANDLE* write_handle, HANDLE* child_handle) + { + SECURITY_ATTRIBUTES saAttr; + + // Set the bInheritHandle flag so pipe handles are inherited. + saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); + saAttr.bInheritHandle = TRUE; + saAttr.lpSecurityDescriptor = NULL; + + // Create a pipe for the child process's STDIN. + if (!CreatePipe(read_handle, write_handle, &saAttr,0)) + throw OSError("CreatePipe", 0); + + // Ensure the write handle to the pipe for STDIN is not inherited. + if (!SetHandleInformation(*child_handle, HANDLE_FLAG_INHERIT, 0)) + throw OSError("SetHandleInformation", 0); + } + + // env_map_t MapFromWindowsEnvironment() + // * Imports current Environment in a C-string table + // * Parses the strings by splitting on the first "=" per line + // * Creates a map of the variables + // * Returns the map + inline env_map_t MapFromWindowsEnvironment(){ + wchar_t *variable_strings_ptr; + wchar_t *environment_strings_ptr; + std::wstring delimeter(L"="); + int del_len = delimeter.length(); + env_map_t mapped_environment; + + // Get a pointer to the environment block. + environment_strings_ptr = GetEnvironmentStringsW(); + // If the returned pointer is NULL, exit. + if (environment_strings_ptr == NULL) + { + throw OSError("GetEnvironmentStringsW", 0); + } + + // Variable strings are separated by NULL byte, and the block is + // terminated by a NULL byte. + + variable_strings_ptr = (wchar_t *) environment_strings_ptr; + + //Since the environment map ends with a null, we can loop until we find it. + while (*variable_strings_ptr) + { + // Create a string from Variable String + env_string_t current_line(variable_strings_ptr); + // Find the first "equals" sign. + auto pos = current_line.find(delimeter); + // Assuming it's not missing ... + if(pos!=std::wstring::npos){ + // ... parse the key and value. + env_string_t key = current_line.substr(0, pos); + env_string_t value = current_line.substr(pos + del_len); + // Map the entry. + mapped_environment[key] = value; + } + // Jump to next line in the environment map. + variable_strings_ptr += std::wcslen(variable_strings_ptr) + 1; + } + // We're done with the old environment map buffer. + FreeEnvironmentStringsW(environment_strings_ptr); + + // Return the map. + return mapped_environment; + } + + // env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) + // * Creates a vector buffer for the new environment string table + // * Copies in the mapped variables + // * Returns the vector + inline env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) + { + // Make a new environment map buffer. + env_vector_t environment_map_buffer; + // Give it some space. + environment_map_buffer.reserve(4096); + + // And fill'er up. + for(auto kv: source_map){ + // Create the line + env_string_t current_line(kv.first); current_line += L"="; current_line += kv.second; + // Add the line to the buffer. + std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer)); + // Append a null + environment_map_buffer.push_back(0); + } + // Append one last null because of how Windows does it's environment maps. + environment_map_buffer.push_back(0); + + return environment_map_buffer; + } + + // env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map) + // * Merges host environment with new mapped variables + // * Creates and returns string vector based on map + inline env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map){ + // Import the environment map + env_map_t environment_map = MapFromWindowsEnvironment(); + // Merge in the changes with overwrite + for(auto& it: changes_map) + { + environment_map[it.first] = it.second; + } + // Create a Windows-usable Environment Map Buffer + env_vector_t environment_map_strings_vector = WindowsEnvironmentVectorFromMap(environment_map); + + return environment_map_strings_vector; + } + +#endif + + /*! + * Function: split + * Parameters: + * [in] str : Input string which needs to be split based upon the + * delimiters provided. + * [in] deleims : Delimiter characters based upon which the string needs + * to be split. Default constructed to ' '(space) and '\t'(tab) + * [out] vector : Vector of strings split at deleimiter. + */ + static inline std::vector + split(const std::string& str, const std::string& delims=" \t") + { + std::vector res; + size_t init = 0; + + while (true) { + auto pos = str.find_first_of(delims, init); + if (pos == std::string::npos) { + res.emplace_back(str.substr(init, str.length())); + break; + } + res.emplace_back(str.substr(init, pos - init)); + pos++; + init = pos; + } + + return res; + } + + + /*! + * Function: join + * Parameters: + * [in] vec : Vector of strings which needs to be joined to form + * a single string with words seperated by a seperator char. + * [in] sep : String used to seperate 2 words in the joined string. + * Default constructed to ' ' (space). + * [out] string: Joined string. + */ + static inline + std::string join(const std::vector& vec, + const std::string& sep = " ") + { + std::string res; + for (auto& elem : vec) res.append(elem + sep); + res.erase(--res.end()); + return res; + } + + +#ifndef __USING_WINDOWS__ + /*! + * Function: set_clo_on_exec + * Sets/Resets the FD_CLOEXEC flag on the provided file descriptor + * based upon the `set` parameter. + * Parameters: + * [in] fd : The descriptor on which FD_CLOEXEC needs to be set/reset. + * [in] set : If 'true', set FD_CLOEXEC. + * If 'false' unset FD_CLOEXEC. + */ + static inline + void set_clo_on_exec(int fd, bool set = true) + { + int flags = fcntl(fd, F_GETFD, 0); + if (set) flags |= FD_CLOEXEC; + else flags &= ~FD_CLOEXEC; + //TODO: should check for errors + fcntl(fd, F_SETFD, flags); + } + + + /*! + * Function: pipe_cloexec + * Creates a pipe and sets FD_CLOEXEC flag on both + * read and write descriptors of the pipe. + * Parameters: + * [out] : A pair of file descriptors. + * First element of pair is the read descriptor of pipe. + * Second element is the write descriptor of pipe. + */ + static inline + std::pair pipe_cloexec() noexcept(false) + { + int pipe_fds[2]; + int res = pipe(pipe_fds); + if (res) { + throw OSError("pipe failure", errno); + } + + set_clo_on_exec(pipe_fds[0]); + set_clo_on_exec(pipe_fds[1]); + + return std::make_pair(pipe_fds[0], pipe_fds[1]); + } +#endif + + + /*! + * Function: write_n + * Writes `length` bytes to the file descriptor `fd` + * from the buffer `buf`. + * Parameters: + * [in] fd : The file descriptotr to write to. + * [in] buf: Buffer from which data needs to be written to fd. + * [in] length: The number of bytes that needs to be written from + * `buf` to `fd`. + * [out] int : Number of bytes written or -1 in case of failure. + */ + static inline + int write_n(int fd, const char* buf, size_t length) + { + size_t nwritten = 0; + while (nwritten < length) { + int written = write(fd, buf + nwritten, length - nwritten); + if (written == -1) return -1; + nwritten += written; + } + return nwritten; + } + + + /*! + * Function: read_atmost_n + * Reads at the most `read_upto` bytes from the + * file object `fp` before returning. + * Parameters: + * [in] fp : The file object from which it needs to read. + * [in] buf : The buffer into which it needs to write the data. + * [in] read_upto: Max number of bytes which must be read from `fd`. + * [out] int : Number of bytes written to `buf` or read from `fd` + * OR -1 in case of error. + * NOTE: In case of EINTR while reading from socket, this API + * will retry to read from `fd`, but only till the EINTR counter + * reaches 50 after which it will return with whatever data it read. + */ + static inline + int read_atmost_n(FILE* fp, char* buf, size_t read_upto) + { +#ifdef __USING_WINDOWS__ + return (int)fread(buf, 1, read_upto, fp); +#else + int fd = fileno(fp); + int rbytes = 0; + int eintr_cnter = 0; + + while (1) { + int read_bytes = read(fd, buf + rbytes, read_upto - rbytes); + if (read_bytes == -1) { + if (errno == EINTR) { + if (eintr_cnter >= 50) return -1; + eintr_cnter++; + continue; + } + return -1; + } + if (read_bytes == 0) return rbytes; + + rbytes += read_bytes; + } + return rbytes; +#endif + } + + + /*! + * Function: read_all + * Reads all the available data from `fp` into + * `buf`. Internally calls read_atmost_n. + * Parameters: + * [in] fp : The file object from which to read from. + * [in] buf : The buffer of type `class Buffer` into which + * the read data is written to. + * [out] int: Number of bytes read OR -1 in case of failure. + * + * NOTE: `class Buffer` is a exposed public class. See below. + */ + + static inline int read_all(FILE* fp, std::vector& buf) + { + auto buffer = buf.data(); + int total_bytes_read = 0; + int fill_sz = buf.size(); + + while (1) { + const int rd_bytes = read_atmost_n(fp, buffer, fill_sz); + + if (rd_bytes == -1) { // Read finished + if (total_bytes_read == 0) return -1; + break; + + } else if (rd_bytes == fill_sz) { // Buffer full + const auto orig_sz = buf.size(); + const auto new_sz = orig_sz * 2; + buf.resize(new_sz); + fill_sz = new_sz - orig_sz; + + //update the buffer pointer + buffer = buf.data(); + total_bytes_read += rd_bytes; + buffer += total_bytes_read; + + } else { // Partial data ? Continue reading + total_bytes_read += rd_bytes; + fill_sz -= rd_bytes; + break; + } + } + buf.erase(buf.begin()+total_bytes_read, buf.end()); // remove extra nulls + return total_bytes_read; + } + +#ifndef __USING_WINDOWS__ + /*! + * Function: wait_for_child_exit + * Waits for the process with pid `pid` to exit + * and returns its status. + * Parameters: + * [in] pid : The pid of the process. + * [out] pair: + * pair.first : Return code of the waitpid call. + * pair.second : Exit status of the process. + * + * NOTE: This is a blocking call as in, it will loop + * till the child is exited. + */ + static inline + std::pair wait_for_child_exit(int pid) + { + int status = 0; + int ret = -1; + while (1) { + ret = waitpid(pid, &status, 0); + if (ret == -1) break; + if (ret == 0) continue; + return std::make_pair(ret, status); + } + + return std::make_pair(ret, status); + } +#endif + +} // end namespace util + + + +/* ------------------------------- + * Popen Arguments + * ------------------------------- + */ + +/*! + * The buffer size of the stdin/stdout/stderr + * streams of the child process. + * Default value is 0. + */ +struct bufsize { + explicit bufsize(int siz): bufsiz(siz) {} + int bufsiz = 0; +}; + +/*! + * Option to defer spawning of the child process + * till `Popen::start_process` API is called. + * Default value is false. + */ +struct defer_spawn { + explicit defer_spawn(bool d): defer(d) {} + bool defer = false; +}; + +/*! + * Option to close all file descriptors + * when the child process is spawned. + * The close fd list does not include + * input/output/error if they are explicitly + * set as part of the Popen arguments. + * + * Default value is false. + */ +struct close_fds { + explicit close_fds(bool c): close_all(c) {} + bool close_all = false; +}; + +/*! + * Option to make the child process as the + * session leader and thus the process + * group leader. + * Default value is false. + */ +struct session_leader { + explicit session_leader(bool sl): leader_(sl) {} + bool leader_ = false; +}; + +struct shell { + explicit shell(bool s): shell_(s) {} + bool shell_ = false; +}; + +/*! + * Base class for all arguments involving string value. + */ +struct string_arg +{ + string_arg(const char* arg): arg_value(arg) {} + string_arg(std::string&& arg): arg_value(std::move(arg)) {} + string_arg(std::string arg): arg_value(std::move(arg)) {} + std::string arg_value; +}; + +/*! + * Option to specify the executable name seperately + * from the args sequence. + * In this case the cmd args must only contain the + * options required for this executable. + * + * Eg: executable{"ls"} + */ +struct executable: string_arg +{ + template + executable(T&& arg): string_arg(std::forward(arg)) {} +}; + +/*! + * Option to set the current working directory + * of the spawned process. + * + * Eg: cwd{"/som/path/x"} + */ +struct cwd: string_arg +{ + template + cwd(T&& arg): string_arg(std::forward(arg)) {} +}; + +/*! + * Option to specify environment variables required by + * the spawned process. + * + * Eg: environment{{ {"K1", "V1"}, {"K2", "V2"},... }} + */ +struct environment +{ + environment(env_map_t&& env): + env_(std::move(env)) {} + explicit environment(const env_map_t& env): + env_(env) {} + env_map_t env_; +}; + + +/*! + * Used for redirecting input/output/error + */ +enum IOTYPE { + STDOUT = 1, + STDERR, + PIPE, +}; + +//TODO: A common base/interface for below stream structures ?? + +/*! + * Option to specify the input channel for the child + * process. It can be: + * 1. An already open file descriptor. + * 2. A file name. + * 3. IOTYPE. Usual a PIPE + * + * Eg: input{PIPE} + * OR in case of redirection, output of another Popen + * input{popen.output()} + */ +struct input +{ + // For an already existing file descriptor. + explicit input(int fd): rd_ch_(fd) {} + + // FILE pointer. + explicit input (FILE* fp):input(fileno(fp)) { assert(fp); } + + explicit input(const char* filename) { + int fd = open(filename, O_RDONLY); + if (fd == -1) throw OSError("File not found: ", errno); + rd_ch_ = fd; + } + explicit input(IOTYPE typ) { + assert (typ == PIPE && "STDOUT/STDERR not allowed"); +#ifndef __USING_WINDOWS__ + std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); +#endif + } + + int rd_ch_ = -1; + int wr_ch_ = -1; +}; + + +/*! + * Option to specify the output channel for the child + * process. It can be: + * 1. An already open file descriptor. + * 2. A file name. + * 3. IOTYPE. Usually a PIPE. + * + * Eg: output{PIPE} + * OR output{"output.txt"} + */ +struct output +{ + explicit output(int fd): wr_ch_(fd) {} + + explicit output (FILE* fp):output(fileno(fp)) { assert(fp); } + + explicit output(const char* filename) { + int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640); + if (fd == -1) throw OSError("File not found: ", errno); + wr_ch_ = fd; + } + explicit output(IOTYPE typ) { + assert (typ == PIPE && "STDOUT/STDERR not allowed"); +#ifndef __USING_WINDOWS__ + std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); +#endif + } + + int rd_ch_ = -1; + int wr_ch_ = -1; +}; + + +/*! + * Option to specify the error channel for the child + * process. It can be: + * 1. An already open file descriptor. + * 2. A file name. + * 3. IOTYPE. Usually a PIPE or STDOUT + * + */ +struct error +{ + explicit error(int fd): wr_ch_(fd) {} + + explicit error(FILE* fp):error(fileno(fp)) { assert(fp); } + + explicit error(const char* filename) { + int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640); + if (fd == -1) throw OSError("File not found: ", errno); + wr_ch_ = fd; + } + explicit error(IOTYPE typ) { + assert ((typ == PIPE || typ == STDOUT) && "STDERR not aloowed"); + if (typ == PIPE) { +#ifndef __USING_WINDOWS__ + std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); +#endif + } else { + // Need to defer it till we have checked all arguments + deferred_ = true; + } + } + + bool deferred_ = false; + int rd_ch_ = -1; + int wr_ch_ = -1; +}; + +// Impoverished, meager, needy, truly needy +// version of type erasure to store function pointers +// needed to provide the functionality of preexec_func +// ATTN: Can be used only to execute functions with no +// arguments and returning void. +// Could have used more efficient methods, ofcourse, but +// that wont yield me the consistent syntax which I am +// aiming for. If you know, then please do let me know. + +class preexec_func +{ +public: + preexec_func() {} + + template + explicit preexec_func(Func f): holder_(new FuncHolder(std::move(f))) + {} + + void operator()() { + (*holder_)(); + } + +private: + struct HolderBase { + virtual void operator()() const = 0; + virtual ~HolderBase(){}; + }; + template + struct FuncHolder: HolderBase { + FuncHolder(T func): func_(std::move(func)) {} + void operator()() const override { func_(); } + // The function pointer/reference + T func_; + }; + + std::unique_ptr holder_ = nullptr; +}; + +// ~~~~ End Popen Args ~~~~ + + +/*! + * class: Buffer + * This class is a very thin wrapper around std::vector + * This is basically used to determine the length of the actual + * data stored inside the dynamically resized vector. + * + * This is what is returned as the output to communicate and check_output + * functions, so, users must know about this class. + * + * OutBuffer and ErrBuffer are just different typedefs to this class. + */ +class Buffer +{ +public: + Buffer() {} + explicit Buffer(size_t cap) { buf.resize(cap); } + void add_cap(size_t cap) { buf.resize(cap); } + +#if 0 + Buffer(const Buffer& other): + buf(other.buf), + length(other.length) + { + std::cout << "COPY" << std::endl; + } + + Buffer(Buffer&& other): + buf(std::move(other.buf)), + length(other.length) + { + std::cout << "MOVE" << std::endl; + } +#endif + +public: + std::vector buf; + size_t length = 0; +}; + +// Buffer for storing output written to output fd +using OutBuffer = Buffer; +// Buffer for storing output written to error fd +using ErrBuffer = Buffer; + + +// Fwd Decl. +class Popen; + +/*--------------------------------------------------- + * DETAIL NAMESPACE + *--------------------------------------------------- + */ + +namespace detail { + +// Metaprogram for searching a type within +// a variadic parameter pack +// This is particularly required to do a compile time +// checking of the arguments provided to 'check_ouput' function +// wherein the user is not expected to provide an 'ouput' option. + +template struct param_pack{}; + +template struct has_type; + +template +struct has_type> { + static constexpr bool value = false; +}; + +template +struct has_type> { + static constexpr bool value = true; +}; + +template +struct has_type> { + static constexpr bool value = + std::is_same::type>::value ? true : has_type>::value; +}; + +//---- + +/*! + * A helper class to Popen class for setting + * options as provided in the Popen constructor + * or in check_ouput arguments. + * This design allows us to _not_ have any fixed position + * to any arguments and specify them in a way similar to what + * can be done in python. + */ +struct ArgumentDeducer +{ + ArgumentDeducer(Popen* p): popen_(p) {} + + void set_option(executable&& exe); + void set_option(cwd&& cwdir); + void set_option(bufsize&& bsiz); + void set_option(environment&& env); + void set_option(defer_spawn&& defer); + void set_option(shell&& sh); + void set_option(input&& inp); + void set_option(output&& out); + void set_option(error&& err); + void set_option(close_fds&& cfds); + void set_option(preexec_func&& prefunc); + void set_option(session_leader&& sleader); + +private: + Popen* popen_ = nullptr; +}; + +/*! + * A helper class to Popen. + * This takes care of all the fork-exec logic + * in the execute_child API. + */ +class Child +{ +public: + Child(Popen* p, int err_wr_pipe): + parent_(p), + err_wr_pipe_(err_wr_pipe) + {} + + void execute_child(); + +private: + // Lets call it parent even though + // technically a bit incorrect + Popen* parent_ = nullptr; + int err_wr_pipe_ = -1; +}; + +// Fwd Decl. +class Streams; + +/*! + * A helper class to Streams. + * This takes care of management of communicating + * with the child process with the means of the correct + * file descriptor. + */ +class Communication +{ +public: + Communication(Streams* stream): stream_(stream) + {} + void operator=(const Communication&) = delete; +public: + int send(const char* msg, size_t length); + int send(const std::vector& msg); + + std::pair communicate(const char* msg, size_t length); + std::pair communicate(const std::vector& msg) + { return communicate(msg.data(), msg.size()); } + + void set_out_buf_cap(size_t cap) { out_buf_cap_ = cap; } + void set_err_buf_cap(size_t cap) { err_buf_cap_ = cap; } + +private: + std::pair communicate_threaded( + const char* msg, size_t length); + +private: + Streams* stream_; + size_t out_buf_cap_ = DEFAULT_BUF_CAP_BYTES; + size_t err_buf_cap_ = DEFAULT_BUF_CAP_BYTES; +}; + + + +/*! + * This is a helper class to Popen. + * It takes care of management of all the file descriptors + * and file pointers. + * It dispatches of the communication aspects to the + * Communication class. + * Read through the data members to understand about the + * various file descriptors used. + */ +class Streams +{ +public: + Streams():comm_(this) {} + void operator=(const Streams&) = delete; + +public: + void setup_comm_channels(); + + void cleanup_fds() + { + if (write_to_child_ != -1 && read_from_parent_ != -1) { + close(write_to_child_); + } + if (write_to_parent_ != -1 && read_from_child_ != -1) { + close(read_from_child_); + } + if (err_write_ != -1 && err_read_ != -1) { + close(err_read_); + } + } + + void close_parent_fds() + { + if (write_to_child_ != -1) close(write_to_child_); + if (read_from_child_ != -1) close(read_from_child_); + if (err_read_ != -1) close(err_read_); + } + + void close_child_fds() + { + if (write_to_parent_ != -1) close(write_to_parent_); + if (read_from_parent_ != -1) close(read_from_parent_); + if (err_write_ != -1) close(err_write_); + } + + FILE* input() { return input_.get(); } + FILE* output() { return output_.get(); } + FILE* error() { return error_.get(); } + + void input(FILE* fp) { input_.reset(fp, fclose); } + void output(FILE* fp) { output_.reset(fp, fclose); } + void error(FILE* fp) { error_.reset(fp, fclose); } + + void set_out_buf_cap(size_t cap) { comm_.set_out_buf_cap(cap); } + void set_err_buf_cap(size_t cap) { comm_.set_err_buf_cap(cap); } + +public: /* Communication forwarding API's */ + int send(const char* msg, size_t length) + { return comm_.send(msg, length); } + + int send(const std::vector& msg) + { return comm_.send(msg); } + + std::pair communicate(const char* msg, size_t length) + { return comm_.communicate(msg, length); } + + std::pair communicate(const std::vector& msg) + { return comm_.communicate(msg); } + + +public:// Yes they are public + + std::shared_ptr input_ = nullptr; + std::shared_ptr output_ = nullptr; + std::shared_ptr error_ = nullptr; + +#ifdef __USING_WINDOWS__ + HANDLE g_hChildStd_IN_Rd = nullptr; + HANDLE g_hChildStd_IN_Wr = nullptr; + HANDLE g_hChildStd_OUT_Rd = nullptr; + HANDLE g_hChildStd_OUT_Wr = nullptr; + HANDLE g_hChildStd_ERR_Rd = nullptr; + HANDLE g_hChildStd_ERR_Wr = nullptr; +#endif + + // Buffer size for the IO streams + int bufsiz_ = 0; + + // Pipes for communicating with child + + // Emulates stdin + int write_to_child_ = -1; // Parent owned descriptor + int read_from_parent_ = -1; // Child owned descriptor + + // Emulates stdout + int write_to_parent_ = -1; // Child owned descriptor + int read_from_child_ = -1; // Parent owned descriptor + + // Emulates stderr + int err_write_ = -1; // Write error to parent (Child owned) + int err_read_ = -1; // Read error from child (Parent owned) + +private: + Communication comm_; +}; + +} // end namespace detail + + + +/*! + * class: Popen + * This is the single most important class in the whole library + * and glues together all the helper classes to provide a common + * interface to the client. + * + * API's provided by the class: + * 1. Popen({"cmd"}, output{..}, error{..}, cwd{..}, ....) + * Command provided as a sequence. + * 2. Popen("cmd arg1"m output{..}, error{..}, cwd{..}, ....) + * Command provided in a single string. + * 3. wait() - Wait for the child to exit. + * 4. retcode() - The return code of the exited child. + * 5. pid() - PID of the spawned child. + * 6. poll() - Check the status of the running child. + * 7. kill(sig_num) - Kill the child. SIGTERM used by default. + * 8. send(...) - Send input to the input channel of the child. + * 9. communicate(...) - Get the output/error from the child and close the channels + * from the parent side. + *10. input() - Get the input channel/File pointer. Can be used for + * cutomizing the way of sending input to child. + *11. output() - Get the output channel/File pointer. Usually used + in case of redirection. See piping examples. + *12. error() - Get the error channel/File poiner. Usually used + in case of redirection. + *13. start_process() - Start the child process. Only to be used when + * `defer_spawn` option was provided in Popen constructor. + */ +class Popen +{ +public: + friend struct detail::ArgumentDeducer; + friend class detail::Child; + + template + Popen(const std::string& cmd_args, Args&& ...args): + args_(cmd_args) + { + vargs_ = util::split(cmd_args); + init_args(std::forward(args)...); + + // Setup the communication channels of the Popen class + stream_.setup_comm_channels(); + + if (!defer_process_start_) execute_process(); + } + + template + Popen(std::initializer_list cmd_args, Args&& ...args) + { + vargs_.insert(vargs_.end(), cmd_args.begin(), cmd_args.end()); + init_args(std::forward(args)...); + + // Setup the communication channels of the Popen class + stream_.setup_comm_channels(); + + if (!defer_process_start_) execute_process(); + } + + template + Popen(std::vector vargs_, Args &&... args) : vargs_(vargs_) + { + init_args(std::forward(args)...); + + // Setup the communication channels of the Popen class + stream_.setup_comm_channels(); + + if (!defer_process_start_) execute_process(); + } + +/* + ~Popen() + { +#ifdef __USING_WINDOWS__ + CloseHandle(this->process_handle_); +#endif + } +*/ + + void start_process() noexcept(false); + + int pid() const noexcept { return child_pid_; } + + int retcode() const noexcept { return retcode_; } + + int wait() noexcept(false); + + int poll() noexcept(false); + + // Does not fail, Caller is expected to recheck the + // status with a call to poll() + void kill(int sig_num = 9); + + void set_out_buf_cap(size_t cap) { stream_.set_out_buf_cap(cap); } + + void set_err_buf_cap(size_t cap) { stream_.set_err_buf_cap(cap); } + + int send(const char* msg, size_t length) + { return stream_.send(msg, length); } + + int send(const std::string& msg) + { return send(msg.c_str(), msg.size()); } + + int send(const std::vector& msg) + { return stream_.send(msg); } + + std::pair communicate(const char* msg, size_t length) + { + auto res = stream_.communicate(msg, length); + retcode_ = wait(); + return res; + } + + std::pair communicate(const std::string& msg) + { + return communicate(msg.c_str(), msg.size()); + } + + std::pair communicate(const std::vector& msg) + { + auto res = stream_.communicate(msg); + retcode_ = wait(); + return res; + } + + std::pair communicate() + { + return communicate(nullptr, 0); + } + + FILE* input() { return stream_.input(); } + FILE* output() { return stream_.output();} + FILE* error() { return stream_.error(); } + + /// Stream close APIs + void close_input() { stream_.input_.reset(); } + void close_output() { stream_.output_.reset(); } + void close_error() { stream_.error_.reset(); } + +private: + template + void init_args(F&& farg, Args&&... args); + void init_args(); + void populate_c_argv(); + void execute_process() noexcept(false); + +private: + detail::Streams stream_; + +#ifdef __USING_WINDOWS__ + HANDLE process_handle_; + std::future cleanup_future_; +#endif + + bool defer_process_start_ = false; + bool close_fds_ = false; + bool has_preexec_fn_ = false; + bool shell_ = false; + bool session_leader_ = false; + + std::string exe_name_; + std::string cwd_; + env_map_t env_; + preexec_func preexec_fn_; + + // Command in string format + std::string args_; + // Comamnd provided as sequence + std::vector vargs_; + std::vector cargv_; + + bool child_created_ = false; + // Pid of the child process + int child_pid_ = -1; + + int retcode_ = -1; +}; + +inline void Popen::init_args() { + populate_c_argv(); +} + +template +inline void Popen::init_args(F&& farg, Args&&... args) +{ + detail::ArgumentDeducer argd(this); + argd.set_option(std::forward(farg)); + init_args(std::forward(args)...); +} + +inline void Popen::populate_c_argv() +{ + cargv_.clear(); + cargv_.reserve(vargs_.size() + 1); + for (auto& arg : vargs_) cargv_.push_back(&arg[0]); + cargv_.push_back(nullptr); +} + +inline void Popen::start_process() noexcept(false) +{ + // The process was started/tried to be started + // in the constructor itself. + // For explicitly calling this API to start the + // process, 'defer_spawn' argument must be set to + // true in the constructor. + if (!defer_process_start_) { + assert (0); + return; + } + execute_process(); +} + +inline int Popen::wait() noexcept(false) +{ +#ifdef __USING_WINDOWS__ + int ret = WaitForSingleObject(process_handle_, INFINITE); + + return 0; +#else + int ret, status; + std::tie(ret, status) = util::wait_for_child_exit(pid()); + if (ret == -1) { + if (errno != ECHILD) throw OSError("waitpid failed", errno); + return 0; + } + if (WIFEXITED(status)) return WEXITSTATUS(status); + if (WIFSIGNALED(status)) return WTERMSIG(status); + else return 255; + + return 0; +#endif +} + +inline int Popen::poll() noexcept(false) +{ +#ifdef __USING_WINDOWS__ + int ret = WaitForSingleObject(process_handle_, 0); + if (ret != WAIT_OBJECT_0) return -1; + + DWORD dretcode_; + if (FALSE == GetExitCodeProcess(process_handle_, &dretcode_)) + throw OSError("GetExitCodeProcess", 0); + + retcode_ = (int)dretcode_; + CloseHandle(process_handle_); + + return retcode_; +#else + if (!child_created_) return -1; // TODO: ?? + + int status; + + // Returns zero if child is still running + int ret = waitpid(child_pid_, &status, WNOHANG); + if (ret == 0) return -1; + + if (ret == child_pid_) { + if (WIFSIGNALED(status)) { + retcode_ = WTERMSIG(status); + } else if (WIFEXITED(status)) { + retcode_ = WEXITSTATUS(status); + } else { + retcode_ = 255; + } + return retcode_; + } + + if (ret == -1) { + // From subprocess.py + // This happens if SIGCHLD is set to be ignored + // or waiting for child process has otherwise been disabled + // for our process. This child is dead, we cannot get the + // status. + if (errno == ECHILD) retcode_ = 0; + else throw OSError("waitpid failed", errno); + } else { + retcode_ = ret; + } + + return retcode_; +#endif +} + +inline void Popen::kill(int sig_num) +{ +#ifdef __USING_WINDOWS__ + if (!TerminateProcess(this->process_handle_, (UINT)sig_num)) { + throw OSError("TerminateProcess", 0); + } +#else + if (session_leader_) killpg(child_pid_, sig_num); + else ::kill(child_pid_, sig_num); +#endif +} + + +inline void Popen::execute_process() noexcept(false) +{ +#ifdef __USING_WINDOWS__ + if (this->shell_) { + throw OSError("shell not currently supported on windows", 0); + } + + void* environment_string_table_ptr = nullptr; + env_vector_t environment_string_vector; + if(this->env_.size()){ + environment_string_vector = util::CreateUpdatedWindowsEnvironmentVector(env_); + environment_string_table_ptr = (void*)environment_string_vector.data(); + } + + if (exe_name_.length()) { + this->vargs_.insert(this->vargs_.begin(), this->exe_name_); + this->populate_c_argv(); + } + this->exe_name_ = vargs_[0]; + + std::wstring_convert> converter; + std::wstring argument; + std::wstring command_line; + + for (auto arg : this->vargs_) { + argument = converter.from_bytes(arg); + util::quote_argument(argument, command_line, false); + command_line += L" "; + } + + // CreateProcessW can modify szCmdLine so we allocate needed memory + wchar_t *szCmdline = new wchar_t[command_line.size() + 1]; + wcscpy_s(szCmdline, command_line.size() + 1, command_line.c_str()); + PROCESS_INFORMATION piProcInfo; + STARTUPINFOW siStartInfo; + BOOL bSuccess = FALSE; + DWORD creation_flags = CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW; + + // Set up members of the PROCESS_INFORMATION structure. + ZeroMemory(&piProcInfo, sizeof(PROCESS_INFORMATION)); + + // Set up members of the STARTUPINFOW structure. + // This structure specifies the STDIN and STDOUT handles for redirection. + + ZeroMemory(&siStartInfo, sizeof(STARTUPINFOW)); + siStartInfo.cb = sizeof(STARTUPINFOW); + + siStartInfo.hStdError = this->stream_.g_hChildStd_ERR_Wr; + siStartInfo.hStdOutput = this->stream_.g_hChildStd_OUT_Wr; + siStartInfo.hStdInput = this->stream_.g_hChildStd_IN_Rd; + + siStartInfo.dwFlags |= STARTF_USESTDHANDLES; + + // Create the child process. + bSuccess = CreateProcessW(NULL, + szCmdline, // command line + NULL, // process security attributes + NULL, // primary thread security attributes + TRUE, // handles are inherited + creation_flags, // creation flags + environment_string_table_ptr, // use provided environment + NULL, // use parent's current directory + &siStartInfo, // STARTUPINFOW pointer + &piProcInfo); // receives PROCESS_INFORMATION + + // If an error occurs, exit the application. + if (!bSuccess) { + DWORD errorMessageID = ::GetLastError(); + throw CalledProcessError("CreateProcess failed: " + util::get_last_error(errorMessageID), errorMessageID); + } + + CloseHandle(piProcInfo.hThread); + + /* + TODO: use common apis to close linux handles + */ + + this->process_handle_ = piProcInfo.hProcess; + + this->cleanup_future_ = std::async(std::launch::async, [this] { + WaitForSingleObject(this->process_handle_, INFINITE); + + CloseHandle(this->stream_.g_hChildStd_ERR_Wr); + CloseHandle(this->stream_.g_hChildStd_OUT_Wr); + CloseHandle(this->stream_.g_hChildStd_IN_Rd); + }); + +/* + NOTE: In the linux version, there is a check to make sure that the process + has been started. Here, we do nothing because CreateProcess will throw + if we fail to create the process. +*/ + + +#else + + int err_rd_pipe, err_wr_pipe; + std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec(); + + if (shell_) { + auto new_cmd = util::join(vargs_); + vargs_.clear(); + vargs_.insert(vargs_.begin(), {"/bin/sh", "-c"}); + vargs_.push_back(new_cmd); + populate_c_argv(); + } + + if (exe_name_.length()) { + vargs_.insert(vargs_.begin(), exe_name_); + populate_c_argv(); + } + exe_name_ = vargs_[0]; + + child_pid_ = fork(); + + if (child_pid_ < 0) { + close(err_rd_pipe); + close(err_wr_pipe); + throw OSError("fork failed", errno); + } + + child_created_ = true; + + if (child_pid_ == 0) + { + // Close descriptors belonging to parent + stream_.close_parent_fds(); + + //Close the read end of the error pipe + close(err_rd_pipe); + + detail::Child chld(this, err_wr_pipe); + chld.execute_child(); + } + else + { + close (err_wr_pipe);// close child side of pipe, else get stuck in read below + + stream_.close_child_fds(); + + try { + char err_buf[SP_MAX_ERR_BUF_SIZ] = {0,}; + + int read_bytes = util::read_atmost_n( + fdopen(err_rd_pipe, "r"), + err_buf, + SP_MAX_ERR_BUF_SIZ); + close(err_rd_pipe); + + if (read_bytes || strlen(err_buf)) { + // Call waitpid to reap the child process + // waitpid suspends the calling process until the + // child terminates. + int retcode = wait(); + + // Throw whatever information we have about child failure + throw CalledProcessError(err_buf, retcode); + } + } catch (std::exception& exp) { + stream_.cleanup_fds(); + throw; + } + + } +#endif +} + +namespace detail { + + inline void ArgumentDeducer::set_option(executable&& exe) { + popen_->exe_name_ = std::move(exe.arg_value); + } + + inline void ArgumentDeducer::set_option(cwd&& cwdir) { + popen_->cwd_ = std::move(cwdir.arg_value); + } + + inline void ArgumentDeducer::set_option(bufsize&& bsiz) { + popen_->stream_.bufsiz_ = bsiz.bufsiz; + } + + inline void ArgumentDeducer::set_option(environment&& env) { + popen_->env_ = std::move(env.env_); + } + + inline void ArgumentDeducer::set_option(defer_spawn&& defer) { + popen_->defer_process_start_ = defer.defer; + } + + inline void ArgumentDeducer::set_option(shell&& sh) { + popen_->shell_ = sh.shell_; + } + + inline void ArgumentDeducer::set_option(session_leader&& sleader) { + popen_->session_leader_ = sleader.leader_; + } + + inline void ArgumentDeducer::set_option(input&& inp) { + if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_; + if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_; + } + + inline void ArgumentDeducer::set_option(output&& out) { + if (out.wr_ch_ != -1) popen_->stream_.write_to_parent_ = out.wr_ch_; + if (out.rd_ch_ != -1) popen_->stream_.read_from_child_ = out.rd_ch_; + } + + inline void ArgumentDeducer::set_option(error&& err) { + if (err.deferred_) { + if (popen_->stream_.write_to_parent_) { + popen_->stream_.err_write_ = popen_->stream_.write_to_parent_; + } else { + throw std::runtime_error("Set output before redirecting error to output"); + } + } + if (err.wr_ch_ != -1) popen_->stream_.err_write_ = err.wr_ch_; + if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; + } + + inline void ArgumentDeducer::set_option(close_fds&& cfds) { + popen_->close_fds_ = cfds.close_all; + } + + inline void ArgumentDeducer::set_option(preexec_func&& prefunc) { + popen_->preexec_fn_ = std::move(prefunc); + popen_->has_preexec_fn_ = true; + } + + + inline void Child::execute_child() { +#ifndef __USING_WINDOWS__ + int sys_ret = -1; + auto& stream = parent_->stream_; + + try { + if (stream.write_to_parent_ == 0) + stream.write_to_parent_ = dup(stream.write_to_parent_); + + if (stream.err_write_ == 0 || stream.err_write_ == 1) + stream.err_write_ = dup(stream.err_write_); + + // Make the child owned descriptors as the + // stdin, stdout and stderr for the child process + auto _dup2_ = [](int fd, int to_fd) { + if (fd == to_fd) { + // dup2 syscall does not reset the + // CLOEXEC flag if the descriptors + // provided to it are same. + // But, we need to reset the CLOEXEC + // flag as the provided descriptors + // are now going to be the standard + // input, output and error + util::set_clo_on_exec(fd, false); + } else if(fd != -1) { + int res = dup2(fd, to_fd); + if (res == -1) throw OSError("dup2 failed", errno); + } + }; + + // Create the standard streams + _dup2_(stream.read_from_parent_, 0); // Input stream + _dup2_(stream.write_to_parent_, 1); // Output stream + _dup2_(stream.err_write_, 2); // Error stream + + // Close the duped descriptors + if (stream.read_from_parent_ != -1 && stream.read_from_parent_ > 2) + close(stream.read_from_parent_); + + if (stream.write_to_parent_ != -1 && stream.write_to_parent_ > 2) + close(stream.write_to_parent_); + + if (stream.err_write_ != -1 && stream.err_write_ > 2) + close(stream.err_write_); + + // Close all the inherited fd's except the error write pipe + if (parent_->close_fds_) { + int max_fd = sysconf(_SC_OPEN_MAX); + if (max_fd == -1) throw OSError("sysconf failed", errno); + + for (int i = 3; i < max_fd; i++) { + if (i == err_wr_pipe_) continue; + close(i); + } + } + + // Change the working directory if provided + if (parent_->cwd_.length()) { + sys_ret = chdir(parent_->cwd_.c_str()); + if (sys_ret == -1) throw OSError("chdir failed", errno); + } + + if (parent_->has_preexec_fn_) { + parent_->preexec_fn_(); + } + + if (parent_->session_leader_) { + sys_ret = setsid(); + if (sys_ret == -1) throw OSError("setsid failed", errno); + } + + // Replace the current image with the executable + if (parent_->env_.size()) { + for (auto& kv : parent_->env_) { + setenv(kv.first.c_str(), kv.second.c_str(), 1); + } + sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); + } else { + sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); + } + + if (sys_ret == -1) throw OSError("execve failed", errno); + + } catch (const OSError& exp) { + // Just write the exception message + // TODO: Give back stack trace ? + std::string err_msg(exp.what()); + //ATTN: Can we do something on error here ? + util::write_n(err_wr_pipe_, err_msg.c_str(), err_msg.length()); + } + + // Calling application would not get this + // exit failure + _exit (EXIT_FAILURE); +#endif + } + + + inline void Streams::setup_comm_channels() + { +#ifdef __USING_WINDOWS__ + util::configure_pipe(&this->g_hChildStd_IN_Rd, &this->g_hChildStd_IN_Wr, &this->g_hChildStd_IN_Wr); + this->input(util::file_from_handle(this->g_hChildStd_IN_Wr, "w")); + this->write_to_child_ = _fileno(this->input()); + + util::configure_pipe(&this->g_hChildStd_OUT_Rd, &this->g_hChildStd_OUT_Wr, &this->g_hChildStd_OUT_Rd); + this->output(util::file_from_handle(this->g_hChildStd_OUT_Rd, "r")); + this->read_from_child_ = _fileno(this->output()); + + util::configure_pipe(&this->g_hChildStd_ERR_Rd, &this->g_hChildStd_ERR_Wr, &this->g_hChildStd_ERR_Rd); + this->error(util::file_from_handle(this->g_hChildStd_ERR_Rd, "r")); + this->err_read_ = _fileno(this->error()); +#else + + if (write_to_child_ != -1) input(fdopen(write_to_child_, "wb")); + if (read_from_child_ != -1) output(fdopen(read_from_child_, "rb")); + if (err_read_ != -1) error(fdopen(err_read_, "rb")); + + auto handles = {input(), output(), error()}; + + for (auto& h : handles) { + if (h == nullptr) continue; + switch (bufsiz_) { + case 0: + setvbuf(h, nullptr, _IONBF, BUFSIZ); + break; + case 1: + setvbuf(h, nullptr, _IONBF, BUFSIZ); + break; + default: + setvbuf(h, nullptr, _IOFBF, bufsiz_); + }; + } + #endif + } + + inline int Communication::send(const char* msg, size_t length) + { + if (stream_->input() == nullptr) return -1; + return std::fwrite(msg, sizeof(char), length, stream_->input()); + } + + inline int Communication::send(const std::vector& msg) + { + return send(msg.data(), msg.size()); + } + + inline std::pair + Communication::communicate(const char* msg, size_t length) + { + // Optimization from subprocess.py + // If we are using one pipe, or no pipe + // at all, using select() or threads is unnecessary. + auto hndls = {stream_->input(), stream_->output(), stream_->error()}; + int count = std::count(std::begin(hndls), std::end(hndls), nullptr); + const int len_conv = length; + + if (count >= 2) { + OutBuffer obuf; + ErrBuffer ebuf; + if (stream_->input()) { + if (msg) { + int wbytes = std::fwrite(msg, sizeof(char), length, stream_->input()); + if (wbytes < len_conv) { + if (errno != EPIPE && errno != EINVAL) { + throw OSError("fwrite error", errno); + } + } + } + // Close the input stream + stream_->input_.reset(); + } else if (stream_->output()) { + // Read till EOF + // ATTN: This could be blocking, if the process + // at the other end screws up, we get screwed as well + obuf.add_cap(out_buf_cap_); + + int rbytes = util::read_all( + stream_->output(), + obuf.buf); + + if (rbytes == -1) { + throw OSError("read to obuf failed", errno); + } + + obuf.length = rbytes; + // Close the output stream + stream_->output_.reset(); + + } else if (stream_->error()) { + // Same screwness applies here as well + ebuf.add_cap(err_buf_cap_); + + int rbytes = util::read_atmost_n( + stream_->error(), + ebuf.buf.data(), + ebuf.buf.size()); + + if (rbytes == -1) { + throw OSError("read to ebuf failed", errno); + } + + ebuf.length = rbytes; + // Close the error stream + stream_->error_.reset(); + } + return std::make_pair(std::move(obuf), std::move(ebuf)); + } + + return communicate_threaded(msg, length); + } + + + inline std::pair + Communication::communicate_threaded(const char* msg, size_t length) + { + OutBuffer obuf; + ErrBuffer ebuf; + std::future out_fut, err_fut; + const int length_conv = length; + + if (stream_->output()) { + obuf.add_cap(out_buf_cap_); + + out_fut = std::async(std::launch::async, + [&obuf, this] { + return util::read_all(this->stream_->output(), obuf.buf); + }); + } + if (stream_->error()) { + ebuf.add_cap(err_buf_cap_); + + err_fut = std::async(std::launch::async, + [&ebuf, this] { + return util::read_all(this->stream_->error(), ebuf.buf); + }); + } + if (stream_->input()) { + if (msg) { + int wbytes = std::fwrite(msg, sizeof(char), length, stream_->input()); + if (wbytes < length_conv) { + if (errno != EPIPE && errno != EINVAL) { + throw OSError("fwrite error", errno); + } + } + } + stream_->input_.reset(); + } + + if (out_fut.valid()) { + int res = out_fut.get(); + if (res != -1) obuf.length = res; + else obuf.length = 0; + } + if (err_fut.valid()) { + int res = err_fut.get(); + if (res != -1) ebuf.length = res; + else ebuf.length = 0; + } + + return std::make_pair(std::move(obuf), std::move(ebuf)); + } + +} // end namespace detail + +} + +#endif // SUBPROCESS_HPP diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index 4a2ef0a88fad..5169f5fc685f 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -34,7 +34,6 @@ "boost/multi_index/tag.hpp", "boost/multi_index_container.hpp", "boost/pool/pool_alloc.hpp", - "boost/process.hpp", "boost/signals2/connection.hpp", "boost/signals2/optional_last_value.hpp", "boost/signals2/signal.hpp", diff --git a/test/lint/lint-spelling.py b/test/lint/lint-spelling.py index 204d297cedf0..1acc5998d34a 100755 --- a/test/lint/lint-spelling.py +++ b/test/lint/lint-spelling.py @@ -12,7 +12,7 @@ from subprocess import check_output, STDOUT, CalledProcessError IGNORE_WORDS_FILE = 'test/lint/spelling.ignore-words.txt' -FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/bip39_english.h", ":(exclude)src/dashbls/", ":(exclude)src/crc32c/", ":(exclude)src/crypto/", ":(exclude)src/ctpl_stl.h", ":(exclude)src/cxxtimer.hpp", ":(exclude)src/immer/", ":(exclude)src/leveldb/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)src/secp256k1/", ":(exclude)src/minisketch/", ":(exclude)contrib/builder-keys/", ":(exclude)contrib/guix/patches"] +FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/bip39_english.h", ":(exclude)src/dashbls/", ":(exclude)src/crc32c/", ":(exclude)src/crypto/", ":(exclude)src/ctpl_stl.h", ":(exclude)src/cxxtimer.hpp", ":(exclude)src/immer/", ":(exclude)src/leveldb/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)src/secp256k1/", ":(exclude)src/minisketch/", ":(exclude)contrib/builder-keys/", ":(exclude)contrib/guix/patches", ":(exclude)src/util/subprocess.hpp"] def check_codespell_install():