depends: disable Werror when building zmq

greenaddress commented at 3:32 pm on July 17, 2018: contributor

This PR backports this libzmq commit https://github.com/zeromq/libzmq/pull/3140/commits/58d13395ece1fa0dd9b0583d736af4ac342c1267 from this merged upstream PR (but unreleased as of today) https://github.com/zeromq/libzmq/pull/3140 as a patch to our zmq 4.2.3 version. passes –disable-Werror when we build zmq.

For reference see #11844 (comment)

~~This patch~~ A similar patch is already in use in abcore/bitcoin_ndk and needed to build bitcoind for android using NDK or some versions of clang 6+ during cross compilation.

This patch won’t be necessary once zmqlib releases a 4.2.6+ version including the fix and bitcoin core updates to it but it may be good to have

It also reintroduces autogen.sh as it is necessary for the patch to work, otherwise you’ll have aclocal issues (see https://travis-ci.org/greenaddress/bitcoin/jobs/404902106). autogen,sh was removed in master only #11986 as non-necessary but otherwise unreleased.

In the likely case #13578 is merged first I rebased the patch for the newer zmq version (4.2.5) in https://github.com/greenaddress/bitcoin/tree/zmq-upgrade-mruddy-patched and i can update the PR accordingly or close and open a new one.

DrahtBot commented at 4:06 pm on July 17, 2018: member

#13578 ([depends, zmq, doc] upgrade zeromq to 4.2.5 and avoid deprecated zeromq api functions by mruddy)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

theuni commented at 4:28 pm on July 17, 2018: member

@greenaddress Is this not enough to solve the problem?

 0--- a/depends/packages/zeromq.mk
 1+++ b/depends/packages/zeromq.mk
 2@@ -6,7 +6,7 @@ $(package)_sha256_hash=8f1e2b2aade4dbfde98d82366d61baef2f62e812530160d2e6d0a5bb2
 3 $(package)_patches=0001-fix-build-with-older-mingw64.patch 0002-disable-pthread_set_name_np.patch
 4
 5 define $(package)_set_vars
 6-  $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf
 7+  $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf --disable-Werror
 8   $(package)_config_opts_linux=--with-pic
 9   $(package)_cxxflags=-std=c++11
10 endef

I’m assuming the problem is that newer clang versions emit a new warning which turns to error due to -Werror being on by default. It’s considered bad practice to enable Werror in releases for exactly this reason, so disabling it is for sure a reasonable thing to do.

greenaddress commented at 4:32 pm on July 17, 2018: contributor

@theuni I think it should solve it and it would be preferable indeed. I’ll give it a try and get back to you

greenaddress commented at 5:51 pm on July 17, 2018: contributor

@theuni doesn’t seem to work, at least on 64 bit systems it breaks. see https://travis-ci.org/greenaddress/bitcoin_ndk/builds/405007976 when run with https://github.com/greenaddress/bitcoin_ndk/commit/8a726796f9b96ca5207e5f362e506f968d2d6a40#diff-c8562f9b2c76d2f7f4c4d15dd3e9f474R74

configure: WARNING: unrecognized options: --disable-Werror

Ok it didn’t work cause it only became available in 4.2.3 and not in 4.2.2 (used by 0.16.2) https://github.com/zeromq/libzmq/blob/v4.2.2/configure.ac

I’ll test it against master and get back…

greenaddress commented at 6:04 pm on July 17, 2018: contributor

Kicked a test against master with the same patch you suggested running against ndk

https://travis-ci.org/greenaddress/bitcoin_ndk/builds/405019232

depends: disable Werror for zmqlib release, causes ndk build to break a4ba2388fe

greenaddress force-pushed on Jul 17, 2018

greenaddress commented at 6:25 pm on July 17, 2018: contributor

@theuni your suggestions seems to work fine at least in master so it is a better fix, I updated this PR.

greenaddress renamed this:
~~depends: backport fix zmq build with clang 6 on crossbuild~~
depends: disable Werror for zmqlib release, fixes some issues with clang 6+
on Jul 17, 2018

theuni commented at 6:47 pm on July 17, 2018: member

@greenaddress Thanks, looks good now.

Are we confident, though, that this isn’t masking a legitimate bug?

fanquake added the label Build system on Jul 17, 2018

greenaddress commented at 11:46 pm on July 17, 2018: contributor

@theuni even if it isn’t now it could in the future

fanquake renamed this:
~~depends: disable Werror for zmqlib release, fixes some issues with clang 6+~~
depends: disable Werror when building zmq
on Jul 18, 2018

laanwj commented at 2:07 pm on July 18, 2018: member

I’m assuming the problem is that newer clang versions emit a new warning which turns to error due to -Werror being on by default. It’s considered bad practice to enable Werror in releases for exactly this reason, so disabling it is for sure a reasonable thing to do.

agree, Werror is for development, not for releases, it makes builds fragile.

utACK a4ba2388feda0be551daca842984c100f002ea81

fanquake commented at 1:52 am on July 19, 2018: member

utACK a4ba238 @theuni Could you give a last ACK here as well.

theuni commented at 5:23 pm on July 19, 2018: member

utACK a4ba2388feda0be551daca842984c100f002ea81

MarcoFalke merged this on Jul 19, 2018

MarcoFalke closed this on Jul 19, 2018

MarcoFalke referenced this in commit 8c36432791 on Jul 19, 2018

greenaddress deleted the branch on Jul 20, 2018

PastaPastaPasta referenced this in commit bcdd83da7c on Jun 9, 2020

PastaPastaPasta referenced this in commit 274911cbb9 on Jun 9, 2020

PastaPastaPasta referenced this in commit 7b0df1dd89 on Jun 10, 2020

PastaPastaPasta referenced this in commit e9d78ed017 on Jun 10, 2020

PastaPastaPasta referenced this in commit cd96e52b4d on Jun 11, 2020

PastaPastaPasta referenced this in commit 1ee1074e22 on Jun 11, 2020

PastaPastaPasta referenced this in commit b31cb3a365 on Jun 12, 2020

PastaPastaPasta referenced this in commit e84e74cab5 on Jun 14, 2020

gades referenced this in commit 25565e3b1e on Jun 24, 2021

CryptoCentric referenced this in commit 7d0e2fe99b on Jul 2, 2021

MarcoFalke locked this on Sep 8, 2021

depends: disable Werror when building zmq #13689