diff mbox series

[1/1] package/libp11: fix enginesdir

Message ID 20240509204218.77779-1-fontaine.fabrice@gmail.com
State New
Headers show
Series [1/1] package/libp11: fix enginesdir | expand

Commit Message

Fabrice Fontaine May 9, 2024, 8:42 p.m. UTC
pkg-config doesn't return a libcrypto enginesdir prefixed with the
sysroot so drop unneeded call to xargs, readlink and sed

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/libp11/libp11.mk | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Thomas Petazzoni May 9, 2024, 9:10 p.m. UTC | #1
On Thu,  9 May 2024 22:42:18 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> pkg-config doesn't return a libcrypto enginesdir prefixed with the
> sysroot so drop unneeded call to xargs, readlink and sed
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/libp11/libp11.mk | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/package/libp11/libp11.mk b/package/libp11/libp11.mk
> index cd4ed34297..dc489758aa 100644
> --- a/package/libp11/libp11.mk
> +++ b/package/libp11/libp11.mk
> @@ -11,10 +11,8 @@ LIBP11_INSTALL_STAGING = YES
>  LIBP11_LICENSE = LGPL-2.1+
>  LIBP11_LICENSE_FILES = COPYING
>  
> -# pkg-config returns a libcrypto enginesdir prefixed with the sysroot,
> -# so let's rip it out.

Well, this workaround was here for a reason. Since when it is no longer
needed?

Thomas
Thomas Petazzoni July 22, 2024, 8:21 p.m. UTC | #2
Hello Fabrice,

On Thu, 9 May 2024 23:10:50 +0200
Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:

> > -# pkg-config returns a libcrypto enginesdir prefixed with the sysroot,
> > -# so let's rip it out.  
> 
> Well, this workaround was here for a reason. Since when it is no longer
> needed?

Ping on the above question :-)

Thanks!

Thomas
Fabrice Fontaine July 22, 2024, 8:47 p.m. UTC | #3
Hello Thomas,

Le lun. 22 juil. 2024 à 22:21, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> Hello Fabrice,
>
> On Thu, 9 May 2024 23:10:50 +0200
> Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:
>
> > > -# pkg-config returns a libcrypto enginesdir prefixed with the sysroot,
> > > -# so let's rip it out.
> >
> > Well, this workaround was here for a reason. Since when it is no longer
> > needed?
>
> Ping on the above question :-)

It seems that you added this workaround back in 2019 [1].
However, to my understanding pkg-config only prefixes sysroot to a
subset of variables since at least 2018 [2] and enginesdir is not one
of them.
Finally, since its addition in openssl in 2016 [3], enginesdir has
always been set to ${libdir}/engines-...
So, I don't know what triggers the build failure that you fixed at that time.

>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

Best Regards,

Fabrice

[1] https://patchwork.ozlabs.org/project/buildroot/patch/20181207181314.24051-1-tpiepho@impinj.com/
[2] https://git.buildroot.net/buildroot/commit/package/pkgconf/0001-Only-prefix-with-the-sysroot-a-subset-of-variables.patch?id=7125fc5c1a8a96ff8eee057789358702e1b55835
[3] https://github.com/openssl/openssl/commit/cdbbf9900253e8006868eba948248b1092a057de
Thomas Petazzoni Aug. 7, 2024, 9 a.m. UTC | #4
Hello Fabrice,

On Mon, 22 Jul 2024 22:47:04 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> It seems that you added this workaround back in 2019 [1].
> However, to my understanding pkg-config only prefixes sysroot to a
> subset of variables since at least 2018 [2] and enginesdir is not one
> of them.
> Finally, since its addition in openssl in 2016 [3], enginesdir has
> always been set to ${libdir}/engines-...
> So, I don't know what triggers the build failure that you fixed at that time.

So, I had a closer look, and you're right... but things are going to
get broken again soon.

Back when I introduced this code, enginesdir was expressed in terms of
${libdir}:

enginesdir=${libdir}/engines-3

and because ${libdir} is one of the variables that get sysroot-ed,
enginesdir also gets sysrooted, which is why my "hack" was needed.

Then, in OpenSSL commit 2ac569a67b9d0980efa2d8061a6a61e0645f37a7 (first
appeared in OpenSSL 3.3.0), they reworked the whole .pc stuff, and
changed enginesdir like this:

enginesdir={- $OpenSSL::safe::installdata::ENGINESDIR -}

which ends up as:

enginesdir=/usr/lib/engines-3

so enginesdir is no longer expressed as a relative path to ${libdir},
so it doesn't get sysroot'ed, which is the behaviour you observe right
now.

However, more recently (June 2024) commit
30dc37d798a0428fd477d3763086e7e97b3d596f was merged into OpenSSL, which
reworks the above logic to:

enginesdir=${libdir}/{- $OpenSSL::safe::installdata::ENGINESDIR_REL_LIBDIR -}

so, starting from the next release of OpenSSL, enginesdir will be back
to be defining relatively to ${libdir}, so it will be sysrooted again.

See my quick experiment below. First current situation:

$ grep ^enginesdir output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/pkgconfig/libcrypto.pc
enginesdir=/usr/lib/engines-3
$ ./output/host/bin/pkg-config --variable=enginesdir libcrypto
/usr/lib/engines-3

with the next release of OpenSSL, it will look like this:

$ grep ^enginesdir output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/pkgconfig/libcrypto.pc
enginesdir=${libdir}/engines-3

$ ./output/host/bin/pkg-config --variable=enginesdir libcrypto
./output/host/bin/../arm-buildroot-linux-gnueabihf/sysroot/usr/lib/engines-3

So maybe we should merge your patch.. and revert when a new OpenSSL
release is made?

I was wondering whether the problem should be fixed in libcrypto.pc by
a tweak in libopenssl.mk, but how can we know when this enginesdir will
be used to know the location of the engines at compile time (in which
case it must be sysrooted) vs. when it will be used to know the
location of the engines at run time (in which case it should not be
sysrooted) ?

Thomas
diff mbox series

Patch

diff --git a/package/libp11/libp11.mk b/package/libp11/libp11.mk
index cd4ed34297..dc489758aa 100644
--- a/package/libp11/libp11.mk
+++ b/package/libp11/libp11.mk
@@ -11,10 +11,8 @@  LIBP11_INSTALL_STAGING = YES
 LIBP11_LICENSE = LGPL-2.1+
 LIBP11_LICENSE_FILES = COPYING
 
-# pkg-config returns a libcrypto enginesdir prefixed with the sysroot,
-# so let's rip it out.
 LIBP11_CONF_OPTS = \
-	--with-enginesdir=`$(PKG_CONFIG_HOST_BINARY) --variable enginesdir libcrypto | xargs readlink -f | sed 's%^$(STAGING_DIR)%%'`
+	--with-enginesdir=`$(PKG_CONFIG_HOST_BINARY) --variable enginesdir libcrypto`
 
 ifeq ($(BR2_PACKAGE_P11_KIT),y)
 LIBP11_CONF_OPTS += --with-pkcs11-module=/usr/lib/p11-kit-proxy.so