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

pull mruddy wants to merge 1 commits into bitcoin:master from mruddy:zmq-upgrade changing 7 files +33 −27
  1. mruddy commented at 2:54 PM on June 30, 2018: contributor

    Upgrade the ZeroMQ dependency from version 4.2.3 to the latest stable version 4.2.5.

    This PR Follows the lead of #11986.

    I upgraded both patch files to correspond to the version 4.2.5 libzmq files. I assume doing so is still necessary and correct.

    Without updating the patch line numbers, things appear to work, but you get extra log messages while building depends because things don't exactly match, e.g.:

    /bitcoin/depends> make zeromq
Extracting zeromq...
/bitcoin/depends/sources/zeromq-4.2.5.tar.gz: OK
Preprocessing zeromq...
patching file src/windows.hpp
Hunk [#1](/github-metadata-backup-bitcoin-bitcoin/1/) succeeded at 58 (offset 3 lines).
patching file src/thread.cpp
Hunk [#1](/github-metadata-backup-bitcoin-bitcoin/1/) succeeded at 307 with fuzz 2 (offset 87 lines).
Hunk [#2](/github-metadata-backup-bitcoin-bitcoin/2/) succeeded at 323 with fuzz 2 (offset 90 lines).

    Updating the patches seemed cleaner, so I did it. Note that libzmq had some whitespace changes, so that's why the updated patches do too.

    More info: https://github.com/zeromq/libzmq/releases/tag/v4.2.5

    tags: libzmq, zmq, 0mq

  2. fanquake added the label Build system on Jul 1, 2018
  3. fanquake commented at 4:37 AM on July 1, 2018: member

    @mruddy What's the motivation for bumping; is there a specific change/bugfix included in 4.2.3->4.2.5 that you require?

    I've had a look at the patches, updating line numbers and formatting from upstream is fine.

    Looks like we still also require the postprocess sed, as Libs.private: -lstdc++ is still included in 4.2.5's libzmq.pc.

    Which platforms have you tested depends builds/gitian building on? Last time we updated zeromq we were stung with a sneaky change that broke gitian building, hence the need of the sed above.

  4. in depends/patches/zeromq/0001-fix-build-with-older-mingw64.patch:1 in db8096aa8e outdated 
       0 | @@ -1,17 +1,17 @@
   1 | -From 1a159c128c69a42d90819375c06a39994f3fbfc1 Mon Sep 17 00:00:00 2001
    fanquake commented at 4:38 AM on July 1, 2018:

    Given that these are just being updated for upstream, you could leave the original From/Date

    mruddy commented at 12:47 PM on July 1, 2018:

    the date didn't change. the thing that changed was the hex value, which is only the local commit made to my libzmq repo so that i could run git format-patch master --stdout to get a patch that had most of the same formatting as the original one. it's a pretty meaningless line. i was actually thinking of just using the git diff output for the patch file contents as that would make these diffs easier in the future, but then you lose the commit comment being in the patch file.

  5. mruddy commented at 12:36 PM on July 1, 2018: contributor

    @fanquake My primary motivation is that my dev machine is Ubuntu 18.04 LTS and its libzmq5 package is already v4.2.5. Without building against depends, I was already using 4.2.5. Since I did the looking to figure that out, I figured that I'd make this update. This is the only machine where I tested the depends build as well.

    From the 4.2.4 release notes, this one looked a little interesting. Although, I did NOT try to use it to make my node crash, so I don't know if it actually applies: "ZMQ_PUB crash when due to high volume of subscribe and unsubscribe messages, an unmatched unsubscribe message is received in certain conditions".

  6. fanquake cross-referenced this on Jul 5, 2018 from issue zmq: update to avoid deprecated zeromq api functions and log zmq version used by mruddy
  7. mruddy force-pushed on Jul 6, 2018
  8. mruddy renamed this:
    [depends] zeromq 4.2.5
    [depends, zmq, doc] upgrade zeromq to 4.2.5 and avoid deprecated zeromq api functions
    on Jul 6, 2018
  9. mruddy commented at 3:28 AM on July 6, 2018: contributor

    Combined #13596 into this PR. In summary:

    • Upgrade ZeroMQ from 4.2.3 to 4.2.5 (latest stable).
    • Centralize ZeroMQ version documentation into dependencies.md.
    • Note the already existing minimum ZMQ version requirement of 4.0.0 (enforced by configure.ac) which already supports switching to use the non-deprecated API functions.
    • Log what version of the ZMQ lib (libzmq) is used by the node in the zmq debug log messages.
  10. DrahtBot cross-referenced this on Jul 6, 2018 from issue depends: Add RISC-V support by laanwj
  11. mruddy commented at 8:51 PM on July 16, 2018: contributor

    Just noting that I think this PR is ready to merge. The current latest commit 473a48cae5d63f5c0baee4990756d4a9b18c4b5f covers all feedback received.

  12. jmcorgan commented at 9:50 PM on July 16, 2018: contributor

    utACK

  13. greenaddress cross-referenced this on Jul 17, 2018 from issue [depends] zeromq 4.2.3 by fanquake
  14. greenaddress cross-referenced this on Jul 17, 2018 from issue depends: disable Werror when building zmq by greenaddress
  15. greenaddress commented at 3:32 PM on July 17, 2018: contributor

    I know is late for this but it would be great if we could also add this commit as a patch https://github.com/zeromq/libzmq/pull/3140/commits/58d13395ece1fa0dd9b0583d736af4ac342c1267

    If you want to squash it in your PR i rebased it against yours in https://github.com/greenaddress/bitcoin/tree/zmq-upgrade-mruddy-patched

    I also prepared a separate PR in https://github.com/bitcoin/bitcoin/pull/13689

  16. laanwj commented at 2:17 PM on July 18, 2018: member

    utACK 473a48cae5d63f5c0baee4990756d4a9b18c4b5f

  17. laanwj commented at 2:19 PM on July 18, 2018: member

    I think in general, though, it makes sense to avoid small version bumps for libraries such as zeromq unless there's a strong motivation to do so (such as a bug that affects us). This PR is good, but let's not make it a habit.

  18. mruddy commented at 9:22 PM on July 18, 2018: contributor

    @laanwj OK and thanks for reviewing this. @greenaddress Just wanted to let you know that I'm not ignoring you, I've just been following the conversation over at #13689. I think I'd rather just leave them separate since this PR already has some ACKs and it looks like you're pretty well done with your changes in the other PR too. Thanks for your review too.

  19. MarcoFalke referenced this in commit 8c36432791 on Jul 19, 2018
  20. DrahtBot cross-referenced this on Aug 2, 2018 from issue doc: correct versions in dependencies.md by fanquake
  21. DrahtBot commented at 4:08 PM on August 2, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->No more conflicts as of last run.

  22. DrahtBot added the label Needs rebase on Aug 6, 2018
  23. DrahtBot commented at 4:10 PM on August 6, 2018: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->Needs rebase

  24. mruddy force-pushed on Aug 6, 2018
  25. mruddy commented at 9:14 PM on August 6, 2018: contributor

    rebased

  26. fanquake removed the label Needs rebase on Aug 6, 2018
  27. MarcoFalke added the label Needs gitian build on Aug 16, 2018
  28. MarcoFalke removed the label Needs gitian build on Aug 16, 2018
  29. MarcoFalke added the label Needs gitian build on Aug 17, 2018
  30. MarcoFalke removed the label Needs gitian build on Aug 18, 2018
  31. MarcoFalke added the label Needs gitian build on Aug 18, 2018
  32. DrahtBot cross-referenced this on Aug 21, 2018 from issue ZMQ: Use C++ language bindings by domob1812
  33. DrahtBot removed the label Needs gitian build on Aug 22, 2018
  34. mruddy commented at 11:59 AM on August 23, 2018: contributor

    This is ready to merge.

  35. in doc/build-unix.md:50 in 33d997d667 outdated 
      46 | @@ -47,7 +47,7 @@ Optional dependencies:
  47 |   protobuf    | Payments in GUI  | Data interchange format used for payment protocol (only needed when GUI enabled)
  48 |   libqrencode | QR codes in GUI  | Optional for generating QR codes (only needed when GUI enabled)
  49 |   univalue    | Utility          | JSON parsing and encoding (bundled version will be used unless --with-system-univalue passed to configure)
  50 | - libzmq3     | ZMQ notification | Optional, allows generating ZMQ notifications (requires ZMQ version >= 4.x)
  51 | + libzmq3     | ZMQ notification | Optional, allows generating ZMQ notifications
    ken2812221 commented at 12:57 PM on August 23, 2018:

    Why change this?

    mruddy commented at 4:15 PM on August 23, 2018:

    it's already centrally located in doc/dependencies.md. this removes redundant info that can get out of synch later.

    laanwj commented at 12:56 PM on August 31, 2018:

    I'm not convinced that removing this information everywhere is a good idea. Yes, you are correct that the information is in dependencies.md, however I still think, for users actually reading this documentation, it is useful to mention the major version requirement in other places too.

    mruddy commented at 8:37 PM on September 11, 2018:

    ok, i've mostly re-duplicated the version documentation and rebased and squashed it all again.

  36. in doc/build-unix.md:96 in 33d997d667 outdated 
      92 | @@ -93,7 +93,7 @@ Optional (see --with-miniupnpc and --enable-upnp-default):
  93 |  
  94 |      sudo apt-get install libminiupnpc-dev
  95 |  
  96 | -ZMQ dependencies (provides ZMQ API 4.x):
  97 | +ZMQ dependencies (provides ZMQ API):
    ken2812221 commented at 12:58 PM on August 23, 2018:

    Why change this?

    mruddy commented at 4:15 PM on August 23, 2018:

    it's already centrally located in doc/dependencies.md. this removes redundant info that can get out of synch later.

  37. in doc/zmq.md:36 in 33d997d667 outdated 
      32 | @@ -33,8 +33,10 @@ buffering or reassembly.
  33 |  
  34 |  ## Prerequisites
  35 |  
  36 | -The ZeroMQ feature in Bitcoin Core requires ZeroMQ API version 4.x or
  37 | -newer. Typically, it is packaged by distributions as something like
  38 | +The ZeroMQ feature in Bitcoin Core requires the ZeroMQ API
    ken2812221 commented at 1:00 PM on August 23, 2018:

    Why get rid of version limit?

    mruddy commented at 4:15 PM on August 23, 2018:

    it's already centrally located in doc/dependencies.md. this removes redundant info that can get out of synch later.

  38. in doc/zmq.md:83 in 33d997d667 outdated 
      79 | @@ -78,7 +80,7 @@ bytes).
  80 |  These options can also be provided in bitcoin.conf.
  81 |  
  82 |  ZeroMQ endpoint specifiers for TCP (and others) are documented in the
  83 | -[ZeroMQ API](http://api.zeromq.org/4-0:_start).
  84 | +[ZeroMQ API](http://api.zeromq.org/).
    ken2812221 commented at 1:01 PM on August 23, 2018:

    Why change this?

    mruddy commented at 4:15 PM on August 23, 2018:

    it's already centrally located in doc/dependencies.md. this removes redundant info that can get out of synch later.

  39. ken2812221 changes_requested
  40. ken2812221 commented at 1:03 PM on August 23, 2018: contributor

    Maybe you did something wrong when you rebase this. Otherwise utACK

  41. [depends, zmq, doc] upgrade zeromq to 4.2.5 and avoid deprecated zeromq api functions f1bd03eb01
  42. mruddy force-pushed on Sep 11, 2018
  43. ken2812221 commented at 12:04 AM on September 17, 2018: contributor

    utACK f1bd03e

  44. laanwj merged this on Sep 17, 2018
  45. laanwj closed this on Sep 17, 2018
  46. laanwj referenced this in commit 2d4749b366 on Sep 17, 2018
  47. mruddy deleted the branch on Sep 17, 2018
  48. bitcoin deleted a comment on Sep 17, 2018
  49. Bushstar cross-referenced this on Oct 11, 2018 from issue Updates from bitcoin/master by Bushstar
  50. deadalnix referenced this in commit e2dd836231 on Mar 31, 2020
  51. PastaPastaPasta referenced this in commit bcdd83da7c on Jun 9, 2020
  52. PastaPastaPasta referenced this in commit 274911cbb9 on Jun 9, 2020
  53. PastaPastaPasta referenced this in commit 7b0df1dd89 on Jun 10, 2020
  54. PastaPastaPasta referenced this in commit e9d78ed017 on Jun 10, 2020
  55. PastaPastaPasta referenced this in commit cd96e52b4d on Jun 11, 2020
  56. PastaPastaPasta referenced this in commit 1ee1074e22 on Jun 11, 2020
  57. PastaPastaPasta referenced this in commit b31cb3a365 on Jun 12, 2020
  58. PastaPastaPasta referenced this in commit e84e74cab5 on Jun 14, 2020
  59. str4d referenced this in commit edfb4d98e7 on Oct 5, 2020
  60. str4d cross-referenced this on Oct 5, 2020 from issue Update ZeroMQ by str4d
  61. zkbot referenced this in commit e3d5ddbef4 on Oct 8, 2020
  62. zkbot referenced this in commit 6e4090d840 on Oct 8, 2020
  63. gades referenced this in commit 25565e3b1e on Jun 24, 2021
  64. CryptoCentric referenced this in commit 7d0e2fe99b on Jul 2, 2021
  65. ftrader referenced this in commit 323a004923 on Aug 13, 2021
  66. dzutto referenced this in commit 6390d05eaf on Sep 7, 2021
  67. bitcoin locked this on Sep 8, 2021
Contributors
mruddyfanquakejmcorgangreenaddresslaanwjDrahtBotken2812221
Labels
Build system
Linked (view graph)
#1 JSON-RPC support for mobile devices ("ultra-lightweight" clients)#2 Long-term, safe, store-of-value#11981 Fix gitian build after libzmq bump#11986 [depends] zeromq 4.2.3#13596 zmq: update to avoid deprecated zeromq api functions and log zmq version used#13689 depends: disable Werror when building zmq
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