Bug 238127 - devel/libevent: update to 2.1.10
Summary: devel/libevent: update to 2.1.10
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Jan Beich
URL: https://github.com/libevent/libevent/...
Keywords: patch, patch-ready
Depends on: 238433
Blocks: 239599
  Show dependency treegraph
 
Reported: 2019-05-26 00:47 UTC by Jan Beich
Modified: 2019-08-02 12:53 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (mm)
jbeich: merge-quarterly+


Attachments
v1 (has commit message) (12.36 KB, patch)
2019-05-26 00:47 UTC, Jan Beich
jbeich: maintainer-approval? (mm)
Details | Diff
poudriere log with "make test" enabled (189.52 KB, text/plain)
2019-05-26 01:10 UTC, Jan Beich
no flags Details
CMake version (14.12 KB, patch)
2019-05-31 06:21 UTC, Daniel Engberg
no flags Details | Diff
CMake version v2 (14.91 KB, patch)
2019-06-02 09:05 UTC, Daniel Engberg
no flags Details | Diff
CMake version v3 (14.81 KB, patch)
2019-06-02 09:12 UTC, Daniel Engberg
no flags Details | Diff
CMake version v4 (14.74 KB, patch)
2019-06-02 09:15 UTC, Daniel Engberg
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2019-05-26 00:47:50 UTC
Created attachment 204609 [details]
v1 (has commit message)

According to https://abi-laboratory.pro/tracker/timeline/libevent/ exp-run isn't necessary. Only evutil_secure_rng_add_bytes() disappears as explained in bug 230764 comment 6.

Firefox 69 (scheduled on 2019-09-03) may require libevent 2.1.10 for --with-system-libevent.

"poudriere bulk -t" passed:
- 11.2 aarch64/amd64/armv6/i386
- 11.3 i386
- 12.0 aarch64/amd64/armv6/armv7/i386
- 13.0 amd64/armv6/armv7/i386
- OPENSSL=off THREADS=off (on 11.2 i386)
- DEFAULT_VERSIONS += ssl=libressl (on 11.2 amd64)
- DEFAULT_VERSIONS += ssl=libressl-devel (on 11.2 amd64)
- DEFAULT_VERSIONS += ssl=openssl (on 12.0 amd64)
- DEFAULT_VERSIONS += ssl=openssl111 (on 11.2 amd64)
Comment 1 Jan Beich freebsd_committer freebsd_triage 2019-05-26 01:10:05 UTC
Created attachment 204610 [details]
poudriere log with "make test" enabled

diff --git a/devel/libevent/Makefile b/devel/libevent/Makefile
index d02d49a5e0b2..cb57d38707cd 100644
--- a/devel/libevent/Makefile
+++ b/devel/libevent/Makefile
@@ -32,4 +32,8 @@ OPENSSL_CONFIGURE_ENABLE=	openssl
 
 THREADS_CONFIGURE_ENABLE=	thread-support
 
+pre-install:	do-test
+
 .include <bsd.port.mk>
+
+do-test:	.IGNORE
Comment 2 Jan Beich freebsd_committer freebsd_triage 2019-05-26 01:20:51 UTC
Piotr, can you check build on powerpc64?
Comment 3 Jan Beich freebsd_committer freebsd_triage 2019-05-26 06:11:59 UTC
libevent has ~102 consumers. All built fine on 11.2 amd64.

Let's MFH this to 2019Q2. Some upstream changes may affect security but none are on the radar yet.
https://security-tracker.debian.org/tracker/source-package/libevent
Comment 4 Piotr Kubaj freebsd_committer freebsd_triage 2019-05-26 10:08:33 UTC
(In reply to Jan Beich from comment #2)
Builds fine on latest CURRENT.
Comment 5 Daniel Engberg freebsd_committer freebsd_triage 2019-05-31 06:21:46 UTC
Created attachment 204730 [details]
CMake version

Hi,

Here's a version using CMake instead Autotools with a few other fixes

* Replace PORTVERSION with DISTVERSION
* Use packaged release tarball by upstream rather than generated (I guess this boils down to preference)
* Disable testing, benchmark, samples and regress as these are not installed. As a side effect disabing regress also removes the requirement of python
* Add debug option
* include/event2/bufferevent_ssl.h is only installed if OPENSSL is enabled (plist fix)

Best regards,
Daniel
Comment 6 Jan Beich freebsd_committer freebsd_triage 2019-05-31 10:41:23 UTC
Comment on attachment 204730 [details]
CMake version

I'd rather not mix CMake switch and the update in order to facilitate regression tracking each change independently. It may also be worth to wait for upstream to sort out the differences before switching.

https://github.com/libevent/libevent/issues/760

> * Replace PORTVERSION with DISTVERSION

Also in my version.

> * Use packaged release tarball by upstream rather than generated (I guess this boils down to preference)

git-archive(1) snapshots are harder to compromise than vendor provided distfiles. And switching to vendor distfiles only saves USES=autoreconf which isn't used by USES=cmake.

> * Disable testing, benchmark, samples and regress as these are not installed. As a side effect disabing regress also removes the requirement of python

"make test" should be supported to facilitate debugging or tracking regression between different FreeBSD versions or different configurations. Currently, only 1 test fails.

> * Add debug option

OK but it's not new. Current version (2.1.8, autoconf) can add DEBUG_CONFIGURE_ENABLE=debug-mode as well.

> * include/event2/bufferevent_ssl.h is only installed if OPENSSL is enabled (plist fix)

Seems to be CMake-specific.

> +MASTER_SITES=	https://github.com/libevent/libevent/releases/download/release-2.1.10-stable/

2.1.10 should be replaced with DISTVERSION or DISTVERSIONFULL.

> -SHEBANG_FILES=	event_rpcgen.py
[...]
> +USES=		cmake shebangfix

Do you still need USES=shebangfix after SHEBANG_FILES was removed?

>  lib/libevent.so
> -lib/libevent_core-2.1.so.6
> -lib/libevent_core-2.1.so.6.0.2
> -lib/libevent_core.a
> +lib/libevent.so.2.1.10
>  lib/libevent_core.so
> -lib/libevent_extra-2.1.so.6
> -lib/libevent_extra-2.1.so.6.0.2
> -lib/libevent_extra.a
> +lib/libevent_core.so.2.1.10
>  lib/libevent_extra.so

- Unconditionally removing static libs is not nice, see https://github.com/libevent/libevent/pull/820
- SONAME change (ABI versioning) requires mass-bumping PORTREVISION in consumers
Comment 7 Jan Beich freebsd_committer freebsd_triage 2019-05-31 10:49:46 UTC
Comment on attachment 204730 [details]
CMake version

> +CMAKE_ARGS+=	-DBUILD_TESTING:BOOL=OFF -DEVENT__DISABLE_BENCHMARK:BOOL=ON
> +CMAKE_ARGS+=	-DEVENT__DISABLE_SAMPLES:BOOL=ON -DEVENT__DISABLE_TESTS:BOOL=ON
> +CMAKE_ARGS+=	-DEVENT__DISABLE_REGRESS:BOOL=ON

Convert to CMAKE_ON helper, see ports r457677.
Comment 8 Daniel Engberg freebsd_committer freebsd_triage 2019-06-02 09:05:14 UTC
Created attachment 204774 [details]
CMake version v2

* Switched back to GitHub
* Removed all patches
* Removed shebangfix (not needed)
* Add back testing and only gets built when selected
* Use CMake helpers

Best regards,
Daniel
Comment 9 Daniel Engberg freebsd_committer freebsd_triage 2019-06-02 09:08:22 UTC
Many thanks for the pointers Jan!

fwiw, all tests completes on my 12.0 Box (AMD64)

"100% tests passed, 0 tests failed out of 31
Total Test time (real) = 612.92 sec"
Comment 10 Daniel Engberg freebsd_committer freebsd_triage 2019-06-02 09:12:23 UTC
Created attachment 204775 [details]
CMake version v3

Remove MASTER_SITES
Comment 11 Daniel Engberg freebsd_committer freebsd_triage 2019-06-02 09:15:02 UTC
Created attachment 204776 [details]
CMake version v4

Removed old comment, sorry
Comment 12 Jan Beich freebsd_committer freebsd_triage 2019-06-02 17:27:52 UTC
Please, rebase CMake patch against mine and move into a separate bug. I'm only interested in the update which will land after maintainer timeout (2019-06-09).

@koobs, can you help explain the new contributor that hijacking bugs by others in order to blob unrelated changes only adds to the confusion and complicates the review?
Comment 13 Daniel Engberg freebsd_committer freebsd_triage 2019-06-02 18:01:30 UTC
It wasn't my intention so I'll apologize although I would have appreciated if you've told me from the start.
Comment 14 Jan Beich freebsd_committer freebsd_triage 2019-06-02 19:01:41 UTC
Actually, I did say so in comment 6: the first sentence. The subsequent review was based from my own effort to convert to USES=cmake which was buggy in a different way than your version. Another reason to not mix was stated less clearly in comment 3 by the intent MFH to 2019Q2.
Comment 15 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-05 05:02:34 UTC
(In reply to Jan Beich from comment #12)

I think we're all good here now, particularly given Daniels response.

Assuming good faith, it's not very clear up front that our workflow and multiple branch development model means its highly desirable (now, more than ever) to separate logical changesets to facilitate easier merges to the quarterly branch.

Also, in the past, and still today to a non-trivial degree, committers/contributors try to get as much into a single change proposal as possible to 'reduce issue management overhead' and get past resource constraints we have getting changes into the tree.

Unfortunately, doing so also complicates reviews and understanding issues themselves, and increases the time it takes to QA changes, which actually hinders, not enhances productivity.

What we can do to communicate this better in the meantime, is just to ask for changes to be separated where desirable, and provide insights into the reasons. 

Over time, everyone will learn why this is useful and helpful.

I don't believe many contributors intend to hijack issues (but yes, some do try to game the system). It's up to us to guide people in the right direction.
Comment 16 commit-hook freebsd_committer freebsd_triage 2019-06-09 05:53:37 UTC
A commit references this bug:

Author: jbeich
Date: Sun Jun  9 05:52:54 UTC 2019
New revision: 503790
URL: https://svnweb.freebsd.org/changeset/ports/503790

Log:
  devel/libevent2: update to 2.1.10

  Changes:	https://github.com/libevent/libevent/releases/tag/release-2.1.10-stable
  ABI:		https://abi-laboratory.pro/tracker/timeline/libevent/
  PR:		238127
  Reported by:	GitHub (watch releases)
  Tested by:	pkubaj (powerpc64)
  Approved by:	maintainer timeout (2 weeks)
  MFH:		2019Q2 (maybe security)

Changes:
  head/devel/libevent/Makefile
  head/devel/libevent/distinfo
  head/devel/libevent/files/patch-evutil__rand.c
  head/devel/libevent/files/patch-gcc7
  head/devel/libevent/files/patch-libressl
  head/devel/libevent/files/patch-test_bench
  head/devel/libevent/pkg-plist
Comment 17 Martin Matuska freebsd_committer freebsd_triage 2019-06-09 06:13:51 UTC
I have returned the maintainership of devel/libevent back to the pool. You can take it if you want.
Comment 18 commit-hook freebsd_committer freebsd_triage 2019-06-13 00:48:12 UTC
A commit references this bug:

Author: jbeich
Date: Thu Jun 13 00:47:23 UTC 2019
New revision: 504052
URL: https://svnweb.freebsd.org/changeset/ports/504052

Log:
  MFH: r503790 r503811

  devel/libevent2: update to 2.1.10

  Changes:	https://github.com/libevent/libevent/releases/tag/release-2.1.10-stable
  ABI:		https://abi-laboratory.pro/tracker/timeline/libevent/
  PR:		238127
  Reported by:	GitHub (watch releases)
  Tested by:	pkubaj (powerpc64)
  Approved by:	maintainer timeout (2 weeks)
  Approved by:	ports-secteam (miwi)

Changes:
_U  branches/2019Q2/
  branches/2019Q2/devel/libevent/Makefile
  branches/2019Q2/devel/libevent/distinfo
  branches/2019Q2/devel/libevent/files/patch-evutil__rand.c
  branches/2019Q2/devel/libevent/files/patch-gcc7
  branches/2019Q2/devel/libevent/files/patch-libressl
  branches/2019Q2/devel/libevent/files/patch-test_bench
  branches/2019Q2/devel/libevent/pkg-plist
  branches/2019Q2/security/tor/Makefile
  branches/2019Q2/security/tor-devel/Makefile