[PATCH] libevent: enable OpenSSL unconditionally and update packaging

[PATCH] libevent: enable OpenSSL unconditionally and update packaging

[André Draszik]

The original commit describes the reason for disabling openssl
so as to get 'more deterministic build[s]' and size-reduction:
commit 6c36fde6ce2e ("libevent: disable openssl by default"),
commit ad130b97a51a in poky.

Since the introduction of per-recipe sysroots, we always have
deterministic builds.

Size reduction can be achieved by splitting the package into
multiple sub-packages, which each only provide one of the
shared libraries.

Hence there appears no reason anymore to disable OpenSSL
support.

Because this recipe only provides shared libraries which are
handled automatically by bitbake, there is no need to add
the subpackages to the RDEPENDS of PN for backwards
compatibility. The packageing process of dependees will
simply pull in the sub-packages as runtime dependency as
needed.

This also how Debian splits this up.

While updating the packaging, we can also drop event_rpcgen.py
which appears to be a tool for generating rpc bindings, i.e.
something that should normally be in -dev. Given Debian
doesn't package this at all, and given it actually requires
python to run but no runtime dependency is stated at the
moment, it would appear that no users of this exist.

These changes also allow us to build all of nghttp2
out-of-the-box, without affecting existing users.

Signed-off-by: André Draszik <git@andred.net>
---
 meta/recipes-support/libevent/libevent_2.1.11.bb | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/meta/recipes-support/libevent/libevent_2.1.11.bb b/meta/recipes-support/libevent/libevent_2.1.11.bb
index f005ab8bda..c746f22118 100644
--- a/meta/recipes-support/libevent/libevent_2.1.11.bb
+++ b/meta/recipes-support/libevent/libevent_2.1.11.bb
@@ -19,9 +19,6 @@ UPSTREAM_CHECK_URI = "http://libevent.org/"
 
 S = "${WORKDIR}/${BPN}-${PV}-stable"
 
-PACKAGECONFIG ??= ""
-PACKAGECONFIG[openssl] = "--enable-openssl,--disable-openssl,openssl"
-
 inherit autotools
 
 # Needed for Debian packaging
@@ -29,11 +26,19 @@ LEAD_SONAME = "libevent-2.1.so"
 
 inherit ptest multilib_header
 
-DEPENDS = "zlib"
+DEPENDS = "openssl zlib"
+
+PACKAGES =+ "${PN}-core ${PN}-extra ${PN}-openssl ${PN}-pthreads"
+FILES_${PN}-core = "${libdir}/libevent_core*${SOLIBS}"
+FILES_${PN}-extra = "${libdir}/libevent_extra*${SOLIBS}"
+FILES_${PN}-openssl = "${libdir}/libevent_openssl*${SOLIBS}"
+FILES_${PN}-pthreads = "${libdir}/libevent_pthreads-*${SOLIBS}"
 
 BBCLASSEXTEND = "native nativesdk"
 
 do_install_append() {
+	rm ${D}${bindir}/event_rpcgen.py
+	rmdir ${D}${bindir}
         oe_multilib_header event2/event-config.h
 }
 
-- 
2.23.0.rc1

Re: [PATCH] libevent: enable OpenSSL unconditionally and update packaging

[Alexander Kanavin]

[-- Attachment #1: Type: text/plain, Size: 3200 bytes --]

I would rather keep the option to disable openssl, but simply switch it on
by default.

Alex

On Thu, 7 Nov 2019 at 13:11, André Draszik <git@andred.net> wrote:

> The original commit describes the reason for disabling openssl
> so as to get 'more deterministic build[s]' and size-reduction:
> commit 6c36fde6ce2e ("libevent: disable openssl by default"),
> commit ad130b97a51a in poky.
>
> Since the introduction of per-recipe sysroots, we always have
> deterministic builds.
>
> Size reduction can be achieved by splitting the package into
> multiple sub-packages, which each only provide one of the
> shared libraries.
>
> Hence there appears no reason anymore to disable OpenSSL
> support.
>
> Because this recipe only provides shared libraries which are
> handled automatically by bitbake, there is no need to add
> the subpackages to the RDEPENDS of PN for backwards
> compatibility. The packageing process of dependees will
> simply pull in the sub-packages as runtime dependency as
> needed.
>
> This also how Debian splits this up.
>
> While updating the packaging, we can also drop event_rpcgen.py
> which appears to be a tool for generating rpc bindings, i.e.
> something that should normally be in -dev. Given Debian
> doesn't package this at all, and given it actually requires
> python to run but no runtime dependency is stated at the
> moment, it would appear that no users of this exist.
>
> These changes also allow us to build all of nghttp2
> out-of-the-box, without affecting existing users.
>
> Signed-off-by: André Draszik <git@andred.net>
> ---
>  meta/recipes-support/libevent/libevent_2.1.11.bb | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/meta/recipes-support/libevent/libevent_2.1.11.bb
> b/meta/recipes-support/libevent/libevent_2.1.11.bb
> index f005ab8bda..c746f22118 100644
> --- a/meta/recipes-support/libevent/libevent_2.1.11.bb
> +++ b/meta/recipes-support/libevent/libevent_2.1.11.bb
> @@ -19,9 +19,6 @@ UPSTREAM_CHECK_URI = "http://libevent.org/"
>
>  S = "${WORKDIR}/${BPN}-${PV}-stable"
>
> -PACKAGECONFIG ??= ""
> -PACKAGECONFIG[openssl] = "--enable-openssl,--disable-openssl,openssl"
> -
>  inherit autotools
>
>  # Needed for Debian packaging
> @@ -29,11 +26,19 @@ LEAD_SONAME = "libevent-2.1.so"
>
>  inherit ptest multilib_header
>
> -DEPENDS = "zlib"
> +DEPENDS = "openssl zlib"
> +
> +PACKAGES =+ "${PN}-core ${PN}-extra ${PN}-openssl ${PN}-pthreads"
> +FILES_${PN}-core = "${libdir}/libevent_core*${SOLIBS}"
> +FILES_${PN}-extra = "${libdir}/libevent_extra*${SOLIBS}"
> +FILES_${PN}-openssl = "${libdir}/libevent_openssl*${SOLIBS}"
> +FILES_${PN}-pthreads = "${libdir}/libevent_pthreads-*${SOLIBS}"
>
>  BBCLASSEXTEND = "native nativesdk"
>
>  do_install_append() {
> +       rm ${D}${bindir}/event_rpcgen.py
> +       rmdir ${D}${bindir}
>          oe_multilib_header event2/event-config.h
>  }
>
> --
> 2.23.0.rc1
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

[-- Attachment #2: Type: text/html, Size: 4609 bytes --]

Re: [PATCH] libevent: enable OpenSSL unconditionally and update packaging

[André Draszik]

On Thu, 2019-11-07 at 13:26 +0100, Alexander Kanavin wrote:
> I would rather keep the option to disable openssl, but simply switch it on by default

Why complicate things, what's the use-case? If libevent_openssl.so is not
used by anything, that library will not be pulled in, as it is a
separate package now.

Cheers,
A.


> .
> 
> Alex
> 
> On Thu, 7 Nov 2019 at 13:11, André Draszik <git@andred.net> wrote:
> > The original commit describes the reason for disabling openssl
> > so as to get 'more deterministic build[s]' and size-reduction:
> > commit 6c36fde6ce2e ("libevent: disable openssl by default"),
> > commit ad130b97a51a in poky.
> > 
> > Since the introduction of per-recipe sysroots, we always have
> > deterministic builds.
> > 
> > Size reduction can be achieved by splitting the package into
> > multiple sub-packages, which each only provide one of the
> > shared libraries.
> > 
> > Hence there appears no reason anymore to disable OpenSSL
> > support.
> > 
> > Because this recipe only provides shared libraries which are
> > handled automatically by bitbake, there is no need to add
> > the subpackages to the RDEPENDS of PN for backwards
> > compatibility. The packageing process of dependees will
> > simply pull in the sub-packages as runtime dependency as
> > needed.
> > 
> > This also how Debian splits this up.
> > 
> > While updating the packaging, we can also drop event_rpcgen.py
> > which appears to be a tool for generating rpc bindings, i.e.
> > something that should normally be in -dev. Given Debian
> > doesn't package this at all, and given it actually requires
> > python to run but no runtime dependency is stated at the
> > moment, it would appear that no users of this exist.
> > 
> > These changes also allow us to build all of nghttp2
> > out-of-the-box, without affecting existing users.
> > 
> > Signed-off-by: André Draszik <git@andred.net>
> > ---
> >  meta/recipes-support/libevent/libevent_2.1.11.bb | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/meta/recipes-support/libevent/libevent_2.1.11.bb b/meta/recipes-support/libevent/libevent_2.1.11.bb
> > index f005ab8bda..c746f22118 100644
> > --- a/meta/recipes-support/libevent/libevent_2.1.11.bb
> > +++ b/meta/recipes-support/libevent/libevent_2.1.11.bb
> > @@ -19,9 +19,6 @@ UPSTREAM_CHECK_URI = "http://libevent.org/"
> > 
> >  S = "${WORKDIR}/${BPN}-${PV}-stable"
> > 
> > -PACKAGECONFIG ??= ""
> > -PACKAGECONFIG[openssl] = "--enable-openssl,--disable-openssl,openssl"
> > -
> >  inherit autotools
> > 
> >  # Needed for Debian packaging
> > @@ -29,11 +26,19 @@ LEAD_SONAME = "libevent-2.1.so"
> > 
> >  inherit ptest multilib_header
> > 
> > -DEPENDS = "zlib"
> > +DEPENDS = "openssl zlib"
> > +
> > +PACKAGES =+ "${PN}-core ${PN}-extra ${PN}-openssl ${PN}-pthreads"
> > +FILES_${PN}-core = "${libdir}/libevent_core*${SOLIBS}"
> > +FILES_${PN}-extra = "${libdir}/libevent_extra*${SOLIBS}"
> > +FILES_${PN}-openssl = "${libdir}/libevent_openssl*${SOLIBS}"
> > +FILES_${PN}-pthreads = "${libdir}/libevent_pthreads-*${SOLIBS}"
> > 
> >  BBCLASSEXTEND = "native nativesdk"
> > 
> >  do_install_append() {
> > +       rm ${D}${bindir}/event_rpcgen.py
> > +       rmdir ${D}${bindir}
> >          oe_multilib_header event2/event-config.h
> >  }
> > 
> > -- 
> > 2.23.0.rc1
> > 

Re: [PATCH] libevent: enable OpenSSL unconditionally and update packaging

[Richard Purdie]

On Thu, 2019-11-07 at 14:01 +0000, André Draszik wrote:
> On Thu, 2019-11-07 at 13:26 +0100, Alexander Kanavin wrote:
> > I would rather keep the option to disable openssl, but simply
> > switch it on by default
> 
> Why complicate things, what's the use-case? If libevent_openssl.so is
> not
> used by anything, that library will not be pulled in, as it is a
> separate package now.

Build time dependencies and hence build speed?

It sounds trivial but all these inter-dependencies do mount up so if we
don't need it, keeping things minimal has advantages.

If there is a security issue in openssl, its one more thing that would
have to be regenerated if a CVE fix were added too...

Cheers,

Richard

[PATCH v2] libevent: update packaging (one package per shared library)

[André Draszik]

libevent produces several libraries that might or might not
be used in the end. We can prevent those potentially unused
libraries from being pulled into a file-system by splitting
the individual shared libraries into individual packages.

Because this recipe only provides shared libraries which are
handled automatically by bitbake (shlibs), there is no need
to add the subpackages to the RDEPENDS of PN for backwards
compatibility. The packaging process of dependees will
simply pull in the sub-packages as runtime dependency as
needed.

This also how Debian splits this up.

While updating the packaging, we can also drop event_rpcgen.py
which appears to be a tool for generating rpc bindings, i.e.
something that should normally be in -dev. Given Debian
doesn't package this at all, and given it actually requires
python to run but no runtime dependency is stated at the
moment, it would appear that no users of this exist.

Signed-off-by: André Draszik <git@andred.net>

---
v2: keep SSL support turned-off by default
---
 meta/recipes-support/libevent/libevent_2.1.11.bb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meta/recipes-support/libevent/libevent_2.1.11.bb b/meta/recipes-support/libevent/libevent_2.1.11.bb
index f005ab8bda..8c7c49e7dd 100644
--- a/meta/recipes-support/libevent/libevent_2.1.11.bb
+++ b/meta/recipes-support/libevent/libevent_2.1.11.bb
@@ -31,9 +31,17 @@ inherit ptest multilib_header
 
 DEPENDS = "zlib"
 
+PACKAGES_DYNAMIC = "^${PN}-.*$"
+python split_libevent_libs () {
+    do_split_packages(d, '${libdir}', r'^libevent_([a-z]*)-.*\.so\..*', '${PN}-%s', '${SUMMARY} (%s)', prepend=True, allow_links=True)
+}
+PACKAGESPLITFUNCS_prepend = "split_libevent_libs "
+
 BBCLASSEXTEND = "native nativesdk"
 
 do_install_append() {
+	rm ${D}${bindir}/event_rpcgen.py
+	rmdir ${D}${bindir}
         oe_multilib_header event2/event-config.h
 }
 
-- 
2.23.0.rc1

Re: [PATCH] libevent: enable OpenSSL unconditionally and update packaging

[André Draszik]

On Thu, 2019-11-07 at 14:08 +0000, Richard Purdie wrote:
> On Thu, 2019-11-07 at 14:01 +0000, André Draszik wrote:
> > On Thu, 2019-11-07 at 13:26 +0100, Alexander Kanavin wrote:
> > > I would rather keep the option to disable openssl, but simply
> > > switch it on by default
> > 
> > Why complicate things, what's the use-case? If libevent_openssl.so is
> > not
> > used by anything, that library will not be pulled in, as it is a
> > separate package now.
> 
> Build time dependencies and hence build speed?
> 
> It sounds trivial but all these inter-dependencies do mount up so if we
> don't need it, keeping things minimal has advantages.
> 
> If there is a security issue in openssl, its one more thing that would
> have to be regenerated if a CVE fix were added too...

What about helping make network connections more secure by enabling ssl
by default? Is yocto really advocating the use of unencrypted connections?

If build time is the argument, why is stack protection enabled by default
in the compiler?
Why do other packages have OpenSSL support enabled by default?

I could go on, but I don't care enough, v2 sent :-)


Cheers,
Andre'

Re: [PATCH] libevent: enable OpenSSL unconditionally and update packaging

[Richard Purdie]

On Thu, 2019-11-07 at 15:41 +0000, André Draszik wrote:
> On Thu, 2019-11-07 at 14:08 +0000, Richard Purdie wrote:
> > On Thu, 2019-11-07 at 14:01 +0000, André Draszik wrote:
> > > On Thu, 2019-11-07 at 13:26 +0100, Alexander Kanavin wrote:
> > > > I would rather keep the option to disable openssl, but simply
> > > > switch it on by default
> > > 
> > > Why complicate things, what's the use-case? If
> > > libevent_openssl.so is
> > > not
> > > used by anything, that library will not be pulled in, as it is a
> > > separate package now.
> > 
> > Build time dependencies and hence build speed?
> > 
> > It sounds trivial but all these inter-dependencies do mount up so
> > if we
> > don't need it, keeping things minimal has advantages.
> > 
> > If there is a security issue in openssl, its one more thing that
> > would
> > have to be regenerated if a CVE fix were added too...
> 
> What about helping make network connections more secure by enabling
> ssl by default? Is yocto really advocating the use of unencrypted
> connections?

No. Information like that about impact would help sway this discussion
and should probably be in the commit message. Its a question of why as
well as what and how.

> If build time is the argument, why is stack protection enabled by
> default in the compiler?
> Why do other packages have OpenSSL support enabled by default?
> 
> I could go on, but I don't care enough, v2 sent :-)

It is important, I suspect the commit message needs more info to help
ensure we make informed decisions...

Cheers,

Richard