Re: [meta-arm] [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

Hi,

On Wed, Oct 30, 2024 at 08:02:19AM +0000, Gyorgy Szing wrote:
> Hi,
> 
> (Sorry for the delayed reply.)
> 
> Another way to address this would be to use PkgConfig from CMakeLists.txt (see [1]). This might be considered a nicer solution as the yocto recipe would not have to correct the install location, and thus there would be a single source of truth. (Kudos for Ross for the suggestion.)
> Something like (code is not tested):
> 
> Include(FindPkgConfig)
> If (PKG_CONFIG_FOUND)
>                 # TODO: there are other variables. Doublecheck if systemdsystemconfdir is the right one to be used. On X86 Ubuntu this returns /etc/systemd/system. Yocto might define distro specific values.
>                 pkg_get_variable(UNIT_DIR systemd systemdsystemconfdir)
>                 # TODO: check the value to see if pkgconfig successfully found the value. Behavior is not documented, probably if (UNIT_DIR) would work.
>                 # TODO: UNIT_DIR is a string list, use only the first or last value? Rewrite the line below accordingly
> set(CMAKE_INSTALL_SYSTEMDSYSCONFDIR “${ UNIT_DIR }” CACHE PATH “Target directory for system config files”)
> endif()
> # By default, use LIBDIR
> # TODO: CMAKE_INSTALL_SYSCONFDIR might be a better default base dir.
> set(CMAKE_INSTALL_ SYSTEMDSYSCONFDIR “${ CMAKE_INSTALL_LIBDIR}}/systemd/system” CACHE PATH “Target directory for system config files”)
> 
> install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/[email protected] DESTINATION ${ CMAKE_INSTALL_SYSTEMDSYSCONFDIR})
> 
> 
> The above code would default to the current install location under LIBDIR,  and would use PkgConfig if available. Setting CMAKE_INSTALL_ SYSTEMDSYSCONFDIR from the command line allows manual override.
> 
> 1: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html

This would only work if optee-client has "systemd" build time dependency in DEPENDS.
I don't think we should add this dependency. The sd_notify() support for example
dlopen()'s systemd libs at runtime to avoid making systemd a hard build time
dependency. I don't think we should add this dependency due to systemd
service file install paths.

Cheers,

-Mikko

> /George
> 
> On 2024-10-29, 18:03, "[email protected]" <[email protected]> wrote:
> 
> On 10/23/2024 9:54 AM, Mikko Rapeli wrote:
> > Ok got it now.
> >
> > ${nonarch_base_libdir} isn't in the CMake toolchain file. We could
> > move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
> > in the do_install task. Could you test and submit a patch like below?
> >
> > --- a/meta-arm/recipes-security/optee/optee-client.inc
> > +++ b/meta-arm/recipes-security/optee/optee-client.inc
> > @@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
> >
> >   do_install:append() {
> >       # installed by default
> > +    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
> >       if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> > -        rm -rf ${D}${libdir}/systemd
> > +        rm -rf ${D}${noarch_base_libdir}/systemd
> >       fi
> >       if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
> >           install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
> >
> > Cheers,
> >
> > -Mikko
> 
> This works fine in our testing. Sorry for the late response.
> 
> Thanks again!
> 
> Tom