build: undo Werror=unused-variable

Sjors commented at 11:50 am on November 20, 2019: member

Partial revert of #17486.

First we need to avoid tripping this check whenever a dependency introduces an unused variable.

#14920 tackles this

[build]: undo Werror=unused-variable

Partial revert of 18b18f8e813b332d0ce6b0af19c9cb265f043776. First we need to avoid tripping this check whenever a dependency introduces an unused variable.

f327e82b02

laanwj added the label Build system on Nov 20, 2019

laanwj commented at 12:01 pm on November 20, 2019: member

ACK f327e82b0266b9ac455c8b82434528d670eee6a8

fanquake commented at 3:29 pm on November 20, 2019: member

What is actually failing with -Werror=unused-variable? I can build depends and configure master (76e777df83db4fc14d5cf96c67ceaa554a016a3a) with --enable-werror and I’m not seeing any issues.

MarcoFalke commented at 4:17 pm on November 20, 2019: member

@fanquake It depends on what libraries your system comes with. E.g. it fails when your system ships a “broken” boost: https://travis-ci.org/bitcoin/bitcoin/jobs/613924094#L10291

fanquake commented at 4:44 pm on November 20, 2019: member

@MarcoFalke Right, but that failure isn’t using system Boost, it’s building Boost from depends. So are we assuming that builds for people using master and system libraries are also currently failing when using --enable-werror? It would be good to get some examples.

The problem this PR is “fixing”, which is unclear from the description, is that 15382 introduces a dependency on Boost Process, and in Boost 1.70.0 (used in depends) there is an unused variable in the process module boost/process/detail/posix/wait_group.hpp:140:13: error: unused variable 'ret_sig', which is supposedly fixed in Boost 1.71.0.

I just want to be clear why this is being reverted, because when it was introduced in #17486, depends and system libs seemed to be building fine under --enable-werror (and seemingly still are). However now we’re going to revert part of the change because a PR with 1 (outdated) ACK, that introduces a new Boost dependency, is failing to build under --enable-werror.

MarcoFalke commented at 4:55 pm on November 20, 2019: member

I’d presume that this also fails when a system library is broken, but I haven’t checked.

Sjors commented at 5:23 pm on November 21, 2019: member

In my experience it only failed on a PR that started using a previously unused part of Boost #15382. We could of course leave this PR open until it actually breaks something on Travis. Maybe #14920 is merged before that’s needed.

Right, but that failure isn’t using system Boost, it’s building Boost from depends.

I can also produce that failure with system Boost in that PR, but only 1.70; it was fixed in 1.71

MarcoFalke added the label Needs gitian build on Nov 25, 2019

DrahtBot commented at 1:25 am on November 27, 2019: member

Gitian builds

File	commit 2c1c43754b7a29e2fc852c2ad6430ba27aa2d7db(master)	commit bd6dd0572facc8af3e4e3209572540a275c1cf82(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`38cbfb45b6a7086d...`	`0b108c1fe78b067f...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`da5cf67a007aed08...`	`c188d363efa52285...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`184e8ce64969a801...`	`13c507e29344c9cc...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`281f743a1af9f788...`	`841761093972c3c7...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`094e8232c0b5fc1e...`	`b5324da409d60b10...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`ab4deb19e22c2e9c...`	`4cd79d1100dbec48...`
bitcoin-0.19.99-osx-unsigned.dmg	`bd9ae85f63baebb0...`	`31f1d276a3432f14...`
bitcoin-0.19.99-osx64.tar.gz	`61305776c2b32f02...`	`b99afa237f0b863c...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`9c9b077e4c62ca11...`	`82e65d416f47aa9b...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`b33148372a530635...`	`f351f8c4a1a434f7...`
bitcoin-0.19.99-win64-debug.zip	`20a7fd94fde51027...`	`d80562a2f1260b8c...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`94be28f0f2a7d072...`	`8e7129235de79dc5...`
bitcoin-0.19.99-win64.zip	`77303c3b0e4d270b...`	`64b911a986965bce...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`41dec10c82bf5058...`	`eb26fa7e61165f09...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`ee561a8a74b45caa...`	`5602369cd7fb4a54...`
bitcoin-0.19.99.tar.gz	`a2c202a483d1f9e1...`	`365f3441bddd1734...`
bitcoin-core-linux-0.20-res.yml	`5d71778589f1dbbe...`	`0ee38fcdc5d35eed...`
bitcoin-core-osx-0.20-res.yml	`fefe01def2f61fe3...`	`569f80cc254a8c55...`
bitcoin-core-win-0.20-res.yml	`c6d1798223f3d9aa...`	`53b799f95d99d4fd...`
linux-build.log	`7babaa31c8e7e008...`	`3f4898aed811fcbf...`
osx-build.log	`44cec8cec24a7204...`	`4c1d8070c3d9c636...`
win-build.log	`11636a6fb5df52aa...`	`62423ae42b76f6d5...`
bitcoin-core-linux-0.20-res.yml.diff		`a6235a1ce8dc386f...`
bitcoin-core-osx-0.20-res.yml.diff		`c23c75a1a8f7ba11...`
bitcoin-core-win-0.20-res.yml.diff		`dd9d738078e57eda...`
linux-build.log.diff		`4e8a44feab7abe16...`
osx-build.log.diff		`9c8ddbe6165a2b7b...`
win-build.log.diff		`4e0c17fdf455167d...`

DrahtBot removed the label Needs gitian build on Nov 27, 2019

fanquake commented at 4:36 am on January 5, 2020: member

Going to close this. If this change is required by a PR (#15382), it should be included in that PR (looks like it is), rather than applied to the build system in advance. Also see my comment above; #17533 (comment). #15382 could also be taking other approaches, like at least patching boost process in depends, rather than reverting a good, project-wide build system change because a single optional function causes issues.

fanquake closed this on Jan 5, 2020

Sjors deleted the branch on Jan 5, 2020

DrahtBot locked this on Feb 15, 2022

build: undo Werror=unused-variable #17533

Gitian builds