log: harmonize bitcoind logging #16489

pull jonatack wants to merge 1 commits into bitcoin:master from jonatack:daemon-logging-harmonisation changing 2 files +6 −6
  1. jonatack commented at 7:30 PM on July 29, 2019: member

    Harmonize the user-facing output of the bitcoind -daemon, bitcoin-cli help stop, bitcoin-cli stop, and bitcoind -version commands to be consistent with each other as well as with the "Bitcoin Core is probably already running" messages, e.g. git grep 'probably already running.")'.

    Before:

    $ bitcoind -regtest -daemon
Bitcoin Core daemon starting

$ bitcoind -regtest -daemon
Error: Bitcoin Core is probably already running.

$ bitcoind -regtest -version
Bitcoin Core Daemon version v0.18.99.0-e653eeff76-dirty

$ bitcoin-cli -regtest help stop
stop

Stop Bitcoin server.

$ bitcoin-cli -regtest stop
Bitcoin server stopping

    these five commands output:

    "Bitcoin Core daemon" "Bitcoin Core" "Bitcoin Core Daemon" "Bitcoin server" "Bitcoin server"

    After this commit, they are all "Bitcoin Core".

    $ bitcoind -regtest -daemon
Bitcoin Core starting

$ bitcoind -regtest -daemon
Error: Bitcoin Core is probably already running.

$ bitcoind -regtest -version
Bitcoin Core version v0.18.99.0-e90478f43e-dirty

$ bitcoin-cli -regtest help stop
stop

Request a graceful shutdown of Bitcoin Core.

$ bitcoin-cli -regtest stop
Bitcoin Core stopping
  2. DrahtBot added the label RPC/REST/ZMQ on Jul 29, 2019
  3. DrahtBot added the label Utils/log/libs on Jul 29, 2019
  4. in src/rpc/server.cpp:165 in 93f22ce41a outdated 
     161 | @@ -162,7 +162,7 @@ UniValue stop(const JSONRPCRequest& jsonRequest)
 162 |      if (jsonRequest.fHelp || jsonRequest.params.size() > 1)
 163 |          throw std::runtime_error(
 164 |              RPCHelpMan{"stop",
 165 | -                "\nStop Bitcoin server.",
 166 | +                "\nStop the " PACKAGE_NAME " daemon.",
    MarcoFalke commented at 9:15 PM on July 29, 2019:

    The gui is not a daemon, so this should be kept more general

    laanwj commented at 5:43 AM on July 30, 2019:

    I don't think changing this from server to daemon is an improvement to be honest. (definitely not from a documentation perspective, "server" is something everyone understands while "daemon" is very much a UNIX word)

    promag commented at 12:13 AM on August 1, 2019:

    How about "Requests a graceful shutdown"?

  5. fanquake commented at 3:37 AM on July 30, 2019: member

    Concept ACK

  6. fanquake removed the label RPC/REST/ZMQ on Jul 30, 2019
  7. practicalswift commented at 9:23 AM on July 30, 2019: contributor

    utACK 93f22ce41a828443b6cbc3662745e2f1de25d1fa

  8. laanwj added the label Waiting for author on Jul 31, 2019
  9. in src/rpc/server.cpp:176 in 93f22ce41a outdated 
     172 | @@ -173,7 +173,7 @@ UniValue stop(const JSONRPCRequest& jsonRequest)
 173 |      if (jsonRequest.params[0].isNum()) {
 174 |          MilliSleep(jsonRequest.params[0].get_int());
 175 |      }
 176 | -    return "Bitcoin server stopping";
 177 | +    return PACKAGE_NAME " daemon stopping";
    promag commented at 12:14 AM on August 1, 2019:

    Just Stopping...?

  10. jonatack renamed this:
    log: update bitcoind daemon logging
    log: harmonize bitcoind logging
    on Aug 1, 2019
  11. log: harmonize bitcoind server logging
    Harmonize the user-facing output of the `bitcoind -daemon`, `bitcoin-cli help stop`, `bitcoin-cli stop`, and `bitcoind -version` commands to be consistent with each other as well as with the "Bitcoin Core is probably already running" messages, e.g. `git grep 'probably already running.")'`.
    e90478f43e
  12. jonatack commented at 6:38 PM on August 1, 2019: member

    Thanks everyone for reviewing. Agreed, updated, and rebased. Two options for harmonizing the logging seem to be the simplicity of "Bitcoin Core", and the more specific but longer "Bitcoin Core server". Tentatively proposing the first as it is short and understandable with the bonus of not changing the other messages that output just "Bitcoin Core" e.g. git grep 'probably already running.")'.

  13. jonatack force-pushed on Aug 1, 2019
  14. in src/rpc/server.cpp:165 in e90478f43e 
     161 | @@ -162,7 +162,7 @@ UniValue stop(const JSONRPCRequest& jsonRequest)
 162 |      if (jsonRequest.fHelp || jsonRequest.params.size() > 1)
 163 |          throw std::runtime_error(
 164 |              RPCHelpMan{"stop",
 165 | -                "\nStop Bitcoin server.",
 166 | +                "\nRequest a graceful shutdown of " PACKAGE_NAME ".",
    promag commented at 7:04 PM on August 1, 2019:

    What's the point of having the package name here?

    jonatack commented at 7:58 PM on August 1, 2019:

    Harmonization, consistency, clarity.

  15. in src/bitcoind.cpp:149 in e90478f43e 
     142 | @@ -143,7 +143,7 @@ static bool AppInit(int argc, char* argv[])
 143 |  #pragma GCC diagnostic push
 144 |  #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 145 |  #endif
 146 | -            tfm::format(std::cout, PACKAGE_NAME " daemon starting\n");
 147 | +            tfm::format(std::cout, PACKAGE_NAME " starting\n");
 148 |  
 149 |              // Daemonize
 150 |              if (daemon(1, 0)) { // don't chdir (1), do close FDs (0)
    MarcoFalke commented at 8:02 PM on August 1, 2019:

    Why are you removing daemon here?

    jonatack commented at 8:50 PM on August 1, 2019:

    Why are you removing daemon here?

    #16489 (review) combined with the goal of harmonizing the various messages, thus my explanation here #16489 (comment).

  16. jonatack commented at 8:55 PM on August 1, 2019: member

    I updated the PR description to hopefully better explain the motivation.

  17. practicalswift commented at 7:05 AM on August 2, 2019: contributor

    ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4 (read code which looks good)

  18. DrahtBot commented at 7:54 AM on August 4, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  19. fanquake deleted a comment on Aug 4, 2019
  20. fanquake removed the label Waiting for author on Aug 14, 2019
  21. practicalswift commented at 12:56 PM on August 15, 2019: contributor

    ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4 -- diff looks correct

  22. ariard approved
  23. ariard commented at 4:43 PM on September 4, 2019: member

    Tested ACK e90478f

  24. fjahr commented at 8:56 PM on September 4, 2019: member

    utACK e90478f

  25. michaelfolkson commented at 2:10 PM on September 6, 2019: contributor

    ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4. Tested command outputs and as described.

  26. fanquake approved
  27. fanquake commented at 12:39 AM on September 10, 2019: member

    ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4

  28. fanquake referenced this in commit d8fe24cbfb on Sep 10, 2019
  29. fanquake merged this on Sep 10, 2019
  30. fanquake closed this on Sep 10, 2019
  31. jonatack deleted the branch on Sep 10, 2019
  32. sidhujag referenced this in commit 723d0900f4 on Sep 10, 2019
  33. jasonbcox referenced this in commit 3d50b09d90 on Oct 16, 2020
  34. DrahtBot locked this on Dec 16, 2021
Contributors
jonatackMarcoFalkelaanwjpromagfanquakepracticalswiftDrahtBotariardfjahrmichaelfolkson
Labels
Utils/log/libs
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me