From 9b18cc47b174ebc96a0f33c9d074941c657069db Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 7 Jun 2019 14:45:30 +0200 Subject: [PATCH 1/8] Merge #16141: build: remove GZIP export from gitian descriptors bc8863b81922eb878519f328e9b0c7974aaa34ff depends: remove usage of TAR_OPTIONS (fanquake) 3ff1f2a319fc619954736d1e540ccbebc818ff11 build: remove export GZIP from gitian descriptors (fanquake) Pull request description: The `GZIP` environment variable is [deprecated](https://www.gnu.org/software/gzip/manual/gzip.html#Environment), and everywhere that we invoke `gzip` we are already passing `-9n` directly, i.e: ```base find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST ``` ```bash GZIP="-9n" gzip -h gzip: warning: GZIP environment variable is deprecated; use an alias or script Usage: gzip [OPTION]... [FILE]... ``` ACKs for commit bc8863: Tree-SHA512: 2d5277f7bf096fd5bd0dda47dfaf2dc7a31cc5d91eb8cb42db9cbe060d07dff66bf8e1122a89a3a1b597a3b39dbf8d9a8da4f02e642f58e451ce9fb24cc59769 --- contrib/gitian-descriptors/gitian-linux.yml | 10 ++++------ contrib/gitian-descriptors/gitian-osx.yml | 12 +++++------- contrib/gitian-descriptors/gitian-win.yml | 8 +++----- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/contrib/gitian-descriptors/gitian-linux.yml b/contrib/gitian-descriptors/gitian-linux.yml index 458847566f..3b6a6978dc 100755 --- a/contrib/gitian-descriptors/gitian-linux.yml +++ b/contrib/gitian-descriptors/gitian-linux.yml @@ -52,8 +52,6 @@ script: | export QT_RCC_TEST=1 export QT_RCC_SOURCE_DATE_OVERRIDE=1 - export GZIP="-9n" - export TAR_OPTIONS="--mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME"" export TZ="UTC" export BUILD_DIR="$PWD" mkdir -p ${WRAP_DIR} @@ -179,8 +177,8 @@ script: | # Correct tar file order mkdir -p temp pushd temp - tar xf ../$SOURCEDIST - find dashcore-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST + tar -xf ../$SOURCEDIST + find dashcore-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST popd # Workaround for tarball not building with the bare tag version (prep) @@ -220,8 +218,8 @@ script: | find ${DISTNAME}/bin -type f -executable -print0 | xargs -0 -n1 -I{} ../contrib/devtools/split-debug.sh {} {} {}.dbg find ${DISTNAME}/lib -type f -print0 | xargs -0 -n1 -I{} ../contrib/devtools/split-debug.sh {} {} {}.dbg cp ../doc/README.md ${DISTNAME}/ - find ${DISTNAME} -not -name "*.dbg" | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz - find ${DISTNAME} -name "*.dbg" | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz + find ${DISTNAME} -not -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz + find ${DISTNAME} -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz cd ../../ rm -rf distsrc-${i} done diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml index 3fc7c33c72..4a3617c4ee 100644 --- a/contrib/gitian-descriptors/gitian-osx.yml +++ b/contrib/gitian-descriptors/gitian-osx.yml @@ -46,8 +46,6 @@ script: | export QT_RCC_TEST=1 export QT_RCC_SOURCE_DATE_OVERRIDE=1 - export GZIP="-9n" - export TAR_OPTIONS="--mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME"" export TZ="UTC" export BUILD_DIR="$PWD" mkdir -p ${WRAP_DIR} @@ -133,8 +131,8 @@ script: | # Correct tar file order mkdir -p temp pushd temp - tar xf ../$SOURCEDIST - find dashcore-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST + tar -xf ../$SOURCEDIST + find dashcore-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST popd # Workaround for tarball not building with the bare tag version (prep) @@ -171,7 +169,7 @@ script: | cp ${BASEPREFIX}/${i}/native/bin/${i}-pagestuff unsigned-app-${i}/pagestuff mv dist unsigned-app-${i} pushd unsigned-app-${i} - find . | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-osx-unsigned.tar.gz + find . | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-osx-unsigned.tar.gz popd make deploy OSX_DMG="${OUTDIR}/${DISTNAME}-osx-unsigned.dmg" @@ -181,8 +179,8 @@ script: | find . -name "lib*.a" -delete rm -rf ${DISTNAME}/lib/pkgconfig find .. -name "*.dSYM" -exec cp -ra {} ${DISTNAME}/bin \; - find ${DISTNAME} -not -path '*.dSYM*' | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz - find ${DISTNAME} -path '*.dSYM*' | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz + find ${DISTNAME} -not -path '*.dSYM*' | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz + find ${DISTNAME} -path '*.dSYM*' | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz cd ../../ done mkdir -p $OUTDIR/src diff --git a/contrib/gitian-descriptors/gitian-win.yml b/contrib/gitian-descriptors/gitian-win.yml index 36627154a2..ae13f99cec 100755 --- a/contrib/gitian-descriptors/gitian-win.yml +++ b/contrib/gitian-descriptors/gitian-win.yml @@ -41,8 +41,6 @@ script: | export QT_RCC_TEST=1 export QT_RCC_SOURCE_DATE_OVERRIDE=1 - export GZIP="-9n" - export TAR_OPTIONS="--mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME"" export TZ="UTC" export BUILD_DIR="$PWD" mkdir -p ${WRAP_DIR} @@ -151,8 +149,8 @@ script: | # Correct tar file order mkdir -p temp pushd temp - tar xf ../$SOURCEDIST - find dashcore-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST + tar -xf ../$SOURCEDIST + find dashcore-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST mkdir -p $OUTDIR/src cp ../$SOURCEDIST $OUTDIR/src popd @@ -198,7 +196,7 @@ script: | cd $BUILD_DIR/windeploy mkdir unsigned cp $OUTDIR/dashcore-*setup-unsigned.exe unsigned/ - find . | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-win-unsigned.tar.gz + find . | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-win-unsigned.tar.gz mv ${OUTDIR}/${DISTNAME}-x86_64-*-debug.zip ${OUTDIR}/${DISTNAME}-win64-debug.zip mv ${OUTDIR}/${DISTNAME}-x86_64-*.zip ${OUTDIR}/${DISTNAME}-win64.zip From 34d1a742d433e6349949ee6e59adafd05b5f6006 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 13 Jun 2019 13:34:05 +0200 Subject: [PATCH 2/8] Merge #15991: Bugfix: fix pruneblockchain returned prune height MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f402012cc fixup: Fix prunning test (João Barbosa) 97f517dd8 Fix RPC/pruneblockchain returned prune height (Jonas Schnelli) Pull request description: The help of `pruneblockchain` tells us that the return value is `Height of the last block pruned.`,... but the implementation naively returns the provided input `height` and therefore not respecting that pruning can't be done on all possible blockheight due to the fact that we only prune complete blockfiles (which combine multiple blocks). This fixes the return value to actually return the correct prune height. ACKs for commit f40201: MarcoFalke: ACK f402012ccfc596d7d94851dabbf386c278ff5335 Tree-SHA512: 88c910030ffb83196663e5ebebc29d036fcdbbb2ab266e4538991867924a61bacd8361c1fbf294a0ea7e02347ae183d792f10a10b8f6187e8a4c4c6e4124d7e6 --- src/rpc/blockchain.cpp | 7 ++++++- test/functional/feature_pruning.py | 19 +++---------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 42ad888dd1..2dce9e1c96 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1226,7 +1226,12 @@ static UniValue pruneblockchain(const JSONRPCRequest& request) } PruneBlockFilesManual(height); - return uint64_t(height); + const CBlockIndex* block = ::ChainActive().Tip(); + assert(block); + while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) { + block = block->pprev; + } + return uint64_t(block->nHeight); } static UniValue gettxoutsetinfo(const JSONRPCRequest& request) diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index a6d40cf0d7..bc1a07fe2d 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -16,8 +16,6 @@ from test_framework.script import CScript, OP_RETURN, OP_NOP from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, connect_nodes, disconnect_nodes, wait_until -MIN_BLOCKS_TO_KEEP = 288 - # Rescans start at the earliest block up to 2 hours before a key timestamp, so # the manual prune RPC avoids pruning blocks in the same window to be # compatible with pruning based on key creation time. @@ -266,20 +264,9 @@ class PruneTest(BitcoinTestFramework): else: return index - def prune(index, expected_ret=None): + def prune(index): ret = node.pruneblockchain(height=height(index)) - # Check the return value. When use_timestamp is True, just check - # that the return value is less than or equal to the expected - # value, because when more than one block is generated per second, - # a timestamp will not be granular enough to uniquely identify an - # individual block. - if expected_ret is None: - expected_ret = index - if use_timestamp: - assert_greater_than(ret, 0) - assert_greater_than(expected_ret + 1, ret) - else: - assert_equal(ret, expected_ret) + assert_equal(ret, node.getblockchaininfo()['pruneheight']) def has_block(index): return os.path.isfile(os.path.join(self.nodes[node_number].datadir, self.chain, "blocks", "blk{:05}.dat".format(index))) @@ -319,7 +306,7 @@ class PruneTest(BitcoinTestFramework): assert not has_block(1), "blk00001.dat is still there, should be pruned by now" # height=1000 should not prune anything more, because tip-288 is in blk00002.dat. - prune(1000, 1001 - MIN_BLOCKS_TO_KEEP) + prune(1000) assert has_block(2), "blk00002.dat is still there, should be pruned by now" # advance the tip so blk00002.dat and blk00003.dat can be pruned (the last 288 blocks should now be in blk00004.dat) From feafa02be21ab339a719f4caa655c5bedeba7adb Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 20 Jun 2019 17:39:14 -0400 Subject: [PATCH 3/8] Merge #16207: test: stop generating lcov coverage when functional tests fail 9218ce8d48 Failing functional tests stop lcov (Aseem Sood) Pull request description: Fixes #15648 Functional tests can fail and lcov still generates a coverage file, which is inaccurate. This change stops `make` from proceeding if functional tests fail. before: ![image](https://user-images.githubusercontent.com/6106941/59449220-a9a11480-8dd4-11e9-9eff-81c42513aafa.png) after: ![image](https://user-images.githubusercontent.com/6106941/59449234-b160b900-8dd4-11e9-9d80-6e9c7f41c241.png) ACKs for commit 9218ce: laanwj: straightforward enough ACK 9218ce8d48e504aeaa983e82b206a386712e43e4 Tree-SHA512: 6bbba625f021471d897e911b0df7703153634ef133e295e7be8639346e11f5532bac04e9bab7d793e520fdf4b903219cacecc2ce1e25da0a6828a34a396729e2 --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 195622c38f..6e48652ba2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -207,7 +207,7 @@ test_dash_filtered.info: test_dash.info $(LCOV) -a $@ $(LCOV_OPTS) -o $@ functional_test.info: test_dash_filtered.info - -@TIMEOUT=15 test/functional/test_runner.py $(EXTENDED_FUNCTIONAL_TESTS) + @TIMEOUT=15 test/functional/test_runner.py $(EXTENDED_FUNCTIONAL_TESTS) $(LCOV) -c $(LCOV_OPTS) -d $(abs_builddir)/src --t functional-tests -o $@ $(LCOV) -z $(LCOV_OPTS) -d $(abs_builddir)/src From b2f9eef0d5f460116acf1afd9562e90ce847d13c Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 24 Jun 2019 09:15:38 +0800 Subject: [PATCH 4/8] Merge #16263: qt: Use qInfo() if no error occurs a2aabfb749198bce896c9e597082acd67d3b863e Use qInfo() if no error occurs (Hennadii Stepanov) Pull request description: [Warning and Debugging Messages](https://doc.qt.io/qt-5/debug.html#warning-and-debugging-messages): > - `qInfo()` is used for informational messages. > - `qWarning()` is used to report warnings and recoverable errors in your application. > > If the `QT_FATAL_WARNINGS` environment variable is set, `qWarning()` exits after printing the warning message. This makes it easy to obtain a backtrace in the debugger. [`qWarning()`](https://doc.qt.io/qt-5/qtglobal.html#qWarning): > Calls the message handler with the warning message message... This function does nothing if `QT_NO_WARNING_OUTPUT` was defined during compilation; it exits if at the nth warning corresponding to the counter in environment variable `QT_FATAL_WARNINGS`. This PR allows more productive debugging using the environment variable `QT_FATAL_WARNINGS`. Examples: - https://github.com/bitcoin/bitcoin/pull/16118#issuecomment-503184695 - https://github.com/bitcoin/bitcoin/pull/16254#issuecomment-504223404 The behavior, when option `-debug=qt` is set/unset, remains unchanged. ACKs for commit a2aabf: promag: ACK a2aabfb, I also have this change locally. Empact: ACK https://github.com/bitcoin/bitcoin/pull/16263/commits/a2aabfb749198bce896c9e597082acd67d3b863e laanwj: ACK a2aabfb749198bce896c9e597082acd67d3b863e fanquake: ACK a2aabfb749198bce896c9e597082acd67d3b863e. Tree-SHA512: b4df300c9c00a1705b0d3a10227e3deaac19a98b0a898bb60d5a88872cf450fb131eba150d9dd6c29e021566ee04b3b86b7d486bbe28bd894743c128d2309155 --- src/qt/dash.cpp | 2 +- src/qt/optionsmodel.cpp | 2 +- src/qt/paymentserver.cpp | 2 +- src/qt/winshutdownmonitor.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp index fa634fe821..764e33192e 100644 --- a/src/qt/dash.cpp +++ b/src/qt/dash.cpp @@ -344,7 +344,7 @@ void BitcoinApplication::initializeResult(bool success) if(success) { // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete - qWarning() << "Platform customization:" << gArgs.GetArg("-uiplatform", BitcoinGUI::DEFAULT_UIPLATFORM).c_str(); + qInfo() << "Platform customization:" << gArgs.GetArg("-uiplatform", BitcoinGUI::DEFAULT_UIPLATFORM).c_str(); #ifdef ENABLE_WALLET m_wallet_controller = new WalletController(m_node, optionsModel, this); #ifdef ENABLE_BIP70 diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 9af060cf46..84fd7c8d8c 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -296,7 +296,7 @@ static void CopySettings(QSettings& dst, const QSettings& src) /** Back up a QSettings to an ini-formatted file. */ static void BackupSettings(const fs::path& filename, const QSettings& src) { - qWarning() << "Backing up GUI settings to" << GUIUtil::boostPathToQString(filename); + qInfo() << "Backing up GUI settings to" << GUIUtil::boostPathToQString(filename); QSettings dst(GUIUtil::boostPathToQString(filename), QSettings::IniFormat); dst.clear(); CopySettings(dst, src); diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 89daa02353..dc9031bbbb 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -493,7 +493,7 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store) continue; } } - qWarning() << "PaymentServer::LoadRootCAs: Loaded " << nRootCerts << " root certificates"; + qInfo() << "PaymentServer::LoadRootCAs: Loaded " << nRootCerts << " root certificates"; // Project for another day: // Fetch certificate revocation lists, and add them to certStore. diff --git a/src/qt/winshutdownmonitor.cpp b/src/qt/winshutdownmonitor.cpp index b6f9cec05b..243abafb42 100644 --- a/src/qt/winshutdownmonitor.cpp +++ b/src/qt/winshutdownmonitor.cpp @@ -50,7 +50,7 @@ void WinShutdownMonitor::registerShutdownBlockReason(const QString& strReason, c } if (shutdownBRCreate(mainWinId, strReason.toStdWString().c_str())) - qWarning() << "registerShutdownBlockReason: Successfully registered: " + strReason; + qInfo() << "registerShutdownBlockReason: Successfully registered: " + strReason; else qWarning() << "registerShutdownBlockReason: Failed to register: " + strReason; } From 6d6630d25102128e3ff157afd958a78ba1b3f7fe Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 25 Jun 2019 12:13:08 +0200 Subject: [PATCH 5/8] Merge #16252: test: Log to debug.log in all unit tests fabc57e07dc34c103552cb51c9277bb48ef97739 test: Log to debug.log in all tests (MarcoFalke) fa4a04a5a942d582c62773d815c7e1e9897975d0 test: use common setup in gui tests (MarcoFalke) fad3d2a624377de4b0311e6ddd446c36dafd1ddc test: Create data dir in BasicTestingSetup (MarcoFalke) Pull request description: This makes it easier to debug a frozen test or a test that failed. To debug a failed test, remove the line `fs::remove_all(m_path_root);`. The pull is done in three commits: * Create a datadir for every unit test once (and only once). This requires the `SetDataDir` function to go away. * Use the common setup in the gui unit tests. Some of those tests are testing the init sequence, so we'd have to undo some of what the testing setup did. * Log to the debug.log in all tests ACKs for top commit: laanwj: ACK fabc57e07dc34c103552cb51c9277bb48ef97739 Tree-SHA512: 73444210b88172669e2cd22c2703a1e30e105185d2d5f03decbdedcfd09c64ed208d3716c59c8bebb0e44214cee5c8095e3e995d049e1572ee98f1017e413665 --- src/logging.cpp | 8 ++++++++ src/logging.h | 2 ++ src/qt/test/apptests.cpp | 4 ++++ src/qt/test/rpcnestedtests.cpp | 1 + src/qt/test/test_main.cpp | 12 +++--------- src/test/blockfilter_index_tests.cpp | 2 -- src/test/dbwrapper_tests.cpp | 18 +++++++++--------- src/test/flatfile_tests.cpp | 8 ++++---- src/test/fs_tests.cpp | 2 +- src/test/net_tests.cpp | 2 -- src/test/setup_common.cpp | 20 +++++++++----------- src/test/setup_common.h | 7 ++----- src/test/util_tests.cpp | 4 ++-- src/wallet/test/db_tests.cpp | 12 ++++++------ src/wallet/test/init_test_fixture.cpp | 4 ++-- src/wallet/test/wallet_tests.cpp | 2 +- 16 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 1a65a8534c..2ef62b32be 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -76,6 +76,14 @@ bool BCLog::Logger::StartLogging() return true; } +void BCLog::Logger::DisconnectTestLogger() +{ + StdLockGuard scoped_lock(m_cs); + m_buffering = true; + if (m_fileout != nullptr) fclose(m_fileout); + m_fileout = nullptr; +} + void BCLog::Logger::EnableCategory(BCLog::LogFlags flag) { m_categories |= flag; diff --git a/src/logging.h b/src/logging.h index 14e49993e2..03fd968199 100644 --- a/src/logging.h +++ b/src/logging.h @@ -125,6 +125,8 @@ namespace BCLog { /** Start logging (and flush all buffered messages) */ bool StartLogging(); + /** Only for testing */ + void DisconnectTestLogger(); void ShrinkDebugFile(); diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 64a965700a..3126c0958e 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -67,6 +68,9 @@ void AppTests::appTests() } #endif + ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference + LogInstance().DisconnectTestLogger(); + m_app.parameterSetup(); GUIUtil::loadFonts(); m_app.createOptionsModel(true /* reset settings */); diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 1210a4379e..0b49755501 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -41,6 +41,7 @@ void RPCNestedTests::rpcNestedTests() tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); //mempool.setSanityCheck(1.0); + LogInstance().DisconnectTestLogger(); // Already started by the common test setup, so stop it to avoid interference TestingSetup test; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index e4dbc09e56..1d5fb6991e 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -12,10 +12,10 @@ #include #include #include -#include #include #include #include +#include #ifdef ENABLE_WALLET #include @@ -50,14 +50,8 @@ extern void noui_connect(); // This is all you need to run all the tests int main(int argc, char *argv[]) { - SetupEnvironment(); - SetupNetworking(); - SelectParams(CBaseChainParams::REGTEST); - noui_connect(); - ClearDatadirCache(); - fs::path pathTemp = fs::temp_directory_path() / strprintf("test_dash-qt_%lu_%i", (unsigned long)GetTime(), (int)GetRand(1 << 30)); - fs::create_directories(pathTemp); - gArgs.ForceSetArg("-datadir", pathTemp.string()); + BasicTestingSetup test{CBaseChainParams::REGTEST}; + auto node = interfaces::MakeNode(); bool fInvalid = false; diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index b41aed2d2a..2e413e1cf6 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -267,8 +267,6 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(blockfilter_index_init_destroy, BasicTestingSetup) { - SetDataDir("tempdir"); - BlockFilterIndex* filter_index; filter_index = GetBlockFilterIndex(BlockFilterType::BASIC_FILTER); diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 27ea7f91ca..ab86355324 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_obfuscate_true" : "dbwrapper_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'k'; uint256 in = InsecureRand256(); @@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_basic_data) { // Perform tests both obfuscated and non-obfuscated. for (bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_1").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_1_obfuscate_true" : "dbwrapper_1_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), false, true, obfuscate); uint256 res; @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_batch").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_batch_obfuscate_true" : "dbwrapper_batch_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'i'; @@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_iterator").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_iterator_obfuscate_true" : "dbwrapper_iterator_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); // The two keys are intentionally chosen for ordering @@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) { // We're going to share this fs::path between two wrappers - fs::path ph = SetDataDir("existing_data_no_obfuscate"); + fs::path ph = GetDataDir() / "existing_data_no_obfuscate"; create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. @@ -244,7 +244,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) BOOST_AUTO_TEST_CASE(existing_data_reindex) { // We're going to share this fs::path between two wrappers - fs::path ph = SetDataDir("existing_data_reindex"); + fs::path ph = GetDataDir() / "existing_data_reindex"; create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. @@ -279,7 +279,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex) BOOST_AUTO_TEST_CASE(iterator_ordering) { - fs::path ph = SetDataDir("iterator_ordering"); + fs::path ph = GetDataDir() / "iterator_ordering"; CDBWrapper dbw(ph, (1 << 20), true, false, false); for (int x=0x00; x<256; ++x) { uint8_t key = x; @@ -359,7 +359,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) { char buf[10]; - fs::path ph = SetDataDir("iterator_string_ordering"); + fs::path ph = GetDataDir() / "iterator_string_ordering"; CDBWrapper dbw(ph, (1 << 20), true, false, false); for (int x=0x00; x<10; ++x) { for (int y = 0; y < 10; y++) { @@ -405,7 +405,7 @@ BOOST_AUTO_TEST_CASE(unicodepath) // On Windows this test will fail if the directory is created using // the ANSI CreateDirectoryA call and the code page isn't UTF8. // It will succeed if the created with CreateDirectoryW. - fs::path ph = SetDataDir("test_runner_₿_🏃_20191128_104644"); + fs::path ph = GetDataDir() / "test_runner_₿_🏃_20191128_104644"; CDBWrapper dbw(ph, (1 << 20)); fs::path lockPath = ph / "LOCK"; diff --git a/src/test/flatfile_tests.cpp b/src/test/flatfile_tests.cpp index 1db2f8054c..ef3946a115 100644 --- a/src/test/flatfile_tests.cpp +++ b/src/test/flatfile_tests.cpp @@ -11,7 +11,7 @@ BOOST_FIXTURE_TEST_SUITE(flatfile_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(flatfile_filename) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFilePos pos(456, 789); @@ -24,7 +24,7 @@ BOOST_AUTO_TEST_CASE(flatfile_filename) BOOST_AUTO_TEST_CASE(flatfile_open) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 16 * 1024); std::string line1("A purely peer-to-peer version of electronic cash would allow online " @@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) BOOST_AUTO_TEST_CASE(flatfile_allocate) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 100); bool out_of_space; @@ -105,7 +105,7 @@ BOOST_AUTO_TEST_CASE(flatfile_allocate) BOOST_AUTO_TEST_CASE(flatfile_flush) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 100); bool out_of_space; diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index bdb6ef33ab..9261b14f22 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -12,7 +12,7 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(fsbridge_fstream) { - fs::path tmpfolder = SetDataDir("fsbridge_fstream"); + fs::path tmpfolder = GetDataDir(); // tmpfile1 should be the same as tmpfile2 fs::path tmpfile1 = tmpfolder / "fs_tests_₿_🏃"; fs::path tmpfile2 = tmpfolder / "fs_tests_₿_🏃"; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 26ee0faf19..6dc14efea5 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -93,7 +93,6 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_AUTO_TEST_CASE(caddrdb_read) { - SetDataDir("caddrdb_read"); CAddrManUncorrupted addrmanUncorrupted; addrmanUncorrupted.MakeDeterministic(); @@ -138,7 +137,6 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) { - SetDataDir("caddrdb_read_corrupted"); CAddrManCorrupted addrmanCorrupted; addrmanCorrupted.MakeDeterministic(); diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index b02af8374c..2b36c55d43 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,13 @@ std::ostream& operator<<(std::ostream& os, const uint256& num) BasicTestingSetup::BasicTestingSetup(const std::string& chainName) : m_path_root(fs::temp_directory_path() / "test_common_" PACKAGE_NAME / strprintf("%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(1 << 30)))) { + fs::create_directories(m_path_root); + gArgs.ForceSetArg("-datadir", m_path_root.string()); + ClearDatadirCache(); + SelectParams(chainName); + gArgs.ForceSetArg("-printtoconsole", "0"); + InitLogging(); + LogInstance().StartLogging(); SHA256AutoDetect(); ECC_Start(); BLSInit(); @@ -52,7 +60,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName) InitScriptExecutionCache(); CCoinJoin::InitStandardDenominations(); fCheckBlockIndex = true; - SelectParams(chainName); evoDb.reset(new CEvoDB(1 << 20, true, true)); deterministicMNManager.reset(new CDeterministicMNManager(*evoDb)); static bool noui_connected = false; @@ -67,26 +74,17 @@ BasicTestingSetup::~BasicTestingSetup() deterministicMNManager.reset(); evoDb.reset(); + LogInstance().DisconnectTestLogger(); fs::remove_all(m_path_root); ECC_Stop(); } -fs::path BasicTestingSetup::SetDataDir(const std::string& name) -{ - fs::path ret = m_path_root / name; - fs::create_directories(ret); - gArgs.ForceSetArg("-datadir", ret.string()); - return ret; -} - TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - SetDataDir("tempdir"); const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. RegisterAllCoreRPCCommands(tableRPC); - ClearDatadirCache(); // We have to run a scheduler thread to prevent ActivateBestChain // from blocking due to queue overrun. diff --git a/src/test/setup_common.h b/src/test/setup_common.h index c3bbf35127..e3ea19e412 100644 --- a/src/test/setup_common.h +++ b/src/test/setup_common.h @@ -54,22 +54,19 @@ static inline bool InsecureRandBool() { return g_insecure_rand_ctx.randbool(); } static constexpr CAmount CENT{1000000}; /** Basic testing setup. - * This just configures logging and chain parameters. + * This just configures logging, data dir and chain parameters. */ struct BasicTestingSetup { ECCVerifyHandle globalVerifyHandle; explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~BasicTestingSetup(); - - fs::path SetDataDir(const std::string& name); - private: const fs::path m_path_root; }; /** Testing setup that configures a complete environment. - * Included are data directory, coins database, script check threads setup. + * Included are coins database, script check threads setup. */ struct TestingSetup : public BasicTestingSetup { boost::thread_group threadGroup; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 8a0263b4be..8b2828203f 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1136,7 +1136,7 @@ static constexpr char ExitCommand = 'X'; BOOST_AUTO_TEST_CASE(test_LockDirectory) { - fs::path dirname = SetDataDir("test_LockDirectory") / fs::unique_path(); + fs::path dirname = GetDataDir() / "lock_dir"; const std::string lockname = ".lock"; #ifndef WIN32 // Revert SIGCHLD to default, otherwise boost.test will catch and fail on @@ -1225,7 +1225,7 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) BOOST_AUTO_TEST_CASE(test_DirIsWritable) { // Should be able to write to the data dir. - fs::path tmpdirname = SetDataDir("test_DirIsWritable"); + fs::path tmpdirname = GetDataDir(); BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true); // Should not be able to write to a non-existent dir. diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index bed14ea7b1..fc27433dc1 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -16,7 +16,7 @@ BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(getwalletenv_file) { std::string test_name = "test_name.dat"; - fs::path datadir = SetDataDir("tempdir"); + const fs::path datadir = GetDataDir(); fs::path file_path = datadir / test_name; std::ofstream f(file_path.BOOST_FILESYSTEM_C_STR); f.close(); @@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file) BOOST_AUTO_TEST_CASE(getwalletenv_directory) { std::string expected_name = "wallet.dat"; - fs::path datadir = SetDataDir("tempdir"); + const fs::path datadir = GetDataDir(); std::string filename; std::shared_ptr env = GetWalletEnv(datadir, filename); @@ -40,8 +40,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_directory) BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) { - fs::path datadir = SetDataDir("tempdir"); - fs::path datadir_2 = SetDataDir("tempdir_2"); + fs::path datadir = GetDataDir() / "1"; + fs::path datadir_2 = GetDataDir() / "2"; std::string filename; std::shared_ptr env_1 = GetWalletEnv(datadir, filename); @@ -54,8 +54,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_free_instance) { - fs::path datadir = SetDataDir("tempdir"); - fs::path datadir_2 = SetDataDir("tempdir_2"); + fs::path datadir = GetDataDir() / "1"; + fs::path datadir_2 = GetDataDir() / "2"; std::string filename; std::shared_ptr env_1_a = GetWalletEnv(datadir, filename); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 3b828d57f9..fcd1f3fea8 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -13,7 +13,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam std::string sep; sep += fs::path::preferred_separator; - m_datadir = SetDataDir("tempdir"); + m_datadir = GetDataDir(); m_cwd = fs::current_path(); m_walletdir_path_cases["default"] = m_datadir / "wallets"; @@ -41,4 +41,4 @@ InitWalletDirTestingSetup::~InitWalletDirTestingSetup() void InitWalletDirTestingSetup::SetWalletDir(const fs::path& walletdir_path) { gArgs.ForceSetArg("-walletdir", walletdir_path.string()); -} \ No newline at end of file +} diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 4c39c5f84b..53b36aa9ff 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -195,7 +195,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) auto locked_chain = chain->lock(); LockAnnotation lock(::cs_main); - std::string backup_file = (SetDataDir("importwallet_rescan") / "wallet.backup").string(); + std::string backup_file = (GetDataDir() / "wallet.backup").string(); // Import key into wallet and call dumpwallet to create backup file. { From 93f168eee3081ea5ff752024c310bb72ce2a06b2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 25 Jun 2019 13:32:24 +0200 Subject: [PATCH 6/8] Merge #15894: Remove duplicated "Error: " prefix in logs f724f31401b963c75bd64f5e2c5b9d9561a9a9dd Make AbortNode() aware of MSG_NOPREFIX flag (Hennadii Stepanov) 96fd4ee02f6c3be21cade729b95a85c60634b0f8 Add MSG_NOPREFIX flag for user messages (Hennadii Stepanov) f0641f274ffe94fbe7cae43c07a9373013739df2 Prepend the error/warning prefix for GUI messages (Hennadii Stepanov) Pull request description: The "Error" prefix/title is set already in the next functions: - `noui_ThreadSafeMessageBox()`https://github.com/bitcoin/bitcoin/blob/2068f089c8b7b90eb4557d3f67ea0f0ed2059a23/src/noui.cpp#L17 - `ThreadSafeMessageBox()`https://github.com/bitcoin/bitcoin/blob/a720a983015c9ef8cc814c16a5b9ef6379695817/src/qt/bitcoingui.cpp#L1351 Currently on master: ![Screenshot from 2019-04-25 22-08-54](https://user-images.githubusercontent.com/32963518/56763092-25ee8280-67aa-11e9-86c8-6a029dd9ab08.png) With this PR: ![Screenshot from 2019-04-25 22-26-36](https://user-images.githubusercontent.com/32963518/56763107-30108100-67aa-11e9-9021-683cbd7e2aaa.png) ACKs for top commit: laanwj: concept and code-review ACK f724f31401b963c75bd64f5e2c5b9d9561a9a9dd Tree-SHA512: 218a179b81cc2ac64239d833c02b4c4d4da9b976728a2dcd645966726c4c660b6f1fe43aa28f33d1cb566785a4329e7f93bf5a502bf202316db79d2ff5fce0f8 --- src/noui.cpp | 36 ++++++++++++++++++++---------------- src/qt/bitcoingui.cpp | 32 +++++++++++++++++--------------- src/qt/bitcoingui.h | 2 +- src/ui_interface.h | 3 +++ src/validation.cpp | 22 ++++++++++++---------- 5 files changed, 53 insertions(+), 42 deletions(-) diff --git a/src/noui.cpp b/src/noui.cpp index 6a6ab0b0ce..a196760da3 100644 --- a/src/noui.cpp +++ b/src/noui.cpp @@ -25,26 +25,30 @@ bool noui_ThreadSafeMessageBox(const std::string& message, const std::string& ca { bool fSecure = style & CClientUIInterface::SECURE; style &= ~CClientUIInterface::SECURE; + bool prefix = !(style & CClientUIInterface::MSG_NOPREFIX); + style &= ~CClientUIInterface::MSG_NOPREFIX; std::string strCaption; - // Check for usage of predefined caption - switch (style) { - case CClientUIInterface::MSG_ERROR: - strCaption += _("Error"); - break; - case CClientUIInterface::MSG_WARNING: - strCaption += _("Warning"); - break; - case CClientUIInterface::MSG_INFORMATION: - strCaption += _("Information"); - break; - default: - strCaption += caption; // Use supplied caption (can be empty) + if (prefix) { + switch (style) { + case CClientUIInterface::MSG_ERROR: + strCaption = "Error: "; + break; + case CClientUIInterface::MSG_WARNING: + strCaption = "Warning: "; + break; + case CClientUIInterface::MSG_INFORMATION: + strCaption = "Information: "; + break; + default: + strCaption = caption + ": "; // Use supplied caption (can be empty) + } } - if (!fSecure) - LogPrintf("%s: %s\n", strCaption, message); - tfm::format(std::cerr, "%s: %s\n", strCaption.c_str(), message.c_str()); + if (!fSecure) { + LogPrintf("%s%s\n", strCaption, message); + } + tfm::format(std::cerr, "%s%s\n", strCaption.c_str(), message.c_str()); return false; } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index ca9ce3228c..e898faedcc 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1504,49 +1504,51 @@ void BitcoinGUI::setAdditionalDataSyncProgress(double nSyncProgress) progressBar->setToolTip(tooltip); } -void BitcoinGUI::message(const QString &title, const QString &message, unsigned int style, bool *ret) +void BitcoinGUI::message(const QString& title, QString message, unsigned int style, bool* ret) { - QString strTitle = tr("Dash Core"); // default title + // Default title. On macOS, the window title is ignored (as required by the macOS Guidelines). + QString strTitle{PACKAGE_NAME}; // Default to information icon int nMBoxIcon = QMessageBox::Information; int nNotifyIcon = Notificator::Information; - QString msgType; + bool prefix = !(style & CClientUIInterface::MSG_NOPREFIX); + style &= ~CClientUIInterface::MSG_NOPREFIX; - // Prefer supplied title over style based title + QString msgType; if (!title.isEmpty()) { msgType = title; - } - else { + } else { switch (style) { case CClientUIInterface::MSG_ERROR: msgType = tr("Error"); + if (prefix) message = tr("Error: %1").arg(message); break; case CClientUIInterface::MSG_WARNING: msgType = tr("Warning"); + if (prefix) message = tr("Warning: %1").arg(message); break; case CClientUIInterface::MSG_INFORMATION: msgType = tr("Information"); + // No need to prepend the prefix here. break; default: break; } } - // Append title to "Dash Core - " - if (!msgType.isEmpty()) - strTitle += " - " + msgType; - // Check for error/warning icon + if (!msgType.isEmpty()) { + strTitle += " - " + msgType; + } + if (style & CClientUIInterface::ICON_ERROR) { nMBoxIcon = QMessageBox::Critical; nNotifyIcon = Notificator::Critical; - } - else if (style & CClientUIInterface::ICON_WARNING) { + } else if (style & CClientUIInterface::ICON_WARNING) { nMBoxIcon = QMessageBox::Warning; nNotifyIcon = Notificator::Warning; } - // Display message if (style & CClientUIInterface::MODAL) { // Check for buttons, use OK as default, if none was supplied QMessageBox::StandardButton buttons; @@ -1559,9 +1561,9 @@ void BitcoinGUI::message(const QString &title, const QString &message, unsigned int r = mBox.exec(); if (ret != nullptr) *ret = r == QMessageBox::Ok; - } - else + } else { notificator->notify(static_cast(nNotifyIcon), strTitle, message); + } } void BitcoinGUI::changeEvent(QEvent *e) diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index adaf9206ea..ca9247b439 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -262,7 +262,7 @@ public Q_SLOTS: @see CClientUIInterface::MessageBoxFlags @param[in] ret pointer to a bool that will be modified to whether Ok was clicked (modal only) */ - void message(const QString &title, const QString &message, unsigned int style, bool *ret = nullptr); + void message(const QString& title, QString message, unsigned int style, bool* ret = nullptr); #ifdef ENABLE_WALLET void setCurrentWallet(WalletModel* wallet_model); diff --git a/src/ui_interface.h b/src/ui_interface.h index 964019d162..cdaccef304 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -70,6 +70,9 @@ public: /** Force blocking, modal message box dialog (not just OS notification) */ MODAL = 0x10000000U, + /** Do not prepend error/warning prefix */ + MSG_NOPREFIX = 0x20000000U, + /** Do not print contents of message to debug log */ SECURE = 0x40000000U, diff --git a/src/validation.cpp b/src/validation.cpp index bcdf41b6bb..acb80b69e4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1549,20 +1549,22 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex) } /** Abort with a message */ -static bool AbortNode(const std::string& strMessage, const std::string& userMessage="") +static bool AbortNode(const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) { SetMiscWarning(strMessage); LogPrintf("*** %s\n", strMessage); - uiInterface.ThreadSafeMessageBox( - userMessage.empty() ? _("Error: A fatal internal error occurred, see debug.log for details") : userMessage, - "", CClientUIInterface::MSG_ERROR); + if (!userMessage.empty()) { + uiInterface.ThreadSafeMessageBox(userMessage, "", CClientUIInterface::MSG_ERROR | prefix); + } else { + uiInterface.ThreadSafeMessageBox(_("Error: A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR | CClientUIInterface::MSG_NOPREFIX); + } StartShutdown(); return false; } -static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage="") +static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) { - AbortNode(strMessage, userMessage); + AbortNode(strMessage, userMessage, prefix); return state.Error(strMessage); } @@ -2485,7 +2487,7 @@ bool CChainState::FlushStateToDisk( if (fDoFullFlush || fPeriodicWrite) { // Depend on nMinDiskSpace to ensure we can write block index if (!CheckDiskSpace(GetBlocksDir())) { - return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!")); + return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } // First make sure all block and undo data is flushed to disk. { @@ -2534,7 +2536,7 @@ bool CChainState::FlushStateToDisk( // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. if (!CheckDiskSpace(GetDataDir(), 48 * 2 * 2 * CoinsTip().GetCacheSize())) { - return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!")); + return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } // Flush the chainstate (which may refer to block index entries). if (!CoinsTip().Flush()) @@ -3639,7 +3641,7 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n bool out_of_space; size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode("Disk space is low!", _("Error: Disk space is low!")); + return AbortNode("Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } if (bytes_allocated != 0 && fPruneMode) { fCheckForPruning = true; @@ -3663,7 +3665,7 @@ static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos, un bool out_of_space; size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!")); + return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } if (bytes_allocated != 0 && fPruneMode) { fCheckForPruning = true; From 716f124b00e56c79ff1d94ca0d1a7108803bad99 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 26 Jun 2019 18:29:28 -0400 Subject: [PATCH 7/8] Merge #16289: test: Add missing ECC_Stop() in GUI rpcnestedtests.cpp f466c4ce846000b2f984b4759f89f3f793fa0100 Add missing ECC_Stop(); in GUI rpcnestedtests.cpp (Jonas Schnelli) Pull request description: Fixes #16288 Was probably missing in #7783 ACKs for top commit: Sjors: ACK f466c4c. Tested by comparing `make check` on master and this PR with macOS 10.14.5. I also tried with and without `--enable-debug` / `--without-gui`. fanquake: ACK f466c4ce846000b2f984b4759f89f3f793fa0100. Tested running `make check` on macOS. Tree-SHA512: 648e10c2e35bd01fb92e63709169a6c185ac4b62c69af0109d2cd2d7db47e56ae804c788f9a1a1845746f818764799732f9e58e9dbfca3bffeea8f14683c8c7f --- src/qt/test/rpcnestedtests.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 0b49755501..b64e3e19c7 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -41,7 +41,9 @@ void RPCNestedTests::rpcNestedTests() tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); //mempool.setSanityCheck(1.0); - LogInstance().DisconnectTestLogger(); // Already started by the common test setup, so stop it to avoid interference + ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference + LogInstance().DisconnectTestLogger(); + TestingSetup test; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); From c2b9f011ce5460c0aca18deb9fbe10e8cf722f8d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 28 Jun 2019 13:41:10 +0200 Subject: [PATCH 8/8] Merge #16300: util: Explain why the path is cached fa69c3e6ca71800376761e264320c363f072dc2f util: Explain why the path is cached (MarcoFalke) Pull request description: The rationale for caching the datadir is given as ``` // This can be called during exceptions by LogPrintf(), so we cache the // value so we don't have to do memory allocations after that. ``` Since 8c2d695c4a45bdd9378c7970b0fcba6e1efc01f9, the debug log location is actually cached itself in `m_file_path`. So explain that the caching is now only used to guard against disk access on each call. (See also #16255) ACKs for top commit: promag: ACK fa69c3e6ca71800376761e264320c363f072dc2f. laanwj: ACK fa69c3e6ca71800376761e264320c363f072dc2f ryanofsky: utACK fa69c3e6ca71800376761e264320c363f072dc2f. Good cleanup. Previous comment was confusing, and definitely not helpful if outdated. Tree-SHA512: 02108c90026d6d7c02843aaf59a06b4e1fa63d5d4378bb7760f50767efc340dc94c259bf7afb32fa4d47952b48a4e91798d1e0ddc1b051d770405e078636793a --- src/util/system.cpp | 20 +++++++------------- src/util/system.h | 8 +++----- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index e557ea6e46..79d26815fa 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -768,19 +768,16 @@ fs::path GetDefaultDataDir() static fs::path g_blocks_path_cache_net_specific; static fs::path pathCached; static fs::path pathCachedNetSpecific; -static CCriticalSection csPathCached; +static RecursiveMutex csPathCached; const fs::path &GetBlocksDir() { - LOCK(csPathCached); - fs::path &path = g_blocks_path_cache_net_specific; - // This can be called during exceptions by LogPrintf(), so we cache the - // value so we don't have to do memory allocations after that. - if (!path.empty()) - return path; + // Cache the path to avoid calling fs::create_directories on every call of + // this function + if (!path.empty()) return path; if (gArgs.IsArgSet("-blocksdir")) { path = fs::system_complete(gArgs.GetArg("-blocksdir", "")); @@ -800,15 +797,12 @@ const fs::path &GetBlocksDir() const fs::path &GetDataDir(bool fNetSpecific) { - LOCK(csPathCached); - fs::path &path = fNetSpecific ? pathCachedNetSpecific : pathCached; - // This can be called during exceptions by LogPrintf(), so we cache the - // value so we don't have to do memory allocations after that. - if (!path.empty()) - return path; + // Cache the path to avoid calling fs::create_directories on every call of + // this function + if (!path.empty()) return path; if (gArgs.IsArgSet("-datadir")) { path = fs::system_complete(gArgs.GetArg("-datadir", "")); diff --git a/src/util/system.h b/src/util/system.h index 89c8ff25b7..cb0723bdf2 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Copyright (c) 2014-2021 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -21,20 +21,17 @@ #include #include #include -#include #include #include +#include #include #include -#include #include #include #include #include #include -#include -#include #include #include @@ -107,6 +104,7 @@ fs::path GetDefaultDataDir(); const fs::path &GetBlocksDir(); const fs::path &GetDataDir(bool fNetSpecific = true); fs::path GetBackupsDir(); +/** Tests only */ void ClearDatadirCache(); fs::path GetConfigFile(const std::string& confPath); #ifdef WIN32