This prevents Travis from failing if any of the dependencies have e.g. unused variables: https://travis-ci.org/bitcoin/bitcoin/jobs/613924094#L10291 (I ran into this particular unused variable in #15382, and it has been fixed in Boost System 1.71)
https://stackoverflow.com/a/1900578/313633
Alternative is to revert #17486, but I’d rather not.
Oh crap, I knww there was a reason the unused variable check wasn’t enabled.
I remember there has been some resistance against using -isystem in the past (and has been tried to be introduced many times, unsuccesfully: https://github.com/bitcoin/bitcoin/pulls?utf8=%E2%9C%93&q=is%3Apr+isystem ), but can’t find the actual reasoning right now.
#14920 goes beyond this PR by applying -isystem to all dependencies, not just depends/. But actually that makes sense: I just caught the same error without depends on one of my machines that still had Boost 1.70.
It puts this behind --enable-isystem and a warning: Including /usr/include with -isystem is known to cause problems, e.g. breaking include_next due to, the directive changing inclusion order.. This PR doesn’t do that, but that’s because it’s incomplete.
@theuni expressed some concern about it: #14920 (comment)
I’m really not a fan of disabling warnings for 3rd party headers, but I’m not completely opposed. I’m more afraid that this will be one of those changes that causes lots of subtle future grief due to edge-cases in libtool, automake, ccache, non-glibc builds, dependency generation (gcc’s “-M*” options), etc.
I gave a few examples above about possible weird edge cases. Qt demonstrates another with include_next. Non-system toolchains and sysroots containing -I are two others.
I’m really uneasy about this due to the diversity of compilation environments. But other than the things I pointed out, it looks ok to me on paper.
On second thought, as long as it’s off by default, concept ACK.
That probably means reverting the Werror part of #17486 and re-applying it after #14920, but moving it under the use_isystem" = "xyes" check.
| File | commit b4a1da9ef8e4b673c290d5b882527e627ae1b43a(master) | commit ea87f806b17977cd3fc310177e62c6c2aeebb3a6(master and this pull) |
|---|---|---|
| bitcoin-0.19.99-osx-unsigned.dmg | 2aac9afb80675e6b... |
1f7397c66a02e6ee... |
| bitcoin-0.19.99-osx64.tar.gz | fca5c093497da768... |
4c20d52afcedbd8d... |
| bitcoin-0.19.99-win64-debug.zip | 7a0b53fcba538c85... |
96d7787ff1272a97... |
| bitcoin-0.19.99-win64-setup-unsigned.exe | 0dcf41e68351b5d3... |
336e9ead7f07eeb4... |
| bitcoin-0.19.99-win64.zip | c8d229696feb714f... |
21b9daebdd844b3d... |
| bitcoin-0.19.99.tar.gz | 0fbc1c4e20f88988... |
03e686d95879c689... |
| bitcoin-core-osx-0.20-res.yml | a1bd02b0758e2e04... |
d9c7614d7347f224... |
| bitcoin-core-win-0.20-res.yml | 6dba0dd84407549e... |
bac1ac54b448fc44... |
| linux-build.log | d1ee3974fc3a4437... |
5d24a048c1d72a7b... |
| osx-build.log | 0ea76b9d6b91529b... |
16e4799b979ace8c... |
| win-build.log | c66e143a3b0cf570... |
08e8c25b80bdab0d... |
| bitcoin-core-osx-0.20-res.yml.diff | cfbee055d4908336... |
|
| bitcoin-core-win-0.20-res.yml.diff | 5e05bc5578367390... |
|
| linux-build.log.diff | 96d2845d8385e08d... |
|
| osx-build.log.diff | 7073047728d112fc... |
|
| win-build.log.diff | 1c06c5db99a6d56e... |
