chromium: Make yocto-check-layer pass

[nrpt-m]

As explained in: https://github.com/OSSystems/meta-browser/commit/2f5fe79935e1f5350f9e487625343a8f27b165d3 Chromium doesn't allow building with system libwayland anymore, and needs (a static version of) libffi to build its own libwayland. Therefore, libffi_%.bbappend was added to make libffi_pic.a (static library) available while building chromium-ozone-wayland.

This libffi_%.bbappend is causing yocto-check-layer to give compliance error for meta-chromium layer.

By adding "use_system_libffi=true" to GNARGS in chromium-ozone-wayland recipe and removing the libffi%.bbappend, we can make chromium-ozone-wayland to build with libffi.so (shared library). With these changes, we can avoid using libffi_pic.a (static library) for chromium-ozone-wayland build and also resolve yocto-check-layer compliance error for meta-chromium layer.

Added Upstream-Status to required patches to fix test_patches_upstream_status failure in yocto-check-layer.

Fixes: #714

Signed-off-by: Narpat Mali narpat.mali@windriver.com

[nrpt-m]

Looks fine to me, although I need to defer testing to @MaxIhlenfeldt. Thanks for working on this!

From a process perspective, could you add something like Fixes #714 to the PR and commit message to connect this PR to the corresponding issue?

Added.

[MaxIhlenfeldt]

Thanks for the PR!

Unfortunately, I don't really have a good runtime testing setup. @rwmacleod I think you usually test chromium-x11, would it be possible for you to test chromium-wayland in this case?

[nrpt-m]

Have also observed that one of the test called "test_patches_upstream_status" in yocto-check-layer is also failing which is due to missing upstream status.

INFO: test_patches_upstream_status (common.CommonCheckLayer) INFO: ... expected failure INFO: Traceback (most recent call last): File "/buildarea/eng1/wrls/lts23/layers/oe-core/scripts/lib/checklayer/cases/common.py", line 87, in test_patches_upstream_status self.assertEqual(len(patches), 0 , \ AssertionError: 4 != 0 : Found following patches with malformed or missing upstream status: /buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/0009-nomerge-attribute-on-declaration-is-only-available-s.patch /buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/0017-Revert-Fix-std-span-autodetection-7805.patch /buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/0018-Only-default-operator-on-declaration.patch /buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/musl/no-res-ninit-nclose.patch

So, working on it and will add the changes in this PR itself.

[rwmacleod]

Thanks for the PR!

Unfortunately, I don't really have a good runtime testing setup. @rwmacleod I think you usually test chromium-x11, would it be possible for you to test chromium-wayland in this case?

Sure, @nrpt-m and I will figure out how to test cr-Wayland. It's been on my to do list for years!

[MaxIhlenfeldt]

/buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/0009-nomerge-attribute-on-declaration-is-only-available-s.patch

I think this patch shouldn't be needed anymore. It looks like it's only needed when building with clang < 12, which even on dunfell isn't the case anymore. I'll check if it can be removed as part of the 113 update PR.

/buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/0017-Revert-Fix-std-span-autodetection-7805.patch

Ah, I wanted to check with the relevant upstream maintainers regarding this one as part of the update to 112 but forgot, sorry about that. I still don't know why it's needed in the first place, but maybe they're still willing to accept a patch upstream. Feel free to ask them if you want, or I can do it, whatever works best for you.

/buildarea/eng1/wrls/lts23/layers/meta-browser/meta-chromium/recipes-browser/chromium/files/0018-Only-default-operator-on-declaration.patch

I don't think they accept patches that are needed for building with clang 12, as that's not supported (we have some other clang 12-specific patches marked as Inappropriate).

[nrpt-m]

@MaxIhlenfeldt, @rakuco @rwmacleod and I have built the core-image-weston with chromium-ozone-wayland for master branch and tested it. chromium executable is using the system libffi as below:

sh-5.28 ldd /usr/lib/chromium/chromium-bin | grep ffi libffi.so.8 => /usr/lib/libffi.so.8 (0x00007f8938895000)

Will test chromium-ozone-wayland for kirkstone & dunfell branches tomorrow.

[nrpt-m]

Will test chromium-ozone-wayland for kirkstone & dunfell branches tomorrow.

Have tested chromium-ozone-wayland for kirkstone & dunfell branches and below are my observations:

1. For master & kirkstone branch, core-image-weston image with chromium-ozone-wayland builds successfully and we see that the default id comes something like this: "uid=1000(weston) gid=1000 (weston) groups=19 (input), 44(video) 1000 (weston)". With "chromium" command chromium launches, works fine and uses system libffi.so.8 library.

sh-5.28 ldd /usr/lib/chromium/chromium-bin | grep ffi libffi.so.8 => /usr/lib/libffi.so.8 (0x00007f8938895000)

2. For dunfell branch, core-image-weston image with chromium-ozone-wayland builds successfully and but, we see that the default id comes something like this: "uid=0(root) gid=0(root) groups=0(root)". With "chromium" command chromium doesn't launches and it asks us use "chromium --no-sandbox" and with this command chromium launches, works fine and uses system libffi.so.7 library. If I switch to some user and run chromium then it says permission denied -> have attached a screenshot of it.

sh-5.28 ldd /usr/lib/chromium/chromium-bin | grep ffi libffi.so.7=> /usr/lib/libffi.so.7 (0x00007ff7fdac8000)

[kraj]

Will test chromium-ozone-wayland for kirkstone & dunfell branches tomorrow.

Have tested chromium-ozone-wayland for kirkstone & dunfell branches and below are my observations:

1. For master & kirkstone branch, core-image-weston image with chromium-ozone-wayland builds successfully and we see that the default id comes something like this: "uid=1000(weston) gid=1000 (weston) groups=19 (input), 44(video) 1000 (weston)". With "chromium" command chromium launches, works fine and uses system libffi.so.8 library.

sh-5.28 ldd /usr/lib/chromium/chromium-bin | grep ffi libffi.so.8 => /usr/lib/libffi.so.8 (0x00007f8938895000)

2. For dunfell branch, core-image-weston image with chromium-ozone-wayland builds successfully and but, we see that the default id comes something like this: "uid=0(root) gid=0(root) groups=0(root)". With "chromium" command chromium doesn't launches and it asks us use "chromium --no-sandbox" and with this command chromium launches, works fine and uses system libffi.so.7 library. If I switch to some user and run chromium then it says permission denied -> have attached a screenshot of it.

sh-5.28 ldd /usr/lib/chromium/chromium-bin | grep ffi libffi.so.7=> /usr/lib/libffi.so.7 (0x00007ff7fdac8000)

I think thats expected. in dunfell default user for weston shell was root where as in kirkstone it was changed to weston ( non-root user )

[nrpt-m]

@kraj, have added upstream-status for some of the patches so, could you please review it and let me know if these are correct or not.

[rakuco]

Generally I agree with this patch, however, chromium vendors all its dependencies and this might become problematic over time, although this library is not changed as frequently so we might be ok. It would be good to ask upstream if they intend to keep supporting using system provided libffi in future.

Looking at https://chromium.googlesource.com/chromium/src/+/main/build/config/linux/libffi/BUILD.gn, this should be safe as long as ChromeOS continues to use libffi as a shared library. According to the comment there, they are basically trying to avoid a mismatch between the version used to build Chromium and the version used at runtime. I believe OE/Yocto aren't prone to this because a change to libffi should cause Chromium to be rebuilt as well (IIRC).

[nrpt-m]

The changes look fine to me, but I think it'd be better to have separate commits in this PR for the libffi part and the Upstream-Status bits since they are doing independent things. The PR title can then be updated to something more generic like "chromium: Make yocto-check-layer pass".

Done.