Skip to content

Conversation

@practicalswift
Copy link
Contributor

Cherry-picked ef6fa1c from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (#12689).

See previous review in #12689.

Printing to the debug log file can be disabled with -nodebulogfile
@fanquake
Copy link
Member

@ryanofsky Could you re-review here.

@practicalswift
Copy link
Contributor Author

Previous utACK:ers @meshcollider and @jnewbery might want to take a look at this one too? :-)

@instagibbs
Copy link
Member

concept ACK

}

if (fPrintToDebugLog) {
if (gArgs.GetBoolArg("-shrinkdebugfile", logCategories == BCLog::NONE)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa - good catch to move this inside the if (fPrintToDebugLog)

@laanwj
Copy link
Member

laanwj commented Apr 17, 2018

Thanks for reviving this, tested ACK 6a3b0d3:

  • src/bitcoind -regtest works as expected, now prints logging output
  • src/bitcoind -regtest -noprinttoconsole successfully silences logging output
  • src/bitcoind -regtest -daemon successfully silences logging output

@laanwj laanwj merged commit 6a3b0d3 into bitcoin:master Apr 17, 2018
laanwj added a commit that referenced this pull request Apr 17, 2018
6a3b0d3 Print to console by default when not run with -daemon (Evan Klitzke)

Pull request description:

  Cherry-picked ef6fa1c from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (#12689).

  See previous review in #12689.

Tree-SHA512: 8923a89b9c8973286d53e960d3c464b1cd026cd5a5911ba62f9f972c83684417dc4004101815dfe987fc1e1baaec1fdd90748a0866bb5548e974d77b3135d43b
self.setup_clean_chain = True

def relative_log_path(self, name):
return os.path.join(self.nodes[0].datadir, "regtest", name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: IIRC datadir is absolute, so the relative in the method name is confusing.


# check that -nodebuglogfile disables logging
self.stop_node(0)
os.unlink(default_log_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could use the identical and more clear "remove", since unlink seems to be some unix specific thing, but we also run the tests on windows.

@maflcko
Copy link
Member

maflcko commented Apr 17, 2018

utACK 6a3b0d3. Some naming nits in tests.

// Add newlines to the logfile to distinguish this execution from the last
// one; called before console logging is set up, so this is only sent to
// debug.log.
LogPrintf("\n\n\n\n\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, won't it print these newlines even if fPrintToDebugLog ends up being false?

maflcko pushed a commit that referenced this pull request Apr 23, 2018
aee80b0 qt: Don't log to console by default (Wladimir J. van der Laan)

Pull request description:

  Default `-printtoconsole` to false for the GUI. GUI programs should not print to the console unnecessarily. For example, when launched by the window manager, the output might end up in the X session log file,
  resulting in duplicate logging. On Windows, it is pointless as well because bitcoin-qt isn't a console application.

  This same mechanism is used to set `-server` to true by default for bitcoind: https://github.com/bitcoin/bitcoin/blob/master/src/bitcoind.cpp#L116

  (fixes #13004)

Tree-SHA512: 24ae460d9d97130a063f7bf7fa6da1e6cc46643a94ea0827aa64d0f4a80647e5e7394695b24ea0f49a147a1fa07329659d224f04511fc24b97a9869d1c29b890
laanwj added a commit to laanwj/bitcoin that referenced this pull request Apr 25, 2018
It's overly noisy, and not useful to have the bitcoin nodes log to
stdout in the functional tests. In case of troubleshooting the
`debug.log` files can be inspected.

This was the behavior before bitcoin#13004.
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jul 8, 2018
bitcoin#13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.

This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.

At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.
laanwj added a commit that referenced this pull request Jul 9, 2018
…debuglogfile (satwo)

5e362c0 Fix command line help for -printtoconsole and -debuglogfile (Samuel B. Atwood)

Pull request description:

  This is a rebased version of #13589 with the changes to the 0.16.x release notes removed.

  > #13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.

  > This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.

  > At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.

Tree-SHA512: 7461d59a1864039d5a9dfcce765a1169df882f51a4ca50a6066416c0803821cd821be07be534e0bd57f0a22c0b45adb881a93abbe91962bc37d2d228f35ee712
laanwj added a commit that referenced this pull request Jul 18, 2018
…gfile changes

801cb30 doc: Add release notes for -printtoconsole and -debuglogfile changes (Samuel B. Atwood)

Pull request description:

  This adds release notes relevant to the changes in #13004 and documented in command line help in #13614.

Tree-SHA512: ff320415afa9be1ace37ebf0a69ee5a7e6d4167465bc41111be062a556da9b8accfc39553fac610e68521c7ab2095126ace4f012a32f5e9f37cbec39cfa74b04
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Sep 11, 2018
bitcoin#13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.

This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.

At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Sep 11, 2018
This adds release notes relevant to the changes in bitcoin#13004 and documented in command line help in bitcoin#13614
jfhk pushed a commit to jfhk/bitcoin that referenced this pull request Nov 14, 2018
bitcoin#13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.

This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.

At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.
jfhk pushed a commit to jfhk/bitcoin that referenced this pull request Nov 14, 2018
This adds release notes relevant to the changes in bitcoin#13004 and documented in command line help in bitcoin#13614
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 17, 2020
…e and -debuglogfile (satwo)

5e362c0 Fix command line help for -printtoconsole and -debuglogfile (Samuel B. Atwood)

Pull request description:

  This is a rebased version of bitcoin#13589 with the changes to the 0.16.x release notes removed.

  > bitcoin#13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.

  > This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.

  > At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.

Tree-SHA512: 7461d59a1864039d5a9dfcce765a1169df882f51a4ca50a6066416c0803821cd821be07be534e0bd57f0a22c0b45adb881a93abbe91962bc37d2d228f35ee712
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Nov 17, 2020
aee80b0 qt: Don't log to console by default (Wladimir J. van der Laan)

Pull request description:

  Default `-printtoconsole` to false for the GUI. GUI programs should not print to the console unnecessarily. For example, when launched by the window manager, the output might end up in the X session log file,
  resulting in duplicate logging. On Windows, it is pointless as well because bitcoin-qt isn't a console application.

  This same mechanism is used to set `-server` to true by default for bitcoind: https://github.com/bitcoin/bitcoin/blob/master/src/bitcoind.cpp#L116

  (fixes bitcoin#13004)

Tree-SHA512: 24ae460d9d97130a063f7bf7fa6da1e6cc46643a94ea0827aa64d0f4a80647e5e7394695b24ea0f49a147a1fa07329659d224f04511fc24b97a9869d1c29b890
@practicalswift practicalswift deleted the printtoconsole branch April 10, 2021 19:34
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 13, 2021
…aemon

6a3b0d3 Print to console by default when not run with -daemon (Evan Klitzke)

Pull request description:

  Cherry-picked ef6fa1c from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (bitcoin#12689).

  See previous review in bitcoin#12689.

Tree-SHA512: 8923a89b9c8973286d53e960d3c464b1cd026cd5a5911ba62f9f972c83684417dc4004101815dfe987fc1e1baaec1fdd90748a0866bb5548e974d77b3135d43b
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Apr 14, 2021
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 18, 2021
…aemon

6a3b0d3 Print to console by default when not run with -daemon (Evan Klitzke)

Pull request description:

  Cherry-picked ef6fa1c from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (bitcoin#12689).

  See previous review in bitcoin#12689.

Tree-SHA512: 8923a89b9c8973286d53e960d3c464b1cd026cd5a5911ba62f9f972c83684417dc4004101815dfe987fc1e1baaec1fdd90748a0866bb5548e974d77b3135d43b
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 10, 2022
aee80b0 qt: Don't log to console by default (Wladimir J. van der Laan)

Pull request description:

  Default `-printtoconsole` to false for the GUI. GUI programs should not print to the console unnecessarily. For example, when launched by the window manager, the output might end up in the X session log file,
  resulting in duplicate logging. On Windows, it is pointless as well because bitcoin-qt isn't a console application.

  This same mechanism is used to set `-server` to true by default for bitcoind: https://github.com/bitcoin/bitcoin/blob/master/src/bitcoind.cpp#L116

  (fixes bitcoin#13004)

Tree-SHA512: 24ae460d9d97130a063f7bf7fa6da1e6cc46643a94ea0827aa64d0f4a80647e5e7394695b24ea0f49a147a1fa07329659d224f04511fc24b97a9869d1c29b890
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 11, 2022
…e and -debuglogfile (satwo)

5e362c0 Fix command line help for -printtoconsole and -debuglogfile (Samuel B. Atwood)

Pull request description:

  This is a rebased version of bitcoin#13589 with the changes to the 0.16.x release notes removed.

  > bitcoin#13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.

  > This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.

  > At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.

Tree-SHA512: 7461d59a1864039d5a9dfcce765a1169df882f51a4ca50a6066416c0803821cd821be07be534e0bd57f0a22c0b45adb881a93abbe91962bc37d2d228f35ee712
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 21, 2022
…aemon

6a3b0d3 Print to console by default when not run with -daemon (Evan Klitzke)

Pull request description:

  Cherry-picked ef6fa1c from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (bitcoin#12689).

  See previous review in bitcoin#12689.

Tree-SHA512: 8923a89b9c8973286d53e960d3c464b1cd026cd5a5911ba62f9f972c83684417dc4004101815dfe987fc1e1baaec1fdd90748a0866bb5548e974d77b3135d43b
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
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.

7 participants