build: Check libevent minimum version in configure script #18676
Conversation
|
utACK |
|
ACK 4a8e079 |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
If building out of the box on xenial is such a big deal maybe check for |
|
@laanwj I failed to find the reason why |
|
It has been the minimal version ever since libevent was introduced into bitcoin core, it was the version I was using at the time (yes, that long ago). |
|
If you want to change the minimum version to 2.0.21 that's really fine with me. I do vaguely remember newer versions fixed some issues (such as file descriptor leaks?), but, it will more or less work and I don't know which version fixed what anyhow. |
4a8e079 to
b650dce
Compare
b650dce to
b68e717
Compare
|
To give some background: I brought up the topic about checking minimum libevent version in IRC because a fuzz test (http_request) using internal libevent functions caused a linking error on my system. I then realized that I was running libevent 2.0.21, when the minimum required version according to Unfortunately with 2.0.22, the fuzz test wouldn't compile either, a workaround is needed for all versions < 2.1.1 see PR #18682 Nevertheless checking minimum required version is a good thing, so Concept ACK. |
|
w.r.t. #18682 I dropped my own workaround in |
Just out of curiosity, how would a workaround in |
smth. like #18358 with a test-based approach. |
|
Looks like Alpine, Arch, FreeBSD, and Ubuntu are at 2.1.11, while CentOS, Debian, Fedora, Gentoo, and openSUSE ship 2.1.8. Unfortunately, Devuan only ships 2.0.21 still... I don't see anything in 2.0.21..2.0.22 that should break the build, so I suggest we set the minimum enforced to 2.0.21 for now, require 2.1.8 for the fuzzer to build, and consider bumping the project-wide minimum to 2.1.8 sometime in the near future. |
@luke-jr This is what the pull is doing here |
|
Concept ACK |
|
re-utACK |
|
utACK b68e717 |
For example, on bionic with --- a/configure.ac
+++ b/configure.ac
@@ -1265,9 +1265,9 @@ if test x$use_pkgconfig = xyes; then
BITCOIN_QT_CHECK([PKG_CHECK_MODULES([QR], [libqrencode], [have_qrencode=yes], [have_qrencode=no])])
fi
if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench != xnonononono; then
- PKG_CHECK_MODULES([EVENT], [libevent >= 2.0.21], [use_libevent=yes], [AC_MSG_ERROR(libevent version 2.0.21 or greater not found.)])
+ PKG_CHECK_MODULES([EVENT], [libevent >= 2.1.9], [use_libevent=yes], [AC_MSG_ERROR(libevent version 2.1.9 or greater not found.)])
if test x$TARGET_OS != xwindows; then
- PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads >= 2.0.21],, [AC_MSG_ERROR(libevent_pthreads version 2.0.21 or greater not found.)])
+ PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads >= 2.1.9],, [AC_MSG_ERROR(libevent_pthreads version 2.1.9 or greater not found.)])
fi
fi
|
|
ACK b68e717 |
…re script b68e717 build: Set libevent minimum version to 2.0.21 (Hennadii Stepanov) Pull request description: The non-`pkg-config` path is ignored as there is a hope to get rid of all of them in bitcoin#18307. As xenial has [libevent 2.0.21](https://packages.ubuntu.com/xenial-updates/libevent-2.0-5) only, the default bionic Docker image is used in the _"[no depends, only system libs, sanitizers: thread (TSan), no wallet]"_ CI test. ACKs for top commit: theStack: utACK bitcoin@b68e717 laanwj: ACK b68e717 Tree-SHA512: 9825c42aeb166165e99fe5eaf74dbb47c2b51aecdbe53c5ae949fe126e1b8e8b6fe8d228fdde4e8daa4243e5907954202f42eb23c71629e4b2b92a7d4eb892e4
Github-Pull: bitcoin#18676 Rebased-From: b68e717
7f7548d rpc: Do not advertise dumptxoutset as a way to flush the chainstate (MarcoFalke) a9ca65b Fix naming of macOS SDK and clarify version (Andrew Chow) 54d2063 Do not expose and consider -logthreadnames when it does not work (Hennadii Stepanov) 6986b26 build: fix ASLR for bitcoin-cli on Windows (fanquake) 1d1e358 build: Set libevent minimum version to 2.0.21 (Hennadii Stepanov) 842b13a Avoid non-trivial global constants in SHA-NI code (Pieter Wuille) ade4185 gitian: Add missing automake package to gitian-win-signer.yml (Andrew Chow) Pull request description: Currently backports the following to the 0.20 branch: * #18598 - gitian: Add missing automake package to gitian-win-signer.yml * #18702 - build: fix ASLR for bitcoin-cli on Windows * #18676 - build: Check libevent minimum version in configure script * #18665 - Do not expose and consider -logthreadnames when it does not work * #18553 - Avoid non-trivial global constants in SHA-NI code * #18589 - Fix naming of macOS SDK and clarify version ACKs for top commit: laanwj: ACK 7f7548d Tree-SHA512: 2cba748414a440e3fb901940085a7ae059e8b926c9187fbbbdeb7846a32e7374f318cc21e499c911ff803c42aef2c844b04af10b87f9c5a2b3edf6deb03ebb04
Github-Pull: bitcoin#18676 Rebased-From: b68e717
be95147 Updated appveyor job to checkout a specific vcpkg commit ID. (Aaron Clauson) 1fd9cd2 appveyor: Remove clcache (MarcoFalke) 8c0a959 Remove cached directories and associated script blocks from appveyor CI configuration. (Aaron Clauson) d70f700 lint: fix shellcheck URL in CI install (fanquake) f8f7d91 test: remove Cirrus CI FreeBSD job (fanquake) b7e16a8 Add missing QPainterPath include (Andrew Chow) 30a2814 gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged (João Barbosa) 0d87a5b QA: feature_segwit: Check that template "rules" includes "!segwit" as appropriate (Luke Dashjr) bde6a5a Bugfix: Include "csv","!segwit" in "rules" (Luke Dashjr) e422f65 build: Set libevent minimum version to 2.0.21 (Hennadii Stepanov) 0d0dd6a Update with new Windows code signing certificate (Andrew Chow) Pull request description: Backports the following to the 0.19 branch: * #17946 - Fix GBT: Restore "!segwit" and "csv" to "rules" key * #18160 - gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged * #18425 - releases: Update with new Windows code signing certificate * #18676 - build: Check libevent minimum version in configure script * #19097 - qt: Add missing QPainterPath include (as per #19510) * #18640 - appveyor: Remove clcache * #19444 - test: Remove cached directories and associated script blocks from appveyor config * #19612 - lint: fix shellcheck URL in CI install * #18001 - Updated appveyor job to checkout a specific vcpkg commit ID Closes: #19510. ACKs for top commit: jnewbery: ACK be95147 MarcoFalke: cherry-pick ACK be95147 🌎 Tree-SHA512: 2ec7e3ae1da99799ff6f8cfe26095d6885cffe6952b18a7e236dc5e657b3918225c2601b8c8e17cdff5319c40cb0a214d9fad49b0ff2f54af1db7c81d83a1df5
Github-Pull: #18676 Rebased-From: b68e717
The non-
pkg-configpath is ignored as there is a hope to get rid of all of them in #18307.As xenial has libevent 2.0.21 only, the default bionic Docker image is used in the "[no depends, only system libs, sanitizers: thread (TSan), no wallet]" CI test.