Hi Laurent,

On 27.07.2020 14:11, Laurent Pinchart wrote:
Hello,

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 <[email protected]>
To: [email protected]
CC: [email protected], Yann E . MORIN 
<[email protected]>

<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?

<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)?
-------- End of Forwarded Message --------

Thanks,
Andrey

/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 <[email protected]>
---
   .../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"
+




-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#85989): 
https://lists.openembedded.org/g/openembedded-devel/message/85989
Mute This Topic: https://lists.openembedded.org/mt/75819340/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-devel/unsub  
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to