Merge #15382: util: add RunCommandParseJSON

31cf68a3ad [util] add RunCommandParseJSON (Sjors Provoost)
c17f54ee53 [ci] use boost::process (Sjors Provoost)
32128ba682 [doc] include Doxygen comments for HAVE_BOOST_PROCESS (Sjors Provoost)
3c84d85f7d [build] msvc: add boost::process (Sjors Provoost)
c47e4bbf0b [build] make boost-process opt-in (Sjors Provoost)
929cda5470 configure: add ax_boost_process (Sjors Provoost)
8314c23d7b [depends] boost: patch unused variable in boost_process (Sjors Provoost)

Pull request description:

  Prerequisite for external signer support in #16546. Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).

  This adds a new dependency [boost process](https://github.com/boostorg/process/tree/boost-1.64.0). This is part of Boost since 1.64 which is part of `depends`. Because the minimum Boost version is 1.47, this functionality is skipped for older versions of Boost.

  Use `./configure --with-boost-process` to opt in, which checks for the presence of Boost::Process.

  We add `UniValue runCommandParseJSON(const std::string& strCommand)` to `system.{h,cpp}` which calls an arbitrary command and processes the JSON returned by it. This is currently only called by the test suite.

  ~For testing purposes this adds a new regtest-only RPC method `runcommand`, as well as `test/mocks/command.py` used by functional tests.~ (this is no longer the case)

  TODO:
  - [ ] review boost process in #15440

ACKs for top commit:
  achow101:
    ACK 31cf68a3ad
  hebasto:
    re-ACK 31cf68a3ad, only rebased (verified with `git range-diff`) and removed an unintentional tab character since the [previous](https://github.com/bitcoin/bitcoin/pull/15382#pullrequestreview-458371035) review.
  meshcollider:
    Very light utACK 31cf68a3ad, although I am not very confident with build stuff.
  promag:
    Code review ACK 31cf68a3ad, don't mind the nit.
  ryanofsky:
    Code review ACK 31cf68a3ad. I left some comments below that could be ignored or followed up later. The current change is clean and comprehensive.

Tree-SHA512: c506e747014b263606e1f538ed4624a8ad7bcf4e025cb700c12cc5739964e254dc04a2bbb848996b170e2ccec3fbfa4fe9e2b3976b191222cfb82fc3e6ab182d
This commit is contained in:
Samuel Dobson 2020-08-05 23:21:24 +12:00
commit e4df534c60
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
24 changed files with 295 additions and 15 deletions

View file

@ -80,7 +80,7 @@ jobs:
QEMU_USER_CMD=""
- stage: test
name: 'Win64 [GOAL: deploy] [unit tests, no gui, no functional tests]'
name: 'Win64 [GOAL: deploy] [unit tests, no gui, no boost::process, no functional tests]'
env: >-
FILE_ENV="./ci/test/00_setup_env_win64.sh"

View file

@ -0,0 +1,121 @@
# ===========================================================================
# https://www.gnu.org/software/autoconf-archive/ax_boost_process.html
# ===========================================================================
#
# SYNOPSIS
#
# AX_BOOST_PROCESS
#
# DESCRIPTION
#
# Test for Process library from the Boost C++ libraries. The macro
# requires a preceding call to AX_BOOST_BASE. Further documentation is
# available at <http://randspringer.de/boost/index.html>.
#
# This macro calls:
#
# AC_SUBST(BOOST_PROCESS_LIB)
#
# And sets:
#
# HAVE_BOOST_PROCESS
#
# LICENSE
#
# Copyright (c) 2008 Thomas Porschberg <thomas@randspringer.de>
# Copyright (c) 2008 Michael Tindal
# Copyright (c) 2008 Daniel Casimiro <dan.casimiro@gmail.com>
#
# Copying and distribution of this file, with or without modification, are
# permitted in any medium without royalty provided the copyright notice
# and this notice are preserved. This file is offered as-is, without any
# warranty.
#serial 2
AC_DEFUN([AX_BOOST_PROCESS],
[
AC_ARG_WITH([boost-process],
AS_HELP_STRING([--with-boost-process@<:@=special-lib@:>@],
[use the Process library from boost - it is possible to specify a certain library for the linker
e.g. --with-boost-process=boost_process-gcc-mt ]),
[
if test "$withval" = "no"; then
want_boost_process="no"
elif test "$withval" = "yes"; then
want_boost_process="yes"
ax_boost_user_process_lib=""
else
want_boost_process="yes"
ax_boost_user_process_lib="$withval"
fi
],
[want_boost_process="yes"]
)
if test "x$want_boost_process" = "xyes"; then
AC_REQUIRE([AC_PROG_CC])
AC_REQUIRE([AC_CANONICAL_BUILD])
CPPFLAGS_SAVED="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS"
export CPPFLAGS
LDFLAGS_SAVED="$LDFLAGS"
LDFLAGS="$LDFLAGS $BOOST_LDFLAGS"
export LDFLAGS
AC_CACHE_CHECK(whether the Boost::Process library is available,
ax_cv_boost_process,
[AC_LANG_PUSH([C++])
CXXFLAGS_SAVE=$CXXFLAGS
CXXFLAGS=
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[@%:@include <boost/process.hpp>]],
[[boost::process::child* child = new boost::process::child; delete child;]])],
ax_cv_boost_process=yes, ax_cv_boost_process=no)
CXXFLAGS=$CXXFLAGS_SAVE
AC_LANG_POP([C++])
])
if test "x$ax_cv_boost_process" = "xyes"; then
AC_SUBST(BOOST_CPPFLAGS)
AC_DEFINE(HAVE_BOOST_PROCESS,,[define if the Boost::Process library is available])
BOOSTLIBDIR=`echo $BOOST_LDFLAGS | sed -e 's/@<:@^\/@:>@*//'`
LDFLAGS_SAVE=$LDFLAGS
if test "x$ax_boost_user_process_lib" = "x"; then
for libextension in `ls -r $BOOSTLIBDIR/libboost_process* 2>/dev/null | sed 's,.*/lib,,' | sed 's,\..*,,'` ; do
ax_lib=${libextension}
AC_CHECK_LIB($ax_lib, exit,
[BOOST_PROCESS_LIB="-l$ax_lib"; AC_SUBST(BOOST_PROCESS_LIB) link_process="yes"; break],
[link_process="no"])
done
if test "x$link_process" != "xyes"; then
for libextension in `ls -r $BOOSTLIBDIR/boost_process* 2>/dev/null | sed 's,.*/,,' | sed -e 's,\..*,,'` ; do
ax_lib=${libextension}
AC_CHECK_LIB($ax_lib, exit,
[BOOST_PROCESS_LIB="-l$ax_lib"; AC_SUBST(BOOST_PROCESS_LIB) link_process="yes"; break],
[link_process="no"])
done
fi
else
for ax_lib in $ax_boost_user_process_lib boost_process-$ax_boost_user_process_lib; do
AC_CHECK_LIB($ax_lib, exit,
[BOOST_PROCESS_LIB="-l$ax_lib"; AC_SUBST(BOOST_PROCESS_LIB) link_process="yes"; break],
[link_process="no"])
done
fi
if test "x$ax_lib" = "x"; then
AC_MSG_ERROR(Could not find a version of the Boost::Process library!)
fi
if test "x$link_process" = "xno"; then
AC_MSG_ERROR(Could not link against $ax_lib !)
fi
fi
CPPFLAGS="$CPPFLAGS_SAVED"
LDFLAGS="$LDFLAGS_SAVED"
fi
])

View file

@ -47,6 +47,9 @@
/* define if the Boost::Filesystem library is available */
#define HAVE_BOOST_FILESYSTEM /**/
/* define if the Boost::Process library is available */
#define HAVE_BOOST_PROCESS /**/
/* define if the Boost::System library is available */
#define HAVE_BOOST_SYSTEM /**/

View file

@ -1 +1 @@
berkeleydb boost-filesystem boost-multi-index boost-signals2 boost-test boost-thread libevent[thread] zeromq double-conversion
berkeleydb boost-filesystem boost-multi-index boost-process boost-signals2 boost-test boost-thread libevent[thread] zeromq double-conversion

View file

@ -25,4 +25,4 @@ export RUN_FUNCTIONAL_TESTS=true
export GOAL="install"
# -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1"
# This could be removed once the ABI change warning does not show up by default
export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports CXXFLAGS=-Wno-psabi --enable-werror"
export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports CXXFLAGS=-Wno-psabi --enable-werror --with-boost-process"

View file

@ -11,5 +11,5 @@ export CONTAINER_NAME=ci_i686_centos_7
export DOCKER_NAME_TAG=centos:7
export DOCKER_PACKAGES="gcc-c++ glibc-devel.x86_64 libstdc++-devel.x86_64 glibc-devel.i686 libstdc++-devel.i686 ccache libtool make git python3 python36-zmq which patch lbzip2 dash"
export GOAL="install"
export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-reduce-exports"
export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-reduce-exports --with-boost-process"
export CONFIG_SHELL="/bin/dash"

View file

@ -14,4 +14,4 @@ export XCODE_BUILD_ID=11C505
export RUN_UNIT_TESTS=false
export RUN_FUNCTIONAL_TESTS=false
export GOAL="deploy"
export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --enable-werror"
export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --enable-werror --with-boost-process"

View file

@ -10,7 +10,7 @@ export HOST=x86_64-apple-darwin16
export DOCKER_NAME_TAG=ubuntu:18.04 # Check that bionic can cross-compile to macos (bionic is used in the gitian build as well)
export PIP_PACKAGES="zmq"
export GOAL="install"
export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --enable-werror"
export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --enable-werror --with-boost-process"
export NO_DEPENDS=1
export OSX_SDK=""
export CCACHE_SIZE=300M

View file

@ -12,4 +12,4 @@ export DOCKER_NAME_TAG=ubuntu:20.04
export NO_DEPENDS=1
export TEST_RUNNER_EXTRA="--timeout-factor=4" # Increase timeout because sanitizers slow down
export GOAL="install"
export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++"
export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++ --with-boost-process"

View file

@ -14,5 +14,5 @@ export RUN_UNIT_TESTS=false
export RUN_FUNCTIONAL_TESTS=false
export RUN_FUZZ_TESTS=true
export GOAL="install"
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=clang CXX=clang++"
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=clang CXX=clang++ --with-boost-process"
export CCACHE_SIZE=200M

View file

@ -11,5 +11,5 @@ export DOCKER_NAME_TAG=ubuntu:20.04
export PACKAGES="cmake python3"
export DEP_OPTS="MULTIPROCESS=1"
export GOAL="install"
export BITCOIN_CONFIG=""
export BITCOIN_CONFIG="--with-boost-process"
export TEST_RUNNER_ENV="BITCOIND=bitcoin-node"

View file

@ -11,4 +11,4 @@ export DOCKER_NAME_TAG=ubuntu:16.04 # Use xenial to have one config run the tes
export PACKAGES="python3-zmq clang-3.8 llvm-3.8" # Use clang-3.8 to test C++11 compatibility, see doc/dependencies.md
export DEP_OPTS="NO_WALLET=1"
export GOAL="install"
export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports CC=clang-3.8 CXX=clang++-3.8"
export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports CC=clang-3.8 CXX=clang++-3.8 --with-boost-process"

View file

@ -16,4 +16,4 @@ export RUN_UNIT_TESTS_SEQUENTIAL="true"
export RUN_UNIT_TESTS="false"
export GOAL="install"
export PREVIOUS_RELEASES_TO_DOWNLOAD="v0.15.2 v0.16.3 v0.17.1 v0.18.1 v0.19.1"
export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-c++17 --enable-debug CFLAGS=\"-g0 -O2 -funsigned-char\" CXXFLAGS=\"-g0 -O2 -funsigned-char\""
export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-c++17 --enable-debug CFLAGS=\"-g0 -O2 -funsigned-char\" CXXFLAGS=\"-g0 -O2 -funsigned-char\" --with-boost-process"

View file

@ -12,4 +12,4 @@ export PACKAGES="clang llvm libc++abi-dev libc++-dev python3-zmq"
export DEP_OPTS="CC=clang CXX='clang++ -stdlib=libc++'"
export TEST_RUNNER_EXTRA="--exclude feature_block --timeout-factor=4" # Increase timeout because sanitizers slow down. Low memory on Travis machines, exclude feature_block.
export GOAL="install"
export BITCOIN_CONFIG="--enable-zmq --with-gui=no CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' CXXFLAGS='-g' --with-sanitizers=thread CC=clang CXX='clang++ -stdlib=libc++'"
export BITCOIN_CONFIG="--enable-zmq --with-gui=no CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' CXXFLAGS='-g' --with-sanitizers=thread CC=clang CXX='clang++ -stdlib=libc++' --with-boost-process"

View file

@ -22,4 +22,4 @@ export DOCKER_NAME_TAG="debian:buster"
export RUN_UNIT_TESTS=true
export RUN_FUNCTIONAL_TESTS=true
export GOAL="install"
export BITCOIN_CONFIG="--enable-reduce-exports --with-incompatible-bdb"
export BITCOIN_CONFIG="--enable-reduce-exports --with-incompatible-bdb --with-boost-process"

View file

@ -13,4 +13,4 @@ export PACKAGES="python3 nsis g++-mingw-w64-x86-64 wine-binfmt wine64"
export RUN_FUNCTIONAL_TESTS=false
export RUN_SECURITY_TESTS="true"
export GOAL="deploy"
export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests"
export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests --without-boost-process"

View file

@ -1191,6 +1191,9 @@ AX_BOOST_SYSTEM
AX_BOOST_FILESYSTEM
AX_BOOST_THREAD
dnl Opt-in to boost-process
AS_IF([ test x$with_boost_process != x ], [ AX_BOOST_PROCESS ], [ ax_cv_boost_process=no ] )
dnl Boost 1.56 through 1.62 allow using std::atomic instead of its own atomic
dnl counter implementations. In 1.63 and later the std::atomic approach is default.
m4_pattern_allow(DBOOST_AC_USE_STD_ATOMIC) dnl otherwise it's treated like a macro
@ -1683,6 +1686,7 @@ esac
echo
echo "Options used to compile and link:"
echo " boost process = $ax_cv_boost_process"
echo " multiprocess = $build_multiprocess"
echo " with wallet = $enable_wallet"
echo " with gui / qt = $bitcoin_enable_qt"

View file

@ -31,7 +31,9 @@ $(package)_cxxflags_linux=-fPIC
$(package)_cxxflags_android=-fPIC
endef
# Fix unused variable in boost_process, can be removed after upgrading to 1.72
define $(package)_preprocess_cmds
sed -i.old "s/int ret_sig = 0;//" boost/process/detail/posix/wait_group.hpp && \
echo "using $($(package)_toolset_$(host_os)) : : $($(package)_cxx) : <cxxflags>\"$($(package)_cxxflags) $($(package)_cppflags)\" <linkflags>\"$($(package)_ldflags)\" <archiver>\"$($(package)_archiver_$(host_os))\" <striper>\"$(host_STRIP)\" <ranlib>\"$(host_RANLIB)\" <rc>\"$(host_WINDRES)\" : ;" > user-config.jam
endef

View file

@ -2073,7 +2073,7 @@ INCLUDE_FILE_PATTERNS =
# recursively expanded use the := operator instead of the = operator.
# This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
PREDEFINED =
PREDEFINED = HAVE_BOOST_PROCESS
# If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
# tag can be used to specify a list of macro names that should be expanded. The

View file

@ -265,6 +265,7 @@ BITCOIN_TESTS =\
test/skiplist_tests.cpp \
test/streams_tests.cpp \
test/sync_tests.cpp \
test/system_tests.cpp \
test/util_threadnames_tests.cpp \
test/timedata_tests.cpp \
test/torcontrol_tests.cpp \

95
src/test/system_tests.cpp Normal file
View file

@ -0,0 +1,95 @@
// Copyright (c) 2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
//
#include <test/util/setup_common.h>
#include <util/system.h>
#include <univalue.h>
#ifdef HAVE_BOOST_PROCESS
#include <boost/process.hpp>
#endif // HAVE_BOOST_PROCESS
#include <boost/test/unit_test.hpp>
BOOST_FIXTURE_TEST_SUITE(system_tests, BasicTestingSetup)
// At least one test is required (in case HAVE_BOOST_PROCESS is not defined).
// Workaround for https://github.com/bitcoin/bitcoin/issues/19128
BOOST_AUTO_TEST_CASE(dummy)
{
BOOST_CHECK(true);
}
#ifdef HAVE_BOOST_PROCESS
bool checkMessage(const std::runtime_error& ex)
{
// On Linux & Mac: "No such file or directory"
// On Windows: "The system cannot find the file specified."
const std::string what(ex.what());
BOOST_CHECK(what.find("file") != std::string::npos);
return true;
}
bool checkMessageFalse(const std::runtime_error& ex)
{
BOOST_CHECK_EQUAL(ex.what(), std::string("RunCommandParseJSON error: process(false) returned 1: \n"));
return true;
}
bool checkMessageStdErr(const std::runtime_error& ex)
{
const std::string what(ex.what());
BOOST_CHECK(what.find("RunCommandParseJSON error:") != std::string::npos);
return checkMessage(ex);
}
BOOST_AUTO_TEST_CASE(run_command)
{
{
const UniValue result = RunCommandParseJSON("");
BOOST_CHECK(result.isNull());
}
{
#ifdef WIN32
// Windows requires single quotes to prevent escaping double quotes from the JSON...
const UniValue result = RunCommandParseJSON("echo '{\"success\": true}'");
#else
// ... but Linux and macOS echo a single quote if it's used
const UniValue result = RunCommandParseJSON("echo \"{\"success\": true}\"");
#endif
BOOST_CHECK(result.isObject());
const UniValue& success = find_value(result, "success");
BOOST_CHECK(!success.isNull());
BOOST_CHECK_EQUAL(success.getBool(), true);
}
{
// An invalid command is handled by Boost
BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), boost::process::process_error, checkMessage); // Command failed
}
{
// Return non-zero exit code, no output to stderr
BOOST_CHECK_EXCEPTION(RunCommandParseJSON("false"), std::runtime_error, checkMessageFalse);
}
{
// Return non-zero exit code, with error message for stderr
BOOST_CHECK_EXCEPTION(RunCommandParseJSON("ls nosuchfile"), std::runtime_error, checkMessageStdErr);
}
{
BOOST_REQUIRE_THROW(RunCommandParseJSON("echo \"{\""), std::runtime_error); // Unable to parse JSON
}
// Test std::in, except for Windows
#ifndef WIN32
{
const UniValue result = RunCommandParseJSON("cat", "{\"success\": true}");
BOOST_CHECK(result.isObject());
const UniValue& success = find_value(result, "success");
BOOST_CHECK(!success.isNull());
BOOST_CHECK_EQUAL(success.getBool(), true);
}
#endif
}
#endif // HAVE_BOOST_PROCESS
BOOST_AUTO_TEST_SUITE_END()

View file

@ -6,6 +6,10 @@
#include <sync.h>
#include <util/system.h>
#ifdef HAVE_BOOST_PROCESS
#include <boost/process.hpp>
#endif // HAVE_BOOST_PROCESS
#include <chainparamsbase.h>
#include <util/strencodings.h>
#include <util/string.h>
@ -1161,6 +1165,43 @@ void runCommand(const std::string& strCommand)
}
#endif
#ifdef HAVE_BOOST_PROCESS
UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in)
{
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;
}
#endif // HAVE_BOOST_PROCESS
void SetupEnvironment()
{
#ifdef HAVE_MALLOPT_ARENA_MAX

View file

@ -37,6 +37,8 @@
#include <boost/thread/condition_variable.hpp> // for boost::thread_interrupted
class UniValue;
// Application startup time (used for uptime calculation)
int64_t GetStartupTime();
@ -96,6 +98,16 @@ std::string ShellEscape(const std::string& arg);
#if HAVE_SYSTEM
void runCommand(const std::string& strCommand);
#endif
#ifdef HAVE_BOOST_PROCESS
/**
* 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 // HAVE_BOOST_PROCESS
/**
* Most paths passed as configuration arguments are treated as relative to

View file

@ -63,6 +63,7 @@ EXPECTED_BOOST_INCLUDES=(
boost/optional.hpp
boost/preprocessor/cat.hpp
boost/preprocessor/stringize.hpp
boost/process.hpp
boost/signals2/connection.hpp
boost/signals2/optional_last_value.hpp
boost/signals2/signal.hpp