Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[qt5-webengine] fix jumbo build error due to ResolveColor() redefinition #37453

Merged

Conversation

tsondergaard
Copy link
Contributor

Both of the following two files define a static function ResolveColor() with the exact same signature causing a redefinition error when running a jumbo build

  • third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc
  • third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.cc

Error reported:

    In file included from gen/third_party/blink/renderer/core/layout/svg/svg_layout_jumbo_4.cc:16:
    ./../../../../src/5.15.13-38459bd7fb.clean/src/3rdparty/chromium/third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc:255:30: error: redefinition of 'base::Optional<blink::Color> blink::ResolveColor(const blink::ComputedStyle&, const blink::SVGPaint&, const blink::SVGPaint&)'
      255 | static base::Optional<Color> ResolveColor(const ComputedStyle& style,
          |                              ^~~~~~~~~~~~
    In file included from gen/third_party/blink/renderer/core/layout/svg/svg_layout_jumbo_4.cc:5:
    ./../../../../src/5.15.13-38459bd7fb.clean/src/3rdparty/chromium/third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.cc:97:30: note: 'base::Optional<blink::Color> blink::ResolveColor(const blink::ComputedStyle&, const blink::SVGPaint&, const blink::SVGPaint&)' previously defined here
       97 | static base::Optional<Color> ResolveColor(const ComputedStyle& style,
         |                              ^~~~~~~~~~~~
    In file included from gen/third_party/blink/renderer/core/layout/svg/svg_layout_jumbo_4.cc:16:
    ./../../../../src/5.15.13-38459bd7fb.clean/src/3rdparty/chromium/third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc:255:30: warning: 'base::Optional<blink::Color> blink::ResolveColor(const blink::ComputedStyle&, const blink::SVGPaint&, const blink::SVGPaint&)' defined but not used [-Wunused-function]
      255 | static base::Optional<Color> ResolveColor(const ComputedStyle& style,

Fixes #37448

See also QTBUG-123328

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@JonLiu1993 JonLiu1993 added the category:port-bug The issue is with a library, which is something the port should already support label Mar 15, 2024
@JonLiu1993 JonLiu1993 marked this pull request as draft March 18, 2024 10:17
@JonLiu1993
Copy link
Member

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". That way, I can be aware that you've responded since you can't modify the tags.

@tsondergaard tsondergaard force-pushed the qt5-webengine-5.15.13-build-error branch from 0233058 to 91a3ec6 Compare March 18, 2024 12:57
@tsondergaard tsondergaard marked this pull request as ready for review March 18, 2024 12:59
@tsondergaard tsondergaard force-pushed the qt5-webengine-5.15.13-build-error branch from 91a3ec6 to f1b20f9 Compare March 18, 2024 13:01
Both of the following two files define a static function
ResolveColor() with the exact same signature causing a redefinition
error when running a jumbo build

* third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc
* third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.cc

Error reported:

    In file included from gen/third_party/blink/renderer/core/layout/svg/svg_layout_jumbo_4.cc:16:
    ./../../../../src/5.15.13-38459bd7fb.clean/src/3rdparty/chromium/third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc:255:30: error: redefinition of 'base::Optional<blink::Color> blink::ResolveColor(const blink::ComputedStyle&, const blink::SVGPaint&, const blink::SVGPaint&)'
      255 | static base::Optional<Color> ResolveColor(const ComputedStyle& style,
          |                              ^~~~~~~~~~~~
    In file included from gen/third_party/blink/renderer/core/layout/svg/svg_layout_jumbo_4.cc:5:
    ./../../../../src/5.15.13-38459bd7fb.clean/src/3rdparty/chromium/third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.cc:97:30: note: 'base::Optional<blink::Color> blink::ResolveColor(const blink::ComputedStyle&, const blink::SVGPaint&, const blink::SVGPaint&)' previously defined here
       97 | static base::Optional<Color> ResolveColor(const ComputedStyle& style,
         |                              ^~~~~~~~~~~~
    In file included from gen/third_party/blink/renderer/core/layout/svg/svg_layout_jumbo_4.cc:16:
    ./../../../../src/5.15.13-38459bd7fb.clean/src/3rdparty/chromium/third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc:255:30: warning: 'base::Optional<blink::Color> blink::ResolveColor(const blink::ComputedStyle&, const blink::SVGPaint&, const blink::SVGPaint&)' defined but not used [-Wunused-function]
      255 | static base::Optional<Color> ResolveColor(const ComputedStyle& style,

Fixes microsoft#37448

See also QTBUG-123328
@tsondergaard tsondergaard force-pushed the qt5-webengine-5.15.13-build-error branch from f1b20f9 to ab9b7f6 Compare March 19, 2024 12:39
JonLiu1993
JonLiu1993 previously approved these changes Mar 20, 2024
@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Mar 20, 2024
@BillyONeal
Copy link
Member

Why does this build problem affecting jumbo builds affect vcpkg, which does not do jumbo builds?

@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Mar 20, 2024
@tsondergaard
Copy link
Contributor Author

tsondergaard commented Mar 20, 2024

@BillyONeal, I don't see anything in ports/qt5-webengine that explicitly disables jumbo builds. This is from config-x64-linux-dynamic-dbg-err.log on my AlmaLinux 9.3:

Checking for xproto (glproto)... yes
Checking for xtst... yes
Checking for xkbfile... yes
Checking for jumbo build merge limit... 8
Checking for linker supports -z noexecstack... yes
Checking for freetype >= 2.4.2... yes
Checking for glib-2.0 >= 2.32.0... no
Checking for harfbuzz >= 2.4.0... yes
Checking for icu >= 65... yes
Checking for compatible jpeglib... yes
Checking for lcms2... no
Checking for libevent... no
Checking for libvpx... no
Checking for libwebp, libwebpmux and libwebpdemux... yes
Checking for compatible libxml2 and libxslt... no
Checking for minizip... no
Checking for system ninja... yes
Checking for opus... no
Checking for libpng >= 1.6.0... yes
Checking for re2... no
Checking for snappy... no
Checking for zlib... yes
Checking for alsa... no
Checking for embedded build... no
Checking for poppler-cpp... no
Checking for pulseaudio >= 0.9.10... no
Done running configuration tests.

Configure summary:

Qt WebEngine Build Tools:
  Use System Ninja ....................... yes
  Jumbo Build Merge Limit ................ 8
  Developer build ........................ no

I believe the Jumbo build merge limit is auto-detected by the QtWebEngine configure scripts. The jumbo-build of QtWebEngine, or Chromium really, can be explicitly turned off, but that is just going to make it build a lot slower and it is already painfully slow.

@dg0yt
Copy link
Contributor

dg0yt commented Mar 21, 2024

Why does this build problem affecting jumbo builds affect vcpkg, which does not do jumbo builds?

qt5-webenengine embeds a chromium build. That's the root of all difficulties with this port.

@tsondergaard
Copy link
Contributor Author

One more thing to point out - I believe that the jumbo merge limit has an impact on whether the compile will fail or not. The jumbo build creates these bunches of sources and compiles them in a single compilation unit. If third_party/blink/renderer/core/layout/svg/svg_layout_tree_as_text.cc and third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.cc end up being compiled in the same compilation unit then you will run into this bug.

@BillyONeal
Copy link
Member

I don't see anything in ports/qt5-webengine that explicitly disables jumbo builds.

Jumbo builds are normally an opt in thing rather than opt out :sigh: .

@BillyONeal BillyONeal added the info:reviewed Pull Request changes follow basic guidelines label Mar 23, 2024
@BillyONeal BillyONeal merged commit 77fc422 into microsoft:master Mar 25, 2024
16 checks passed
@BillyONeal
Copy link
Member

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
4 participants