Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jul 23, 2024

This is a backport of: zeromq/libzmq#4706.

Similar to #30488.

Addresses #29723 (comment):

Looking at the mingw .pc generated by this PR:

Libs: -L${libdir} -lzmq
Libs.private:
Requires.private:

It looks like we'll need to take zeromq/libzmq#4706 as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 23, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK fanquake
Stale ACK theuni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK and utACK. I'd rather wait until upstream weighs in though, since we don't need this at least until we switch to CMake.

hebasto added a commit to hebasto/bitcoin that referenced this pull request Jul 24, 2024
00355fd depends: switch zmq to CMake (Cory Fields)
5989754 depends: add zeromq no librt patch (fanquake)
b4c5f72 depends: add zeromq cmake minimum patch (fanquake)
caae50a depends: add zeromq windows usage patch (fanquake)
1b55c75 depends: add zeromq builtin sha1 patch (fanquake)
73eda12 depends: add zeromq mktemp macos patch (fanquake)
30e258d fixup! cmake: Add `libzmq` optional package support (Hennadii Stepanov)
fcc075d Revert "depends: Fix Autotools-generated `libzmq.pc` file" (Hennadii Stepanov)

Pull request description:

  This PR ports bitcoin#29723.

  It aims to avoid conflicts when pushing the `cmake-staging` branch as an update of bitcoin#30454.

  Also a documenting comment added in regard to bitcoin#30508.

Top commit has no ACKs.

Tree-SHA512: 7976db24bd347d7ce7d63cbe7a6c6544009a93fdfb1ffc7709f2711a47c5a6c7d0bec3c03fb236a4c52ced01bfd69a778a7373484152157b359ae643a474dc00
@fanquake
Copy link
Member

I'd rather wait until upstream weighs in though, since we don't need this at least until we switch to CMake.

Looks like upstream either forgot about, or missed this, so I guess someone should followup.

@maflcko
Copy link
Member

maflcko commented Aug 21, 2024

Could rebase the upstream pull, which currently fails CI, to pick up the upstream CI fixes?

@hebasto
Copy link
Member Author

hebasto commented Aug 21, 2024

Could rebase the upstream pull, which currently fails CI, to pick up the upstream CI fixes?

Done.

@hebasto
Copy link
Member Author

hebasto commented Aug 21, 2024

I'd rather wait until upstream weighs in though, since we don't need this at least until we switch to CMake.

Looks like upstream either forgot about, or missed this, so I guess someone should followup.

It has been just merged.

@DrahtBot
Copy link
Contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File commit 60b8164
(master)
commit 8e39baa
(master and this pull)
SHA256SUMS.part e1d20819f9fe7a08... 8cd3087329c70a5e...
*-aarch64-linux-gnu-debug.tar.gz d590bd07c550b27e... 6995ea8d83ff65fe...
*-aarch64-linux-gnu.tar.gz 14fe218ce0710623... 90d6642b73088655...
*-arm-linux-gnueabihf-debug.tar.gz f77e35f6875b444e... d0015f2add470b74...
*-arm-linux-gnueabihf.tar.gz b753e4d2cbcf3170... 25b2eb8ae9ebbc5e...
*-arm64-apple-darwin-unsigned.tar.gz 0da23890907ff0e4... 796cd1941987c771...
*-arm64-apple-darwin-unsigned.zip 03a550731d6f916d... 2b9710b1e52f4b68...
*-arm64-apple-darwin.tar.gz 0f1c6d7fd51e93aa... 7f5efe938690e511...
*-powerpc64-linux-gnu-debug.tar.gz d11c0b00be5deae7... 09efb33aac8558ab...
*-powerpc64-linux-gnu.tar.gz 004050b4e0aa297b... ed867ed304e9bd8b...
*-riscv64-linux-gnu-debug.tar.gz 4b9974943c7fde6a... 7faee0485ea7e303...
*-riscv64-linux-gnu.tar.gz c9c043b43ac975cc... 994a1941d5afaa6d...
*-x86_64-apple-darwin-unsigned.tar.gz 0f28bd1b9bab6404... 0c2153e83db6da36...
*-x86_64-apple-darwin-unsigned.zip 3ca821a78bf5243a... 60f3453d772b9518...
*-x86_64-apple-darwin.tar.gz cb4902c99574904c... 326f9a41f13f53d6...
*-x86_64-linux-gnu-debug.tar.gz e3d2e19faaae8906... 8e6a1aadee878f3d...
*-x86_64-linux-gnu.tar.gz 13782a4c41a807bc... 2a2402b17671c7c5...
*.tar.gz 1ef40f849dfafbd5... e8136abd8d3c8eef...
guix_build.log 3a1f71160fcc6070... 9b7ba245b998da86...
guix_build.log.diff e056534bdf250b1e...

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. UtACK f1e39c4

@@ -0,0 +1,35 @@
commit 0977870006bd1b02a184608ccdd27152478e98ce
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't an upstream commit? Can just drop the commit/author/date etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amended per your request.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 371910a

@DrahtBot DrahtBot requested a review from theuni August 28, 2024 11:17
@fanquake fanquake merged commit 2eb358b into bitcoin:master Aug 28, 2024
5 of 16 checks passed
@hebasto hebasto deleted the 240723-zmq-pc branch August 28, 2024 11:36
fanquake added a commit that referenced this pull request Aug 30, 2024
66dd1b4 build: Drop no longer needed workaround (Hennadii Stepanov)

Pull request description:

  This PR deletes a workaround that is no longer needed since #30508 was merged.

ACKs for top commit:
  fanquake:
    ACK 66dd1b4

Tree-SHA512: abb8e79b525989afe88f94899e4dc29c80d4593ea23f44c6b3d08710e6ddd1619e748798534973fa4ee9f48d9fad7226445b7a2cb4aec0bdb5d1b7ff2f6689ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 27, 2024
371910a depends: Fix CMake-generated `libzmq.pc` file (Hennadii Stepanov)

Pull request description:

  This is a backport of: zeromq/libzmq#4706.

  Similar to bitcoin#30488.

  Addresses bitcoin#29723 (comment):
  > Looking at the mingw .pc generated by this PR:
  >
  > ```
  > Libs: -L${libdir} -lzmq
  > Libs.private:
  > Requires.private:
  > ```
  >
  > It looks like we'll need to take [zeromq/libzmq#4706](zeromq/libzmq#4706) as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.

ACKs for top commit:
  fanquake:
    ACK 371910a

Tree-SHA512: 6f9c2e32f83c0e629e32fd3e4c86712af00ffeaf0906bf85e5c2df889302707b9df102e8031249d1bae036eb4fc019c2a5124655682fbc5652d9337cb21c5f2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 27, 2024
371910a depends: Fix CMake-generated `libzmq.pc` file (Hennadii Stepanov)

Pull request description:

  This is a backport of: zeromq/libzmq#4706.

  Similar to bitcoin#30488.

  Addresses bitcoin#29723 (comment):
  > Looking at the mingw .pc generated by this PR:
  >
  > ```
  > Libs: -L${libdir} -lzmq
  > Libs.private:
  > Requires.private:
  > ```
  >
  > It looks like we'll need to take [zeromq/libzmq#4706](zeromq/libzmq#4706) as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.

ACKs for top commit:
  fanquake:
    ACK 371910a

Tree-SHA512: 6f9c2e32f83c0e629e32fd3e4c86712af00ffeaf0906bf85e5c2df889302707b9df102e8031249d1bae036eb4fc019c2a5124655682fbc5652d9337cb21c5f2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 27, 2024
371910a depends: Fix CMake-generated `libzmq.pc` file (Hennadii Stepanov)

Pull request description:

  This is a backport of: zeromq/libzmq#4706.

  Similar to bitcoin#30488.

  Addresses bitcoin#29723 (comment):
  > Looking at the mingw .pc generated by this PR:
  >
  > ```
  > Libs: -L${libdir} -lzmq
  > Libs.private:
  > Requires.private:
  > ```
  >
  > It looks like we'll need to take [zeromq/libzmq#4706](zeromq/libzmq#4706) as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.

ACKs for top commit:
  fanquake:
    ACK 371910a

Tree-SHA512: 6f9c2e32f83c0e629e32fd3e4c86712af00ffeaf0906bf85e5c2df889302707b9df102e8031249d1bae036eb4fc019c2a5124655682fbc5652d9337cb21c5f2c
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 1, 2024
ae6e0ad Merge bitcoin#29725: depends: build libqrencode with CMake (fanquake)
1522896 fix mingw32 build (UdjinM6)
7bb1d0e Merge bitcoin#22724: windres: use PACKAGE_VERSION rather than building more version numbers (fanquake)
6bee8e2 Merge bitcoin#27496: depends: reuse _config_opts for CMake options (fanquake)
24973ee Merge bitcoin#30488: depends: Fix CMake-generated `libevent*.pc` files (merge-script)
a27b08e Merge bitcoin#29835: depends: build libevent with CMake (merge-script)
a204616 Merge bitcoin#30464: test, refactor: Fix MSVC warning C4101 "unreferenced local variable" (merge-script)
02aee12 Merge bitcoin#30508: depends: Fix CMake-generated `libzmq.pc` file (merge-script)
e2b2446 Merge bitcoin#29723: depends: build zeromq with CMake (merge-script)
685b7a7 Merge bitcoin#23611: build: add `LTO` option to depends (laanwj)
d2b8c6b Merge bitcoin#19952: build, ci: Add file-based logging for individual packages (laanwj)
fc1c29c partial Merge bitcoin#23478: build: Add support for Android NDK r23 LTS (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  depends on #6293

  ## What was done?
    _Describe your changes in detail_

  ## How Has This Been Tested?
    _Please describe in detail how you tested your changes._

    _Include details of your testing environment, and the tests you ran
  to see how your change affects other areas of the code, etc._

  ## Breaking Changes
    _Please describe any breaking changes your code introduces_

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK ae6e0ad
  knst:
    utACK ae6e0ad
  knst:
    utACK ae6e0ad

Tree-SHA512: 0606c7596394155417ad0fea96ce7e1f905109ce2978987e1c4132e8b0f5a8593c5c62ea7217510169228e8238ba42b418a74635ded01f5d674f62495ad3b3a9
luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Feb 15, 2025
@bitcoin bitcoin locked and limited conversation to collaborators Aug 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: ⭐ Prerequisites
Development

Successfully merging this pull request may close these issues.

5 participants