build: bump libevent to 2.1.11 in depends #17008

stefanwouldgo commented at 9:38 AM on October 1, 2019: contributor

this doesn't need patches on Android anymore like 2.1.8 did.

fanquake added the label Build system on Oct 1, 2019

laanwj added the label Needs gitian build on Oct 1, 2019

laanwj renamed this:
~~bump libevent to 2.1.11 in depends~~
build: bump libevent to 2.1.11 in depends
on Oct 1, 2019

dongcarl commented at 7:24 PM on October 1, 2019: member

I don't know about the Android part, but I did confirm that the ./configure --help outputs are identical modulo the version number.

in depends/packages/libevent.mk:7 in 51794d91fa outdated

   0 | @@ -1,9 +1,8 @@
   1 |  package=libevent
   2 | -$(package)_version=2.1.8-stable
   3 | +$(package)_version=2.1.11-stable
   4 |  $(package)_download_path=https://github.com/libevent/libevent/archive/
   5 |  $(package)_file_name=release-$($(package)_version).tar.gz
   6 | -$(package)_sha256_hash=316ddb401745ac5d222d7c529ef1eada12f58f6376a66c1118eee803cb70f83d
   7 | -

fanquake commented at 2:22 AM on October 2, 2019:

Please don't remove this blank line

stefanwouldgo commented at 1:05 PM on October 2, 2019:

Sorry, fixed.

fanquake commented at 3:40 AM on October 2, 2019: member

Can you mention which Android patches are no-longer required? As if we don't actually want to bump libevent, maybe we could move them into depends.

2.1.11 changelog is available here.

This PR would also need to update the libevent entry in doc/dependencies.md.

Diff of the configure output comparing master (1f40a91286662a9bf40737c34b4de2f5b5fa8b96) and this PR (51794d91fa5274b60c0585817807ab89e0d40cd4) on macOS:

--- master.txt
+++ pr.txt
@@ -129,14 +129,15 @@
 checking for WIN32... no
+checking for MIDIPIX... no
 checking for CYGWIN... no
@@ -163,14 +164,17 @@
 checking netinet/tcp.h presence... yes
 checking for netinet/tcp.h... yes
+checking sys/un.h usability... yes
+checking sys/un.h presence... yes
+checking for sys/un.h... yes
 checking poll.h usability... yes
 checking poll.h presence... yes
@@ -243,14 +247,15 @@
 checking for arc4random_buf... yes
+checking for arc4random_addrandom... yes
 checking for eventfd... no
@@ -285,15 +290,14 @@
 checking for working kqueue... yes
 checking for epoll_ctl... no
-checking waitpid support WNOWAIT... no
 checking for port_create... no
 checking for pid_t... yes
@@ -303,28 +307,31 @@
 checking size of void *... 8
 checking size of off_t... 8
+checking size of time_t... 8
 checking for struct in6_addr... yes
 checking for struct sockaddr_in6... yes
+checking for struct sockaddr_un... yes
 checking for sa_family_t... yes
 checking for struct addrinfo... yes
 checking for struct sockaddr_storage... yes
 checking for struct in6_addr.s6_addr32... no
 checking for struct in6_addr.s6_addr16... no
 checking for struct sockaddr_in.sin_len... yes
 checking for struct sockaddr_in6.sin6_len... yes
 checking for struct sockaddr_storage.ss_family... yes
 checking for struct sockaddr_storage.__ss_family... no
-checking for struct so_linger... no
+checking for struct linger... yes
 checking for socklen_t... yes
 checking whether our compiler supports __func__... yes
+checking whether our compiler supports __FUNCTION__... yes
 checking for the pthreads library -lpthreads... no
 checking whether pthreads work without any flags... yes
 checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
 checking if more special flags are required for pthreads... -D_THREAD_SAFE
 checking size of pthread_t... 8
 checking that generated files are newer than configure... done
 configure: creating ./config.status

As per the release notes bufferevent_ssl.h is no longer installed. I have a depends diff of master vs this PR here.

Which operating systems have you tested building / running Bitcoin Core on with the new libevent?

laanwj commented at 8:40 AM on October 2, 2019: member

+checking for struct sockaddr_un... yes

That looks vaguely promising. Is there better UNIX socket support ? Seems so: https://github.com/libevent/libevent/commit/737d1beb14423dc212b0c53edf536b3172c87571 Can't find anything similar for the HTTP client though.

stefanwouldgo commented at 12:55 PM on October 2, 2019: contributor

Can you mention which Android patches are no-longer required? As if we don't actually want to bump libevent, maybe we could move them into depends.

This patch from @greenaddress for missing arc4random_addrandom. It is now handled in upstream.

This PR would also need to update the libevent entry in doc/dependencies.md.

Good point. Done.

Which operating systems have you tested building / running Bitcoin Core on with the new libevent?

termux on Android-5 (ndk 20).

DrahtBot removed the label Needs gitian build on Oct 2, 2019

DrahtBot commented at 8:40 PM on October 3, 2019: member

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

Conflicts

No conflicts as of last run.

laanwj commented at 10:04 AM on October 5, 2019: member

ACK after squashing the 968ae2f325ca7bfb0080ad53d3240680ccf1427e commit

laanwj added the label Android on Oct 5, 2019

stefanwouldgo force-pushed on Oct 7, 2019

stefanwouldgo commented at 8:34 AM on October 7, 2019: contributor

ACK after squashing the 968ae2f commit

Done.

MarcoFalke added the label Needs gitian build on Oct 11, 2019

DrahtBot removed the label Needs gitian build on Oct 12, 2019

kristapsk commented at 2:39 PM on October 13, 2019: contributor

Don't know about other Linux distros, but on Gentoo 2.1.8 is latest unmasked (stable) version, 2.1.11 is still masked.

laanwj commented at 6:48 PM on October 17, 2019: member

Don't know about other Linux distros, but on Gentoo 2.1.8 is latest unmasked (stable) version, 2.1.11 is still masked.

FWIW, what distributions use is not important for what is used with depends. The binaries are statically linked to libevent and this doesn't change the minimum supported version for building from source (that's still 2.0.22).

MarcoFalke commented at 7:10 PM on October 17, 2019: member

I think the concern was the we don't want to be the bleeding edge testers of new releases, but rather want to wait for others to test new software and only then pull it in.

MarcoFalke commented at 7:12 PM on October 17, 2019: member

At least it is in debian experimental: https://packages.debian.org/experimental/libevent-dev :grimacing:

MarcoFalke deleted a comment on Oct 17, 2019

laanwj commented at 9:47 AM on October 25, 2019: member

Well, 0.20.0 is still more than 6 months away so by that time it surely won't be bleeding edge anymore.

I think better android support is a good reason to merge this. Would be nice to make some progress in that regard.

DrahtBot added the label Needs rebase on Nov 4, 2019

MarcoFalke deleted a comment on Nov 4, 2019

MarcoFalke added the label Needs gitian build on Nov 4, 2019

MarcoFalke commented at 2:01 PM on November 4, 2019: member

Please remove the patch after rebase, now that it is no longer needed.

stefanwouldgo commented at 2:21 PM on November 4, 2019: contributor

Please remove the patch after rebase, now that it is no longer needed.

The patch isn't in this repo AFAIK.

stefanwouldgo closed this on Nov 4, 2019

stefanwouldgo reopened this on Nov 4, 2019

MarcoFalke commented at 2:25 PM on November 4, 2019: member

@stefanwouldgo Please rebase on current master first

MarcoFalke commented at 2:47 PM on November 4, 2019: member

Could squash the two commits into one?

stefanwouldgo force-pushed on Nov 4, 2019

MarcoFalke commented at 3:05 PM on November 4, 2019: member

cc @icota, @greenaddress , @BlockMechanic from #16110

in depends/packages/libevent.mk:10 in 8d87e9082e outdated

   7 | -$(package)_patches=fix_android_arc4random_addrandom.patch
   8 | +$(package)_sha256_hash=229393ab2bf0dc94694f21836846b424f3532585bac3468738b7bf752c03901e
   9 |  
  10 | -ifneq (,$(findstring android,$(host)))
  11 | -  define $(package)_preprocess_cmds
  12 | -    ./autogen.sh && patch -p1 < $($(package)_patch_dir)/fix_android_arc4random_addrandom.patch

icota commented at 3:26 PM on November 4, 2019:

I haven't tested the build with this change but since the fix_android_arc4random_addrandom.patch is no longer used it's probably best to remove the file itself and its libevent.mk presence as well.

stefanwouldgo commented at 3:48 PM on November 4, 2019:

it's probably best to remove the file itself

Sorry, fixed.

and its libevent.mk presence as well.

I don't see any further presence. Did I overlook anything?

icota commented at 3:51 PM on November 4, 2019:

In (package)_patches?

icota commented at 3:53 PM on November 4, 2019:

Sorry, you've removed it. My bad.

icota changes_requested

MarcoFalke removed the label Needs gitian build on Nov 4, 2019

bump libevent to 2.1.11 in depends

this doesn't need patches on Android anymore like 2.1.8 did.

02ac445b2f

stefanwouldgo force-pushed on Nov 4, 2019

icota approved

icota commented at 3:53 PM on November 4, 2019: contributor

utACK

DrahtBot removed the label Needs rebase on Nov 4, 2019

dongcarl commented at 4:20 PM on November 4, 2019: member

Context for other reviewers:

Here are the upstream equivalent patches: https://github.com/libevent/libevent/commit/6541168d7037457b8e5c51cc354f11bd94e618b6, which introduced a regression tracked in https://github.com/libevent/libevent/issues/488, later fixed in https://github.com/libevent/libevent/commit/266f43af7798befa3d27bfabaa9ae699259c3924

Our current patch: https://github.com/bitcoin/bitcoin/blob/bbc9e4133cc5e2c970a557f7c2368e04911aec43/depends/patches/libevent/fix_android_arc4random_addrandom.patch

MarcoFalke added the label Needs gitian build on Nov 5, 2019

DrahtBot commented at 12:11 PM on November 6, 2019: member

Gitian builds

File	commit bdda137878904e9401a84e308ac74c93c2ef87c1<br>(master)	commit 6ee0f55ad4f4e6eda27d9f2573459f8003b019e9<br>(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`338c2453e92576d6...`	`963f3e507ac8729d...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`ebfe02ebb327a5fd...`	`4be3551897128785...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`10685a8364a42850...`	`ac03b9d7cd8ecb30...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`5debad8ecf03f614...`	`9fae2275536d122a...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`c94274278265de7d...`	`b990b189199a6e0d...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`9199c2a5e431eb9a...`	`a00e4dd540407d3f...`
bitcoin-0.19.99-osx-unsigned.dmg	`7da5a7f569a4318d...`	`e068a60e7382af35...`
bitcoin-0.19.99-osx64.tar.gz	`a459a9173951314d...`	`645f28435a9918bd...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`660347859b4073ee...`	`f306e77ef800b045...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`feb185a593b038d9...`	`f86f2afcccaaad0d...`
bitcoin-0.19.99-win64-debug.zip	`a58bd2fba33f4e15...`	`4164fe781db238a4...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`501af3958d794c3d...`	`22a66849e817d991...`
bitcoin-0.19.99-win64.zip	`fb49f8e495bd1bdc...`	`727f6d0df8f69fdf...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`8b1f655dfa7aab99...`	`4c6b26fb59920f2c...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`21b36fd6521700d8...`	`0b9b8a73e8b67803...`
bitcoin-0.19.99.tar.gz	`96c42231845ab16d...`	`500b4050b258dab7...`
bitcoin-core-linux-0.20-res.yml	`5fa050a02e3cb95c...`	`a7d650248d5044fe...`
bitcoin-core-osx-0.20-res.yml	`d0d8fb2e491f47a0...`	`0c3664b345167297...`
bitcoin-core-win-0.20-res.yml	`6f9be4b4073760a5...`	`3af3c33b05d19d79...`
linux-build.log	`b31ba24d0a233210...`	`1e0a3261d66d7119...`
osx-build.log	`eaeccfa77df8d98a...`	`ee93a0513a6e88da...`
win-build.log	`b9e547f8f55648b3...`	`04bc3c43537c5bb8...`
bitcoin-core-linux-0.20-res.yml.diff		`b9f6b3a9aacdeaad...`
bitcoin-core-osx-0.20-res.yml.diff		`85ba9dd065ecdeb9...`
bitcoin-core-win-0.20-res.yml.diff		`ab11b240b77b69bd...`
linux-build.log.diff		`22b209015dfec5ab...`
osx-build.log.diff		`f41fee0ac527a72d...`
win-build.log.diff		`f5518a36a880d9a8...`

DrahtBot removed the label Needs gitian build on Nov 6, 2019

laanwj commented at 12:22 PM on November 6, 2019: member

ACK 02ac445b2fec60e028d2cc93bbf74a35e3d0f48e

laanwj referenced this in commit 6496bb87af on Nov 20, 2019

laanwj merged this on Nov 20, 2019

laanwj closed this on Nov 20, 2019

MarcoFalke commented at 1:57 PM on November 20, 2019: member

Ah, looks like it made it into bullseye: https://packages.debian.org/bullseye/libevent-dev

sidhujag referenced this in commit 904e105627 on Nov 20, 2019

deadalnix referenced this in commit 1678967611 on Apr 7, 2020

sidhujag referenced this in commit b406d741ba on Nov 10, 2020

barton2526 referenced this in commit 61d30f4bad on Jun 10, 2021

MarcoFalke locked this on Dec 16, 2021