-
Notifications
You must be signed in to change notification settings - Fork 36.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: Fix zeromq package when cross-compiling #24134
Conversation
Assign "23.0" milestone? |
Approach NACK. Why not just pass
Isn't it just any system with libbsd available? |
Correct. The PR description updated. |
|
I agree, but that's also something that should be solved at a higher level in the build system, rather than ad-hoc on a per-package basis. Otherwise we should just go ahead and add the same change to all other packages that use pkg-config.
If this PR makes a change that will ensure zeromqs build system doesn't find libbsd, how is it not about avoiding it? I'm not sure what you mean by restoring the previous behaviour (other than make things work), but I don't necessarily think that is the right goal. We're building a newer version of the package, that has a different build system and new (optional) dependencies.
I'm not suggesting we do that, especially if we end up passing |
This change fixes "Hunk #1 succeeded at 307 (offset -1 lines)."
Since v4.3.3 (068385c951c0608edec6264d55ba9c4c923acccc) libbsd is used by default. As we have no libbsd package in our depends, disable it explicitly. Zeromq will fallback to its internal strlcpy implementation.
We are not running unit tests, therefore it is not required.
Updated c0f664c -> f13e642 (pr24134.01 -> pr24134.02):
PR description has been updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f13e642
f13e642 build: Disable valgrind when building zeromq package in depends (Hennadii Stepanov) b970f03 build: Disable libbsd when building zeromq package in depends (Hennadii Stepanov) 7789999 build: Update netbsd_kevent_void.patch (Hennadii Stepanov) Pull request description: Since v4.3.3 (zeromq/libzmq@068385c) `libzmq` uses `libbsd` by default. This PR disables `libbsd` explicitly, as it's not a part of our depends. Zeromq will fallback to its internal `strlcpy` implementation. Otherwise, on systems with installed `libbsd-dev` package the `zeromq` package build system erroneously detects `libbsd` package from the host system: ```diff --- a/libzmq.pc +++ b/libzmq.pc @@ -8,5 +8,5 @@ Version: 4.3.4 Libs: -L${libdir} -lzmq Libs.private: -lpthread -Requires.private: +Requires.private: libbsd Cflags: -I${includedir} ``` This causes the `configure` fails to detect the `zeromq` package: ``` configure: WARNING: libzmq version 4.x or greater not found, disabling ``` --- Other minor improvements: - fixed `netbsd_kevent_void.patch` offset - disabled valgrind as it's used in unit tests which we do not run: ```diff --- a/zmq-configure-output +++ b/zmq-configure-output @@ -119,11 +119,6 @@ checking whether the g++ -m64 linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes checking dynamic linker characteristics... (cached) GNU/Linux ld.so checking how to hardcode library paths into programs... immediate -checking for valgrind... valgrind -checking for Valgrind tool memcheck... memcheck -checking for Valgrind tool helgrind... helgrind -checking for Valgrind tool drd... drd -checking for Valgrind tool exp-sgcheck... exp-sgcheck checking linker version script flag... --version-script checking if version scripts can use complex wildcards... yes checking for working posix_memalign... yes ``` ACKs for top commit: fanquake: ACK f13e642 Tree-SHA512: d4c86d4a841eb6e7c32157e84972243072f905496c2a4c14ec6f6ab4216df6695cbf29baa2233ce27eaede35d1e250ad2f9975b16f570d01509f0c5da4597cad
Since v4.3.3 (068385c951c0608edec6264d55ba9c4c923acccc) libbsd is used by default. As we have no libbsd package in our depends, disable it explicitly. Zeromq will fallback to its internal strlcpy implementation. Github-Pull: bitcoin#24134 Rebased-From: b970f03
Since v4.3.3 (068385c951c0608edec6264d55ba9c4c923acccc) libbsd is used by default. As we have no libbsd package in our depends, disable it explicitly. Zeromq will fallback to its internal strlcpy implementation. Github-Pull: bitcoin#24134 Rebased-From: b970f03
… to all `$(package)_*_cmds` affbf58 build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564e) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin/bitcoin#22552. Another [bug](bitcoin/bitcoin#22719) in the depends build system has already become a problem at least two times in the past (bitcoin/bitcoin#16883 (comment) and bitcoin/bitcoin#24134). Each time the problem was solved with other means. The initial [solution](bitcoin/bitcoin#19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](bitcoin/bitcoin#24134). The bug is well described in bitcoin/bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin/bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58 Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
…`$(package)_*_cmds` affbf58 build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564e) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin#22552. Another [bug](bitcoin#22719) in the depends build system has already become a problem at least two times in the past (bitcoin#16883 (comment) and bitcoin#24134). Each time the problem was solved with other means. The initial [solution](bitcoin#19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](bitcoin#24134). The bug is well described in bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58 Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
Since v4.3.3 (zeromq/libzmq@068385c)
libzmq
useslibbsd
by default.This PR disables
libbsd
explicitly, as it's not a part of our depends. Zeromq will fallback to its internalstrlcpy
implementation.Otherwise, on systems with installed
libbsd-dev
package thezeromq
package build system erroneously detectslibbsd
package from the host system:This causes the
configure
fails to detect thezeromq
package:Other minor improvements:
netbsd_kevent_void.patch
offset