Skip to content

qt5.qtwebengine: Fix build on darwin#358601

Closed
azuwis wants to merge 1 commit intoNixOS:masterfrom
azuwis:push-zosyvxsrmrnw
Closed

qt5.qtwebengine: Fix build on darwin#358601
azuwis wants to merge 1 commit intoNixOS:masterfrom
azuwis:push-zosyvxsrmrnw

Conversation

@azuwis
Copy link
Contributor

@azuwis azuwis commented Nov 24, 2024

../../3rdparty/chromium/third_party/harfbuzz-ng/src/src/hb-ft.cc:759:73: error: cast from 'void (*)(FT_Face)' (aka 'void (*)(FT_FaceRec_ *)') to 'FT_Generic_Finalizer' (aka 'void (*)(void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

```
../../3rdparty/chromium/third_party/harfbuzz-ng/src/src/hb-ft.cc:759:73: error: cast from 'void (*)(FT_Face)' (aka 'void (*)(FT_FaceRec_ *)') to 'FT_Generic_Finalizer' (aka 'void (*)(void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]
```
@github-actions github-actions bot added the 6.topic: qt/kde Object-oriented framework for GUI creation label Nov 24, 2024
@emilazy
Copy link
Member

emilazy commented Nov 24, 2024

I believe that on Linux we devendor harfbuzz-ng, right? It would be really nice if we could do the same for Darwin.

@azuwis
Copy link
Contributor Author

azuwis commented Nov 24, 2024

Some weird thing happened.

https://hydra.nixos.org/job/nixpkgs/trunk/qt5.qtwebengine.aarch64-darwin

Screenshot 2024-11-24 at 11 23 20

Grepping the logs of those 2 builds:

The green one:

Project MESSAGE: Using clang++ from /nix/store/3vhfsa0ysc545n970bf7jrjawp7yalcn-clang-wrapper-15.0.7/bin/clang++
clang version 15.0.7
Target: arm64-apple-darwin
Thread model: posix
InstalledDir: /nix/store/g1iqk1kj8q36zqwrmchwic7dkxmj8mq6-clang-15.0.7/bin

The red one:

Project MESSAGE: Using clang++ from /nix/store/v39c0h6xv6hvki2k1bsyp5n090q8y2bp-clang-wrapper-16.0.6/bin/clang++
clang version 16.0.6
Target: arm64-apple-darwin
Thread model: posix
InstalledDir: /nix/store/44hqr4414m3jldixkm4cq47q5pnpy83d-clang-16.0.6/bin

According to:

qtwebengine = callPackage ../modules/qtwebengine.nix {
# The version of Chromium used by Qt WebEngine 5.15.x does not build with clang 16 due
# to the following errors:
# * -Wenum-constexpr-conversion: This is a downgradable error in clang 16, but it is planned
# to be made into a hard error in a future version of clang. Patches are not available for
# the version of v8 used by Chromium in Qt WebEngine, and fixing the code is non-trivial.
# * -Wincompatible-function-pointer-types: This is also a downgradable error generated
# starting with clang 16. Patches are available upstream that can be backported.
# Because the first error is non-trivial to fix and suppressing it risks future breakage,
# clang is pinned to clang 15. That also makes fixing the second set of errors unnecessary.
stdenv = if stdenv.cc.isClang then overrideLibcxx llvmPackages_15.stdenv else stdenv;
inherit (srcs.qtwebengine) version;
inherit (darwin) bootstrap_cmds;
python = python3;
};

Clang 15 should be used, commit ab80059 broke it? @reckenrode

@azuwis
Copy link
Contributor Author

azuwis commented Nov 24, 2024

I believe that on Linux we devendor harfbuzz-ng, right? It would be really nice if we could do the same for Darwin.

While grepping the logs, I find use_system_harfbuzz=true args is added to the gn command in the linux build logs, but not in the darwin build.

I believe use_system_harfbuzz=true will be automatically set on linux, but not on darwin.

@emilazy
Copy link
Member

emilazy commented Nov 24, 2024

IIRC some of the devendoring flags only function on Linux because of weird Chromium build system stuff. But maybe this one works?

@azuwis
Copy link
Contributor Author

azuwis commented Nov 24, 2024

While devendoring is nice, but I think it won't fix the build because:

* -Wenum-constexpr-conversion: This is a downgradable error in clang 16, but it is planned 
  to be made into a hard error in a future version of clang. Patches are not available for 
  the version of v8 used by Chromium in Qt WebEngine, and fixing the code is non-trivial.

Need to figure out why overrideLibcxx llvmPackages_15.stdenv does not work.

@emilazy
Copy link
Member

emilazy commented Nov 24, 2024

Fair enough. FWIW, I think we won‘t ship Qt 5 WebEngine in 25.05 due to the upcoming EOL, so it might not be worth spending too much time on it anyway.

@K900 Maybe we should just work to drop Qt 5 WebEngine early in the cycle? There’s no reasonable expectation that the KDE people are going to be able to keep adequately on top of Chromium security backports, right?

@K900
Copy link
Contributor

K900 commented Nov 24, 2024

Would be nice, but probably impractical. If someone wants to go and estimate the blast radius, that would be good though.

@emilazy
Copy link
Member

emilazy commented Nov 24, 2024

Well, I feel like the alternative is that we end up having to set it knownVulnerabilities a month after 25.05 release, which is all the same blast but with far more pain. Do we expect that a significant number of users will migrate off it within the next half a year?

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Nov 24, 2024
@ofborg ofborg bot requested a review from matthewbauer November 24, 2024 19:25
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 24, 2024
@azuwis azuwis closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 6.topic: qt/kde Object-oriented framework for GUI creation 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants