search

OP-TEE / optee_client

Normal World Client side of the TEE
Other
188 stars 235 forks source link

tee-supplicant@.service: Getting build error in case yocto uses sysvinit and don't use usrmerge #393

Open sahilnxp opened 3 weeks ago

sahilnxp commented 3 weeks ago

Getting following error when compiling optee-client-ls with this patch through yocto. Including the meta-arm optee-client.inc in optee-client-ls My yocto environment has sysvinit and don't use usrmerge.

ERROR: optee-client-4.4.0+git-r0 do_package: QA Issue: optee-client: Files/directories were installed but not shipped in any package:
  /usr/lib/systemd
  /usr/lib/systemd/system
  /usr/lib/systemd/system/tee-supplicant@.service
Please set FILES such that these items are packaged. Alternatively if they are unneeded, avoid installing them or delete them within do_install.
optee-client: 3 installed and not shipped files. [installed-vs-shipped]

Recipe is installing the tee-supplicant@.service in {systemd_system_unitdir} which is in my case is /lib/ OP-TEE client src is installing it in /usr/lib/ - Which I think is causing the issue..

We just added the do_install: append() in our optee-client-ls recipe which is just deleting /usr/lib/systemd and this error disappars, but that is a workaround only for such environments.

Do we need to put some check in CMakefile so that it only gets installed on systemd enabled systems ?

cc: @thochstein

jforissier commented 3 weeks ago

CC: @mikkorapeli-linaro

mikkorapeli-linaro commented 3 weeks ago

Hi, sorry about the transitional issue. I plan to port this patch to meta-arm and remove the existing service file. The systemd service file will only be installed when systemd distro feature is selected. The bbappend workaround on your side is the right thing to do, IMO.

I did not add a optee-client side CMake flag to disable systemd support. Also the systemd sd-notify support is enabled unconditionally, though that does not show up as build/install or runtime problems if systemd is not available.

sahilnxp commented 3 weeks ago

I did not add a optee-client side CMake flag to disable systemd support. Also the systemd sd-notify support is enabled unconditionally, though that does not show up as build/install or runtime problems if systemd is not available.

IIUC, In our case it is throwing an error during packaging, I think it is better to have CMake config flag to disable systemd support, currently it is installing these files by default. User can keep this flag enabled/disabled based on the environment he is using.

mikkorapeli-linaro commented 3 weeks ago

Upstream meta-arm recipe changes are in https://lists.yoctoproject.org/g/meta-arm/message/6192

thochstein commented 3 weeks ago

Hi @mikkorapeli-linaro, the QA error only happens for our QorIQ devices, while our i.MX devices have no such problem. The difference seems to be that for QorIQ we do not use usrmerge. In this case, the cmake does install the service file to /usr/lib/systemd/system, while bitbake expects /lib/systemd/system, hence the QA error.

mikkorapeli-linaro commented 3 weeks ago

Which yocto branch are you building? AFAIK, usrmerge is mandatory in latest poky master branch when systemd is selected.

sahilnxp commented 3 weeks ago

@mikkorapeli-linaro We are currently using scarthgapbranch.

mikkorapeli-linaro commented 3 weeks ago

Hmm, usrmerge feature is really mandatory with systemd https://git.yoctoproject.org/poky/tree/meta/conf/distro/include/init-manager-systemd.inc?h=scarthgap https://git.yoctoproject.org/poky/tree/meta/recipes-core/systemd/systemd_255.4.bb?h=scarthgap#n13 So your configuration enables systemd but disables usrmerge on scarthgap?

sahilnxp commented 3 weeks ago

No, qoriq platforms don't use systemd, it uses sysvinitwithout usrmerge.

mikkorapeli-linaro commented 3 weeks ago

Ok, then I think the meta-arm patch change will cover your use case as it doesn't install the systemd file if systemd distro feature is not selected. Install paths are always standard CMake ones which expect usrmerge. I'm not sure if meta-arm scarthgap branch will accept the same patch but I can try.

sahilnxp commented 3 weeks ago

I am not so much familiar with CMAKE, but is there any variable in CMAKE, that can adjust to /usr/lib or /lib/ is usrmergeis enabled or not ? If there is one, we can use it here in place of CMAKE_INSTALL_LIBDIR

mikkorapeli-linaro commented 3 weeks ago

I don't think CMake supports /lib install paths via any variable. It's also about using the install prefix or not and for libdir it should be used.

As said, your use case is covered by the meta-arm side optee-client recipe changes. They remove the systemd service file if systemd distro feature is not selected.

thochstein commented 3 weeks ago

Given that usrmerge is required now for systemd, I think your recipe change is sufficient.