Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Sep 6, 2018

Testing and debugging after shutdown are harder if the node was run through the gui, because errors and warnings would not be logged to the debug.log or written to the stderr (as is the case for bitcoind).

@maflcko maflcko added the GUI label Sep 6, 2018
@promag
Copy link
Contributor

promag commented Sep 7, 2018

utACK fa7e969.

I guess calling handleMessageBox and friends is not worth it.

BTW noui.cpp is in libbitcoin_server which is shared between bitcoind and bitcoin-qt — initially thought noui.cpp wasn't included in bitcoin-qt.

@jonasschnelli
Copy link
Contributor

utACK fa7e969

@laanwj
Copy link
Member

laanwj commented Sep 11, 2018

I'm fairly sure that this used to be the behavior, the noui handlers were always registered.

But it regressed when moving away from boost::signals in #13961, or earlier, which dropped support for having multiple handlers.

utACK fa7e969 anyhow.

Edit: checked that this does not need backport to 0.17.

@laanwj laanwj merged commit fa7e969 into bitcoin:master Sep 11, 2018
laanwj added a commit that referenced this pull request Sep 11, 2018
…oind

fa7e969 qt: Also log and print messages or questions like bitcoind (MarcoFalke)
dd031e3 noui: Move handlers to header file (MarcoFalke)

Pull request description:

  Testing and debugging after shutdown are harder if the node was run through the gui, because errors and warnings would not be logged to the debug.log or written to the stderr (as is the case for bitcoind).

Tree-SHA512: 1154e2bf02e3c2616c8d28609569d6c3c7344c5877ad5c1303245044cc7aced9eaec9627f1e1258ed087b49c2a2e6f99bc6c1ad0abe0a855b61e737bdf2059bc
@laanwj laanwj added the Bug label Sep 11, 2018
@maflcko maflcko deleted the Mf1808-uiSignals2 branch September 11, 2018 13:31
@maflcko
Copy link
Member Author

maflcko commented Sep 11, 2018

@laanwj I am pretty sure this has always been the case.

Also, it exists on 0.17:

$ ./src/qt/bitcoin-qt -regtest -walletdir=/tmp/aaaab
$

master:

$ ./src/qt/bitcoin-qt -regtest -walletdir=/tmp/aaaab
Error: Specified -walletdir "/tmp/aaaab" does not exist
$

@laanwj
Copy link
Member

laanwj commented Sep 11, 2018

seems you're right, i've checked back as far as 0.10.0 and there's no noui_connect in the GUI

must have imagined it then...

@laanwj
Copy link
Member

laanwj commented Sep 11, 2018

I think I was confused with InitMessage, both qt and the noui implementation have (before this PR):

static void InitMessage(const std::string &message)
{
    LogPrintf("init message: %s\n", message);
}

so they were logged for both the GUI and with bitcoind

the init window also subscribes to this handler, but that's no problem, as multiple handlers for these signals are still supported (in contrast to what I thought)

@maflcko
Copy link
Member Author

maflcko commented Sep 11, 2018

Indeed, init messages are both handled. Though, not the errors, warnings and questions.

Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 11, 2021
…ke bitcoind

fa7e969 qt: Also log and print messages or questions like bitcoind (MarcoFalke)
dd031e3 noui: Move handlers to header file (MarcoFalke)

Pull request description:

  Testing and debugging after shutdown are harder if the node was run through the gui, because errors and warnings would not be logged to the debug.log or written to the stderr (as is the case for bitcoind).

Tree-SHA512: 1154e2bf02e3c2616c8d28609569d6c3c7344c5877ad5c1303245044cc7aced9eaec9627f1e1258ed087b49c2a2e6f99bc6c1ad0abe0a855b61e737bdf2059bc

# Conflicts:
#	src/noui.cpp
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants