meta-browser icon indicating copy to clipboard operation
meta-browser copied to clipboard

chromium: use clang >= 16

Open MaxIhlenfeldt opened this issue 1 year ago • 24 comments

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:

  • dunfell: 12
  • kirkstone: 14
  • mickledore: 15

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 avatar May 30 '23 10:05 MaxIhlenfeldt

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:

MaxIhlenfeldt avatar Jun 05 '23 14:06 MaxIhlenfeldt

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.

kraj avatar Jun 06 '23 03:06 kraj

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?

MaxIhlenfeldt avatar Jun 15 '23 10:06 MaxIhlenfeldt

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.

kraj avatar Jun 15 '23 16:06 kraj

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 avatar Jun 20 '23 13:06 MaxIhlenfeldt

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?

MaxIhlenfeldt avatar Jul 24 '23 13:07 MaxIhlenfeldt

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.

rakuco avatar Jul 28 '23 07:07 rakuco

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 :/

MaxIhlenfeldt avatar Jul 28 '23 07:07 MaxIhlenfeldt

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 :/

rakuco avatar Jul 28 '23 08:07 rakuco

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 avatar Jul 28 '23 08:07 MaxIhlenfeldt

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:

MaxIhlenfeldt avatar Sep 27 '23 09:09 MaxIhlenfeldt

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.

rakuco avatar Sep 27 '23 09:09 rakuco

Sounds reasonable!

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

MaxIhlenfeldt avatar Sep 27 '23 09:09 MaxIhlenfeldt

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

kraj avatar Sep 27 '23 18:09 kraj

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.

MaxIhlenfeldt avatar Sep 28 '23 08:09 MaxIhlenfeldt

I meant to say clang15 not 16.

kraj avatar Sep 28 '23 15:09 kraj

Thanks for the clarification!

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

MaxIhlenfeldt avatar Sep 28 '23 15:09 MaxIhlenfeldt

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

kraj avatar Sep 28 '23 16:09 kraj

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

MaxIhlenfeldt avatar Nov 07 '23 13:11 MaxIhlenfeldt

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 avatar Nov 14 '23 09:11 MaxIhlenfeldt

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?

MaxIhlenfeldt avatar Feb 07 '24 11:02 MaxIhlenfeldt

@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.

kraj avatar Feb 07 '24 20:02 kraj

@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!

MaxIhlenfeldt avatar Feb 08 '24 10:02 MaxIhlenfeldt

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

rwmacleod avatar Mar 26 '24 20:03 rwmacleod