depends: disable Werror when building zmq #13689

pull greenaddress wants to merge 1 commits into bitcoin:master from greenaddress:zmq_clang_depends changing 1 files +1 −1
  1. 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.

  2. greenaddress cross-referenced this on Jul 17, 2018 from issue [depends, zmq, doc] upgrade zeromq to 4.2.5 and avoid deprecated zeromq api functions by mruddy
  3. greenaddress cross-referenced this on Jul 17, 2018 from issue [depends] zeromq 4.2.3 by fanquake
  4. DrahtBot commented at 4:06 PM on July 17, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->Note to reviewers: This pull request conflicts with the following ones:

    • #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.

  5. theuni commented at 4:28 PM on July 17, 2018: member

    @greenaddress Is this not enough to solve the problem?

    --- a/depends/packages/zeromq.mk
+++ b/depends/packages/zeromq.mk
@@ -6,7 +6,7 @@ $(package)_sha256_hash=8f1e2b2aade4dbfde98d82366d61baef2f62e812530160d2e6d0a5bb2
 $(package)_patches=0001-fix-build-with-older-mingw64.patch 0002-disable-pthread_set_name_np.patch

 define $(package)_set_vars
-  $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf
+  $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf --disable-Werror
   $(package)_config_opts_linux=--with-pic
   $(package)_cxxflags=-std=c++11
 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.

  6. 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

  7. 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...

  8. 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

  9. depends: disable Werror for zmqlib release, causes ndk build to break a4ba2388fe
  10. greenaddress force-pushed on Jul 17, 2018
  11. 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.

  12. 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
  13. 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?

  14. fanquake added the label Build system on Jul 17, 2018
  15. greenaddress commented at 11:46 PM on July 17, 2018: contributor

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

  16. 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
  17. greenaddress cross-referenced this on Jul 18, 2018 from issue Bitcoin core build on Android NDK by laanwj
  18. 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

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

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

  20. theuni commented at 5:23 PM on July 19, 2018: member

    utACK a4ba2388feda0be551daca842984c100f002ea81

  21. MarcoFalke merged this on Jul 19, 2018
  22. MarcoFalke closed this on Jul 19, 2018
  23. MarcoFalke referenced this in commit 8c36432791 on Jul 19, 2018
  24. greenaddress deleted the branch on Jul 20, 2018
  25. Bushstar cross-referenced this on Jul 24, 2018 from issue commits from bitcoin/master by Bushstar
  26. UdjinM6 cross-referenced this on May 18, 2020 from issue depends: add backports 11986 12402 13543 by 10xcryptodev
  27. PastaPastaPasta referenced this in commit bcdd83da7c on Jun 9, 2020
  28. PastaPastaPasta referenced this in commit 274911cbb9 on Jun 9, 2020
  29. PastaPastaPasta referenced this in commit 7b0df1dd89 on Jun 10, 2020
  30. PastaPastaPasta referenced this in commit e9d78ed017 on Jun 10, 2020
  31. PastaPastaPasta referenced this in commit cd96e52b4d on Jun 11, 2020
  32. PastaPastaPasta referenced this in commit 1ee1074e22 on Jun 11, 2020
  33. PastaPastaPasta referenced this in commit b31cb3a365 on Jun 12, 2020
  34. PastaPastaPasta referenced this in commit e84e74cab5 on Jun 14, 2020
  35. gades referenced this in commit 25565e3b1e on Jun 24, 2021
  36. CryptoCentric referenced this in commit 7d0e2fe99b on Jul 2, 2021
  37. bitcoin locked this on Sep 8, 2021
Contributors
greenaddressDrahtBottheunilaanwjfanquake
Labels
Build system
Linked (view graph)
#11844 Bitcoin core build on Android NDK#11986 [depends] zeromq 4.2.3#13578 [depends, zmq, doc] upgrade zeromq to 4.2.5 and avoid deprecated zeromq api functions
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC