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.

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

build: quiet annoying warnings without adding new ones #7954

Merged
merged 1 commit into from Apr 27, 2016

Conversation

Projects
None yet
5 participants
@theuni
Member

theuni commented Apr 27, 2016

addresses #7394 and some of #7902

Disabling warnings can be tricky, because doing so can cause a different
compiler to create new warnings about unsupported disable flags. Also, some
warnings don't surface until they're paired with another warning (gcc). For
example, adding "-Wno-foo" won't cause any trouble, but if there's a legitimate
warning emitted, the "unknown option -Wno-foo" will show up as well.

Work around this in 2 ways:

  1. When checking to see if -Wno-foo is supported, check for "-Wfoo" instead.
  2. Enable -Werror while checking 1.

If "-Werror -Wfoo" compiles, "-Wno-foo" is almost guaranteed to be supported.

-Werror itself is also checked. If that fails to compile by itself, it likely
means that the user added a flag that adds a warning. In that case, -Werror
won't be used while checking, and the build may be extra noisy. The user would
need to fix the bad input flag.

Also, silence 2 more additional warnings that can show up post-c++11.

build: quiet annoying warnings without adding new ones
Disabling warnings can be tricky, because doing so can cause a different
compiler to create new warnings about unsupported disable flags. Also, some
warnings don't surface until they're paired with another warning (gcc). For
example, adding "-Wno-foo" won't cause any trouble, but if there's a legitimate
warning emitted, the "unknown option -Wno-foo" will show up as well.

Work around this in 2 ways:

1. When checking to see if -Wno-foo is supported, check for "-Wfoo" instead.
2. Enable -Werror while checking 1.

If "-Werror -Wfoo" compiles, "-Wno-foo" is almost guaranteed to be supported.

-Werror itself is also checked. If that fails to compile by itself, it likely
means that the user added a flag that adds a warning. In that case, -Werror
won't be used while checking, and the build may be extra noisy. The user would
need to fix the bad input flag.

Also, silence 2 more additional warnings that can show up post-c++11.
@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 27, 2016

Contributor

ACK 63b3111

Contributor

paveljanik commented Apr 27, 2016

ACK 63b3111

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Apr 27, 2016

Member

Tested ACK 63b3111

Member

btcdrak commented Apr 27, 2016

Tested ACK 63b3111

@fanquake

This comment has been minimized.

Show comment
Hide comment
@fanquake

fanquake Apr 27, 2016

Member

ACK 63b3111

Diff of the output of ./autogen.sh + ./configure --enable-hardening with the patch.

@@ -163,6 +163,15 @@ checking for readelf... no
 checking for c++filt... /usr/bin/c++filt
 checking for pkg-config... /usr/local/bin/pkg-config
 checking pkg-config is at least version 0.9.0... yes
-checking whether C++ compiler accepts -Werror... yes
-checking whether C++ compiler accepts -Wall... yes
-checking whether C++ compiler accepts -Wextra... yes
-checking whether C++ compiler accepts -Wformat... yes
-checking whether C++ compiler accepts -Wformat-security... yes
-checking whether C++ compiler accepts -Wunused-parameter... yes
-checking whether C++ compiler accepts -Wself-assign... yes
-checking whether C++ compiler accepts -Wunused-local-typedef... yes
-checking whether C++ compiler accepts -Wdeprecated-register... yes
 checking for port... no
 checking for brew... brew
 checking whether the linker accepts -Wl,-headerpad_max_install_names... yes
Member

fanquake commented Apr 27, 2016

ACK 63b3111

Diff of the output of ./autogen.sh + ./configure --enable-hardening with the patch.

@@ -163,6 +163,15 @@ checking for readelf... no
 checking for c++filt... /usr/bin/c++filt
 checking for pkg-config... /usr/local/bin/pkg-config
 checking pkg-config is at least version 0.9.0... yes
-checking whether C++ compiler accepts -Werror... yes
-checking whether C++ compiler accepts -Wall... yes
-checking whether C++ compiler accepts -Wextra... yes
-checking whether C++ compiler accepts -Wformat... yes
-checking whether C++ compiler accepts -Wformat-security... yes
-checking whether C++ compiler accepts -Wunused-parameter... yes
-checking whether C++ compiler accepts -Wself-assign... yes
-checking whether C++ compiler accepts -Wunused-local-typedef... yes
-checking whether C++ compiler accepts -Wdeprecated-register... yes
 checking for port... no
 checking for brew... brew
 checking whether the linker accepts -Wl,-headerpad_max_install_names... yes

@laanwj laanwj added the Build system label Apr 27, 2016

@laanwj laanwj merged commit 63b3111 into bitcoin:master Apr 27, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 27, 2016

Merge #7954: build: quiet annoying warnings without adding new ones
63b3111 build: quiet annoying warnings without adding new ones (Cory Fields)

kyuupichan referenced this pull request in kyuupichan/BitcoinUnlimited Apr 8, 2017

Merge #7954: build: quiet annoying warnings without adding new ones
63b3111 build: quiet annoying warnings without adding new ones (Cory Fields)

@kyuupichan kyuupichan referenced this pull request Apr 8, 2017

Merged

Configure backports #433

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7954: build: quiet annoying warnings without adding new ones
63b3111 build: quiet annoying warnings without adding new ones (Cory Fields)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7954: build: quiet annoying warnings without adding new ones
63b3111 build: quiet annoying warnings without adding new ones (Cory Fields)

@str4d str4d referenced this pull request Dec 1, 2017

Merged

Build system improvements #2786

zkbot added a commit to zcash/zcash that referenced this pull request Dec 1, 2017

Auto merge of #2786 - str4d:2074-build, r=<try>
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.

zkbot added a commit to zcash/zcash that referenced this pull request Dec 1, 2017

Auto merge of #2786 - str4d:2074-build, r=<try>
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.

zkbot added a commit to zcash/zcash that referenced this pull request Dec 15, 2017

Auto merge of #2786 - str4d:2074-build, r=str4d
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.

kotodev added a commit to koto-dev/koto.old that referenced this pull request Jan 25, 2018

Auto merge of #2786 - str4d:2074-build, r=str4d
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.

renium9 added a commit to koto-dev/koto.old that referenced this pull request Feb 6, 2018

Auto merge of #2786 - str4d:2074-build, r=str4d
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.

# Conflicts:
#	configure.ac
#	src/Makefile.am
#	src/Makefile.gtest.include
#	src/Makefile.test.include
#	zcutil/build.sh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment