Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Log debug build status and warn when running benchmarks #12197

Merged
merged 2 commits into from Jan 29, 2018
Merged

Log debug build status and warn when running benchmarks #12197

merged 2 commits into from Jan 29, 2018

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Jan 16, 2018

Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

@laanwj
Log debug build status and warn when running benchmarks
0c74e2e 
Log whether the starting instance of bitcoin core is a debug or release
build (--enable-debug).

Also warn when running the benchmarks with a debug build, to prevent
mistakes comparing debug to non-debug results.
laanwj
laanwj commented Jan 16, 2018
View reviewed changes
src/init.cpp
@@ -808,7 +808,13 @@ void InitLogging()
fLogIPs = gArgs.GetBoolArg("-logips", DEFAULT_LOGIPS);

LogPrintf("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n");
LogPrintf("Bitcoin version %s\n", FormatFullVersion());
std::string version_string = FormatFullVersion();
#ifdef DEBUG
Copy link
Member Author

Choose a reason for hiding this comment

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

Could move this into FormatFullVersion if we want it in all places where the version is printed, but I wasn't entirely sure about that being useful, the point is just to have this in the log for post-mortem analysis.

promag
promag reviewed Jan 16, 2018
View reviewed changes
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

Concept ACK.

src/init.cpp
LogPrintf("Bitcoin version %s\n", FormatFullVersion());
std::string version_string = FormatFullVersion();
#ifdef DEBUG
version_string += " (debug build)";
Copy link
Member

Choose a reason for hiding this comment

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

µNit:

#ifdef DEBUG
    const std::string version_string = FormatFullVersion() + " (debug build)";
#else
    const std::string version_string = FormatFullVersion() + " (release build)";
#endif

Copy link
Member Author

Choose a reason for hiding this comment

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

Meh, I prefer it this way.

maflcko
maflcko reviewed Jan 16, 2018
View reviewed changes
src/init.cpp Outdated
#else
version_string += " (release build)";
#endif
LogPrintf("Bitcoin version %s\n", version_string);
Copy link
Member

Choose a reason for hiding this comment

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

Should say "Bitcoin Core" according to the rebranding 4 years ago: #3408

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't introduce this message here, but sure, can update this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've changed it to use PACKAGE_NAME (which is Bitcoin Core), same as for the --help messages.

@maflcko maflcko mentioned this pull request Jan 16, 2018
Closed
@maflcko
Copy link
Member

maflcko commented Jan 18, 2018

Will test after squash

@maflcko
Copy link
Member

maflcko commented Jan 23, 2018

Works for me:

./src/qt/bitcoin-qt -printtoconsole|grep "Bitcoin Core"
2018-01-23 17:08:21 Bitcoin Core version v0.15.99.0-a4057776c4-dirty (release build)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Tested ACK 34328b4980848bca372347c8680b94d9d02eca0a
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJaZ2xYAAoJENLqSFDnUoslMDIP/3hG5gdV4ErYcur/NsYVqI+M
oD4kABfyk+HDDka/h/ZXrA2Yt37z08Rit682nT2r4yf/ZHPZ/zH3B+vmFLkX/bml
JQNQDYrncW7U6R9xaM49P6bLzL2JYN6wQ403lN9R3DXL7besMe5PlxpAvVf7gQ+u
E44bFLGl//rG/rn5TdXiZaT697DMb07IDLwez/vpml+UrrQ47NAhFZ4zgpcUItUv
1ClvFiBPMonXA6y/P8iMS++akOQjsQgjCcQXhiRwwwvJ4/PgB69HQnyL9SQztlQt
bugLrEqkxHWHjO7LSAFrqAWPxAsHfCMKu/+Z41zcuGXscEdkF/cdhc5xJjlMt1l9
xZZIpY1EpnG0AZSKO49foSxiOXJ3d1hzph7Dckt6jATooqUPkea0IEzlWx1Y00EY
KZdtrsTMfYndlqF6x3aaZyzPd8Mtt6ZCcOf3VUMXdhoXB+B37NAVTvoLQ0g7bwmi
g8oWpJOURbU9ciuZnSGiAG02x1Z46OD59C3G9GLr3Ho600E25nybqaIdNV0BK3Ps
EGDpcWyioi15cV3KXZ113XBCDLCV1XfJpLD5OavWxHG6DnY77vtRfc+vhtzIJsD8
EHHUwJ4JRBpELBmXzbs0MDWyCFtA4PZnAnHdiBmUYmabyS+E/3JpwJFF1kDj3SHz
GuSlr/E1M7/2cGxuz7KK
=GFqT
-----END PGP SIGNATURE-----

@maflcko
Copy link
Member

maflcko commented Jan 23, 2018

Note that gcc yells at me for compiling with _FORTIFY_SOURCE (default to on when hardening) and --enable-debug. So maybe we should disable hardening for debug builds, but that clearly is out of scope for this pull request.

@laanwj laanwj merged commit 34328b4 into bitcoin:master Jan 29, 2018
laanwj added a commit that referenced this pull request Jan 29, 2018
@laanwj
Merge #12197: Log debug build status and warn when running benchmarks
000ac4f 
34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 20, 2019
@laanwj @majcosta
Log debug build status and warn when running benchmarks
81c6819 
Summary:
"34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results."

This is a backport from Core PR12197: bitcoin/bitcoin#12197

Test Plan:
  cmake .. -DCMAKE_BUILD_TYPE=Debug -GNinja
  ninja check
  ninja bench-bitcoin

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, nakihito, Fabien

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, nakihito, Fabien

Subscribers: Fabien, nakihito

Differential Revision: https://reviews.bitcoinabc.org/D4687
@sickpig sickpig mentioned this pull request May 5, 2020
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12197: Log debug build status and warn when running ben…
d26173f 
…chmarks

34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12197: Log debug build status and warn when running ben…
73193c7 
…chmarks

34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 17, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12197: Log debug build status and warn when running ben…
cda702f 
…chmarks

34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 18, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12197: Log debug build status and warn when running ben…
fa9fbba 
…chmarks

34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 28, 2021
@laanwj @gades
Merge bitcoin#12197: Log debug build status and warn when running ben…
65e484c 
…chmarks

34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)

Pull request description:

  Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).

  Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.

Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
@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.
Reviewers

@promag promag promag left review comments

@maflcko maflcko maflcko left review comments

Assignees
No one assigned
Labels
Utils/log/libs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@laanwj @maflcko @promag