OSSystems / meta-browser

OpenEmbedded/Yocto BSP layer for Web Browsers
MIT License
183 stars 189 forks source link

chromium: use clang >= 16 #716

Open MaxIhlenfeldt opened 1 year ago

MaxIhlenfeldt commented 1 year ago

Chromium is going to switch to using C++20 soon (one or two months). According to Google's Peter Kasting, "Anything before clang 16 is basically not going to work".

Here's the current clang versions that we use:

i.e. every Yocto release is currently using a version that's going to cause a lot of problems very soon.

Maybe it's possible to provide clang 16 in a similar way to the recently introduced possibility of using Node 14 on dunfell?

MaxIhlenfeldt commented 1 year ago

I learned about this by coincidence talking to Peter. It's quite urgent, as the switch to C++20 seems to be only a few major versions away from being released :sweat_smile:

kraj commented 1 year ago

using clang16 just for chromium might be fine for dunfell and kirkstone however, this would need work since we wont be able to build rest of OE packages with clang16 without major surgery.

MaxIhlenfeldt commented 1 year ago

we wont be able to build rest of OE packages with clang16 without major surgery.

I would have expected using a newer clang version to be backwards compatible, what kind of problems can arise from updating clang?

using clang16 just for chromium might be fine

Is it possible to use clang16 just for chromium and use the default clang version otherwise?

kraj commented 1 year ago

we wont be able to build rest of OE packages with clang16 without major surgery.

I would have expected using a newer clang version to be backwards compatible, what kind of problems can arise from updating clang?

Think of this, we are trying to use a compiler released in 2023 to compile packages which were predominantly released in 2020, clang-16 was not around back then, so these packages may have code which new clang compiler will find errors in or expose errors which older compiler was incapable of. This is commonly seen issue when mixing newer compiler with older source code of packages.

using clang16 just for chromium might be fine

Is it possible to use clang16 just for chromium and use the default clang version otherwise?

I think yes. If we create dunfell_clang16 then this branch may only be used to compile chromium alone. this will be troublesome for folks who are using clang as default system compiler with dunfell and also using chromium and want to upgrade to latest chromium. They will have to switch system compiler to gcc and select clang only for chromium for best results.

MaxIhlenfeldt commented 1 year ago

This is commonly seen issue when mixing newer compiler with older source code of packages.

I had feared that my expectation regarding backwards compatibility might be a bit optimistic, thanks for confirming.

this will be troublesome for folks who are using clang as default system compiler with dunfell and also using chromium and want to upgrade to latest chromium. They will have to switch system compiler to gcc and select clang only for chromium for best results.

Why is that? Is it not possible to use clang 12 as the default system compiler and clang 16 for Chromium only? Either way, I suppose it'd be a reasonable restriction, given that the alternative is not supporting Chromium on dunfell at all.

MaxIhlenfeldt commented 1 year ago

Updating to m115 gives more errors that presumably wouldn't happen with clang 16:

| In file included from ../../chrome/test/chromedriver/capabilities.cc:5:
| In file included from ../../chrome/test/chromedriver/capabilities.h:10:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/map:541:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__node_handle:63:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/memory:846:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__memory/allocate_at_least.h:13:
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__memory/allocator_traits.h:298:9: error: no matching function for call to 'construct_at'
|         _VSTD::construct_at(__p, _VSTD::forward<_Args>(__args)...);
|         ^~~~~~~~~~~~~~~~~~~
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__config:658:17: note: expanded from macro '_VSTD'
| #  define _VSTD std
|                 ^
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/vector:818:21: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<BrandVersion>>::construct<BrandVersion, const std::string &, const std::string &, void, void>' requested here
|     __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__tx.__pos_),
|                     ^
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/vector:1631:9: note: in instantiation of function template specialization 'std::vector<BrandVersion>::__construct_one_at_end<const std::string &, const std::string &>' requested here
|         __construct_one_at_end(_VSTD::forward<_Args>(__args)...);
|         ^
| ../../chrome/test/chromedriver/capabilities.cc:358:16: note: in instantiation of function template specialization 'std::vector<BrandVersion>::emplace_back<const std::string &, const std::string &>' requested here
|         brands.emplace_back(*brand, *version);
|                ^
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__memory/construct_at.h:33:38: note: candidate template ignored: substitution failure [with _Tp = BrandVersion, _Args = <const std::string &, const std::string &>]: no matching constructor for initialization of 'BrandVersion'
| _LIBCPP_HIDE_FROM_ABI constexpr _Tp* construct_at(_Tp* __location, _Args&&... __args) {
|                                      ^
| 1 error generated.

(I suspect the substitution error mentioned at the end doesn't happen with clang >= 16.)

As already mentioned, there will be more errors as slowly the code base uses more and more C++20 features (this might not even be the only error of this kind in m115). What's the plan for moving to using clang 16 on dunfell, kirkstone and mickledore?

rakuco commented 1 year ago

FWIW, I've been toying with a dunfell-clang14 branch that's basically cherry-picking a ton of commits from meta-clang's kirkstone branch. chromium-x11 and chromium-ozone-wayland built fine, but I haven't tried running the binaries yet.

MaxIhlenfeldt commented 1 year ago

That sounds promising! But I fear using clang 14 won't get rid of all the errors, as even mickledore with clang 15 is causing problems :/

rakuco commented 1 year ago

That's probably true. At this point I'm trying to do this as a learning experience; if this works, we can have both dunfell and kirkstone using the same compiler version, in which case moving both to a newer version.

With that said, clang 15 is less than a year old, so my only fear is that we'll always spend a non-trivial amount of time chasing compiler failures simply because Chromium's always using LLVM's main branch. Building the Chromium recipe with Chromium's LLVM is probably a no-go :/

MaxIhlenfeldt commented 1 year ago

if this works, we can have both dunfell and kirkstone using the same compiler version, in which case moving both to a newer version.

Ah, good point. That does sound useful.

so my only fear is that we'll always spend a non-trivial amount of time chasing compiler failures simply because Chromium's always using LLVM's main branch

It did work relatively well in the past, didn't it? I think what's currently causing the increased amount of problems is upstream's switch to C++20, which isn't well supported before clang 16.

MaxIhlenfeldt commented 11 months ago

How should we proceed here now that meta-clang has a dunfell-clang14 branch? The update to 117 once more needs many build fixes that wouldn't be needed with clang 16. And while these fixes usually are trivial, they still take up a lot of my time :slightly_frowning_face:

rakuco commented 11 months ago

I think the course of action would be figuring out which meta-clang branch has clang 16 and trying to backport any required changes to a kirkstone-clang16 branch and then try to replicate the same thing to a dunfell-clang16 branch. Easier said than done though :smile:

I don't have much spare time to work on this myself, but can try to help review any changes.

MaxIhlenfeldt commented 11 months ago

Sounds reasonable!

@kraj would you have time for that work? If not, I suppose I could give it a try.

kraj commented 11 months ago

Sounds reasonable!

@kraj would you have time for that work? If not, I suppose I could give it a try.

we use clang16 on mickledore and moved master to clang17, dunfell will be EOL in Apr 24 so lets see if we can live with clang14

MaxIhlenfeldt commented 11 months ago

we use clang16 on mickledore

https://github.com/kraj/meta-clang/blob/mickledore/recipes-devtools/clang/clang.inc#L7 shows clang 15 being used, is this still WIP?

lets see if we can live with clang14

kirkstone is also still on clang 14 and will be supported until April 2026, so we definitely need to update to clang 16. And iiuc, meta-clang's dunfell-clang14 and kirkstone branches are quite similar, so if we get clang 16 to work on kirkstone, we might also get it to work on dunfell? There will be another seven Chromium releases until April 2024, I think that's enough to warrant at least trying.

kraj commented 11 months ago

I meant to say clang15 not 16.

MaxIhlenfeldt commented 11 months ago

Thanks for the clarification!

Do you have time to look into updating mickledore and kirkstone to clang 16?

kraj commented 11 months ago

Thanks for the clarification!

Do you have time to look into updating mickledore and kirkstone to clang 16?

Not in coming 2-4 weeks. Perhaps after October

MaxIhlenfeldt commented 9 months ago

@kraj do you think you'll have time for this soon?

MaxIhlenfeldt commented 9 months ago

It was just announced that many C++20 features are now allowed in Chromium, so maintaining support for clang 14 will probably get very hard very soon.

MaxIhlenfeldt commented 6 months ago

It seems like with Chromium 121 it's not really feasible to make it work with clang < 16. I will focus on getting nanbield and master to compile, but the amount of patches needed for kirkstone seems too high. (e.g. just look at https://crrev.com/c/5027747 and its relation chain)

@kraj if possible, can you please take a look at providing a meta-clang/kirkstone-clang16 branch?

kraj commented 6 months ago

@MaxIhlenfeldt I guess we have to bite the bullet, perhaps its better to move to clang-18 in one go. So perhaps we can live with that for a while.

MaxIhlenfeldt commented 6 months ago

@MaxIhlenfeldt I guess we have to bite the bullet, perhaps its better to move to clang-18 in one go. So perhaps we can live with that for a while.

You're right, probably longer than with clang 16. If clang 18 is possible, that would be great of course!

rwmacleod commented 5 months ago

@MaxIhlenfeldt @kraj Is there something that either @rjanani-p or I can do to help this along?