[libcamera-devel] [oe] [meta-multimedia][PATCH] libcamera: fix packaging and installation

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Jul 27 18:03:10 CEST 2020


Hi Andrey,

On Mon, Jul 27, 2020 at 06:46:47PM +0300, Andrey Konovalov wrote:
> On 27.07.2020 14:11, Laurent Pinchart wrote:
> > On Mon, Jul 27, 2020 at 12:58:23PM +0300, Andrey Konovalov wrote:
> >> On 27.07.2020 12:42, Kieran Bingham wrote:
> >>> On 27/07/2020 10:21, Andrey Konovalov wrote:
> >>>> libcamera checks if RPATH or RUNPATH dynamic tag is present in
> >>>> libcamera.so. If it does, it assumes that libcamera binaries are
> >>>> run directly from the build directory without installing them, and
> >>>> tries to use resorces like IPA modules from the build directory.
> >>>> Mainline meson strips RPATH/RUNPATH out at install time (for
> >>>> meson versions up to 0.54; the things are somewhat changed in 0.55).
> >>>> But openembedded-core patches meson to disable RPATH/RUNPATH removal.
> >>>> That's why we need to remove this tag manually in do_install_append().
> >>>
> >>> Uh oh, what's changed... (I'll have to go take a look).
> >>>
> >>>    -
> >>> https://mesonbuild.com/Release-notes-for-0-55-0.html#rpath-removal-now-more-careful
> >>>
> >>> If we're reliant upon meson behaviour which is no longer consistent,
> >>> then we are going to have to do something else in libcamera.
> >>
> >> I haven't tried meson 0.55 yet, but my impression was that 0.55 should work
> >> just as before for "usual" (as per libcamera's README) libcamera build. And
> >> starting from 0.55 the patch in openembedded-core to disable RPATH/RUNPATH removal
> >> *might* be dropped - if all the packages would be able to set RUNPATH to
> >> what they need, and meson would detect that OK in all those cases.
> > 
> > I think that if the problem is caused by a meson patch in openembedded,
> > then it would make sense to fix it there. We can decide to address the
> > issue in libcamera itself if it's found to affect other distributions
> > too, or if meson's behaviour changes in an incompatible way.
> 
> It looks like it is not openembedded only issue:
> 
> -------- Forwarded Message --------
> Subject: [libcamera-devel] [PATCH v4 0/2] package/libcamera: bump version to 96fab38
> Date: Tue, 16 Jun 2020 20:59:49 +0200
> From: Peter Seiderer <ps.report at gmx.net>
> To: buildroot at busybox.net
> CC: libcamera-devel at lists.libcamera.org, Yann E . MORIN <yann.morin.1998 at free.fr>
> 
> <snip>
> 
> With the following patch libcamera is forced to believe it is running
> in a installed environment:
> 
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index d55338f..4ff9dac 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -346,15 +346,18 @@ details::StringSplitter split(const std::string &str, const std::string &delim)
>    */
>   bool isLibcameraInstalled()
>   {
> +#if 0
>   	/*
>   	 * DT_RUNPATH (DT_RPATH when the linker uses old dtags) is removed on
>   	 * install.
>   	 */
>   	for (const ElfW(Dyn) *dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn) {
> -		if (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH)
> +		if (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH) {
> +			printf("XXXXX - dyn->d_un.d_ptr: %s\n", (char*)dyn->d_un.d_ptr);
>   			return false;
> +		}
>   	}
> -
> +#endif
>   	return true;
>   }
> 
> Maybe this is because of the buildroot local meson patch ([1]), leading
> to an empty (but not absent) RPATH?

buildroot preserves empty RPATH when installing. Maybe we could adapt
isLibcameraInstalled() to return true only if RPATH is found *and* not
empty ?

For openembedded, why is RPATH stripping skipped ?

> <snip>
> 
> [0:02:18.125804232] [252] DEBUG IPAManager ipa_manager.cpp:316 IPA module /usr/lib/libcamera/ipa_rpi.so signature is not valid
> 
> <snip>
> 
> This can be avoided with the following patch/hack (disable signature check):
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 505cf61..3d64898 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -301,6 +301,9 @@ std::unique_ptr<IPAProxy> IPAManager::createIPA(PipelineHandler *pipe,
> 
>   bool IPAManager::isSignatureValid(IPAModule *ipa) const
>   {
> +#if 1
> +	return true;
> +#else
>   #if HAVE_IPA_PUBKEY
>   	File file{ ipa->path() };
>   	if (!file.open(File::ReadOnly))
> @@ -320,6 +323,7 @@ bool IPAManager::isSignatureValid(IPAModule *ipa) const
>   #else
>   	return false;
>   #endif
> +#endif
>   }
> 
>   } /* namespace libcamera */
> 
> 
> Maybe related to the buildroot finalize and/or sanitizing RPATH in target tree
> step (and/or strip after install with BR2_ENABLE_DEBUG=y/BR2_STRIP_strip=y
> enabled)?

For this, let's first see how module re-signing works with
openemebedded, possibly improving the resigning script. If it's
successful, I think we can then use the same methods for buildroot and
other distributions. A packaging document to explain all this would be
useful.

> -------- End of Forwarded Message --------
> 
> >>> /me sighs ...
> >>>
> >>>> IPA module is signed (with openssl dgst) after it is built. But
> >>>> during packaging the OE build system 1) splits out debugging info,
> >>>> and 2) strips the binaries. So the IPA module *.so file installed
> >>>> isn't the one which the signature was calculated against. Then
> >>>> the signature check fails, and libcamera tries to run the IPA
> >>>> module isolated (in a sandbox), which doesn't work if the IPA
> >>>> module wasn't designed to run isolated. The easiest way to fix that
> >>>> is to disable splitting out debug information and stripping the binaries
> >>>> during packaging with INHIBIT_PACKAGE_DEBUG_SPLIT and
> >>>> INHIBIT_PACKAGE_STRIP.
> >>>
> >>> This sounds like an effective solution for openembedded, but it needs to
> >>> be fixed in libcamera all the same.
> >>>
> >>> I'll try to follow up with the meson guys to see what we can do,.
> > 
> > We re-sign the IPA modules at install time for this very specific
> > reason. If openembedded modifies the binaries after installing them,
> > should it re-run the signing script ?
> > 
> >>>> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
> >>>> ---
> >>>>    .../recipes-multimedia/libcamera/libcamera.bb            | 9 ++++++++-
> >>>>    1 file changed, 8 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/meta-multimedia/recipes-multimedia/libcamera/libcamera.bb b/meta-multimedia/recipes-multimedia/libcamera/libcamera.bb
> >>>> index 00a5c480d..573366f08 100644
> >>>> --- a/meta-multimedia/recipes-multimedia/libcamera/libcamera.bb
> >>>> +++ b/meta-multimedia/recipes-multimedia/libcamera/libcamera.bb
> >>>> @@ -18,13 +18,20 @@ PV = "202006+git${SRCPV}"
> >>>>    
> >>>>    S = "${WORKDIR}/git"
> >>>>    
> >>>> -DEPENDS = "python3-pyyaml-native udev gnutls boost"
> >>>> +DEPENDS = "python3-pyyaml-native udev gnutls boost chrpath-native"
> >>>>    DEPENDS += "${@bb.utils.contains('DISTRO_FEATURES', 'qt', 'qtbase qtbase-native', '', d)}"
> >>>>    
> >>>>    RDEPENDS_${PN} = "${@bb.utils.contains('DISTRO_FEATURES', 'wayland qt', 'qtwayland', '', d)}"
> >>>>    
> >>>>    inherit meson pkgconfig python3native
> >>>>    
> >>>> +do_install_append() {
> >>>> +        chrpath -d ${D}${libdir}/libcamera.so
> >>>
> >>> Aha, I didn't know about chrpath, that looks helpful. Perhaps part of
> >>> the solution will be handling our own strip/install actions to do this
> >>> explicitly in the build.
> >>>
> >>> It will be a pain to have to pull in another external dependency though...
> >>>
> >>>> +}
> >>>> +
> >>>>    FILES_${PN}-dev = "${includedir} ${libdir}/pkgconfig"
> >>>>    FILES_${PN} += " ${libdir}/libcamera.so"
> >>>>    
> >>>> +INHIBIT_PACKAGE_DEBUG_SPLIT = "1"
> >>>> +INHIBIT_PACKAGE_STRIP = "1"
> >>>> +

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list