[depends] libevent 2.1.7rc #9471

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:depends-0-14-0-libevent changing 3 files +5 −46

fanquake commented at 2:08 PM on January 4, 2017: member

See #8867 for discussion around moving to libevent 2.1.x .

We can drop both of our patches with the switch to the 2.1.x branch.

The libevent-2-fixes patch (https://github.com/bitcoin/bitcoin/pull/8730), was already included in the 2.1.6beta release, so can be removed.

The reuseaddr patch was introduced here. Looking through the 2.17 code, it looks like it might have been a backport from the 2.1.x branch, and so can also be dropped.

libevent 2.1.7-rc changelog Closes #8867
[depends] libevent 2.1.7rc 8217bd1d4c
fanquake added the label Build system on Jan 4, 2017
fanquake added this to the milestone 0.14.0 on Jan 4, 2017
fanquake requested review from theuni on Jan 4, 2017
in depends/packages/libevent.mk:None in 8217bd1d4c
```
   5 | -$(package)_sha256_hash=71c2c49f0adadacfdbe6332a372c38cf9c8b7895bb73dabeaa53cdcc1d4e1fa3
   6 | -$(package)_patches=reuseaddr.patch libevent-2-fixes.patch
   7 | +$(package)_version=2.1.7
   8 | +$(package)_download_path=https://github.com/libevent/libevent/archive/
   9 | +$(package)_file_name=release-$($(package)_version)-rc.tar.gz
  10 | +$(package)_sha256_hash=548362d202e22fe24d4c3fad38287b4f6d683e6c21503341373b89785fa6f991
```
MarcoFalke commented at 2:25 PM on January 4, 2017:

Minor note: This hash is not guaranteed to be constant, I think.

laanwj commented at 9:35 AM on January 5, 2017:

Hm but not checking the hash at all would not be acceptable.

MarcoFalke commented at 10:59 AM on January 5, 2017:

Of course, but we need to keep a backup of the GitHub generated zip file. (Which reminds me we already do so on bitcoincore.org/)

kallewoof commented at 2:45 PM on January 5, 2017:

I may be off track, but if the hash changes for the given release, doesn't that mean we WANT the build system to explode in everyone's faces so people react? I can only imagine a hash change for the same package version would be due to some serious issue being hot-fixed upstream. Keeping and using a backup would mean that hot-fix goes unnoticed.

MarcoFalke commented at 4:15 PM on January 5, 2017:

I can only imagine a hash change for the same package version would be due to some serious issue being hot-fixed upstream

This would require a new release of libevent. What I was referring to, is that the hash changes after GitHub purges its caches for zipped downloads of git tags/commits. (Note the above zip is not provided/signed by libevent but generated by GitHub for convenience.) I could image that GitHub does not deterministically generate zips.

theuni commented at 9:03 PM on January 5, 2017:

@MarcoFalke I remember dealing with this at one point already, and I seem to remember github fixing the downloads to be deterministic, but I can't find any reference to that now.

It looks like our native_cctools comes from a similar link, so I assume that we would've noticed new hashes by now. @kallewoof You're very much on-track. The issue (if I recall) is that github always serves the same .tar, but the gzipped result can change if it's been regenerated.

sipa commented at 9:05 PM on January 5, 2017:

Wouldn't it be possible to have a checksum check for the decompressed .tar then?

theuni commented at 9:06 PM on January 5, 2017:

@sipa Sure, if that's necessary. Given that we have similar downloads atm that aren't causing issues though, I suspect this is no longer an issue.

laanwj commented at 12:50 PM on January 10, 2017:

Let's just take the chance then and merge this.
laanwj commented at 12:51 PM on January 10, 2017: member

Is there any reason this still has a WIP tag?
fanquake renamed this:
~~[WIP][depends] libevent 2.1.7rc~~
[depends] libevent 2.1.7rc
on Jan 10, 2017
fanquake commented at 12:00 AM on January 11, 2017: member

@laanwj Really only due to the URL uncertainty. However we can update to a more stable URL in future if needed.
MarcoFalke commented at 12:05 AM on January 11, 2017: member

Agree with @fanquake
theuni approved
laanwj merged this on Jan 11, 2017
laanwj closed this on Jan 11, 2017
laanwj referenced this in commit ca615e6c05 on Jan 11, 2017
laanwj commented at 12:57 PM on January 11, 2017: member

utACK 8217bd1. I'm not worried about the URL.
fanquake deleted the branch on Jan 11, 2017
jtimon commented at 11:07 PM on January 11, 2017: contributor

post-merge Concept ACK

in depends/packages/libevent.mk:None in 8217bd1d4c

   0 | @@ -1,13 +1,11 @@
   1 |  package=libevent
   2 | -$(package)_version=2.0.22
   3 | -$(package)_download_path=https://github.com/libevent/libevent/releases/download/release-2.0.22-stable
   4 | -$(package)_file_name=$(package)-$($(package)_version)-stable.tar.gz
   5 | -$(package)_sha256_hash=71c2c49f0adadacfdbe6332a372c38cf9c8b7895bb73dabeaa53cdcc1d4e1fa3
   6 | -$(package)_patches=reuseaddr.patch libevent-2-fixes.patch
   7 | +$(package)_version=2.1.7
   8 | +$(package)_download_path=https://github.com/libevent/libevent/archive/
   9 | +$(package)_file_name=release-$($(package)_version)-rc.tar.gz

MarcoFalke commented at 4:11 PM on January 29, 2017:

Looks like they just released the stable version, so we could use that, maybe.

laanwj commented at 9:05 AM on February 2, 2017:

That's annoying, we delay this update for months in the expectation that there will be a new stable version any time, and after we merge this there suddenly it is, the new stable version.

zkbot referenced this in commit b0afc4ba1d on Mar 22, 2017
zkbot referenced this in commit f9f48667be on Mar 25, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Milestone
0.14.0

Linked (view graph)

#5677 libevent-based http server #8730 depends: Add libevent compatibility patch for windows #8867 Move to Libevent 2.1.x