libsForQt5.qt5.qtwebengine: fix build with clang#358759
libsForQt5.qt5.qtwebengine: fix build with clang#358759emilazy merged 1 commit intoNixOS:stagingfrom
Conversation
github-actions
bot
added
the
6.topic: qt/kde
nix-owners
bot
requested review from
K900,
NickCao,
SuperSandro2000 and
ttuegel
tricktron
added
0.kind: ZHF Fixes
tricktron
force-pushed
the
fix-qt5-webengine-on-darwin
branch
from
dbd4066 to
99c38a8
Compare
|
@emilazy Changed to using fetchpatch to directly use the upstream patch and switched to staging. |
There was a problem hiding this comment.
I don’t think this will work; it’ll just add a .diff file to the source tree.
Could you fetch the upstream HarfBuzz commit instead and use extraPrefix/stripLen to apply it to the right place in the WebEngine source?
There was a problem hiding this comment.
@emilazy I cannot make it work with the upstream commit because it seems to patch more than we want which then breaks. See the latest commit for reference to try it out.
There was a problem hiding this comment.
I see – in that case, we can probably just use fetchurl with https://github.com/macports/macports-ports/raw/dd7bc40d8de48c762bf9757ce0a0672840c5d8c2/aqua/qt5/files/patch-qtwebengine_hb-ft.cc_error.diff.
There was a problem hiding this comment.
@emilazy Fetchurl also does not work, see latest commit again. I don't know why.
EDIT: Is it because of the timestamps at the end of the path and thus it cannot find the correct file?
There was a problem hiding this comment.
@emilazy I reverted back to using our own patch because fetchurl did not work due to timestamps.
There was a problem hiding this comment.
The timestamps are a normal part of the diff format. What’s the error you were getting?
There was a problem hiding this comment.
@emilazy I get the following error:
applying patch /nix/store/3nny5pr2xq6cak6wyivia8ibcj1r29iq-patch-qtwebengine_hb-ft.cc_error.diff
can't find file to patch at input line 3
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|--- src/3rdparty/chromium/third_party/harfbuzz-ng/src/src/hb-ft.cc 2024-09-27 18:46:30
|+++ src/3rdparty/chromium/third_party/harfbuzz-ng/src/src/hb-ft.cc 2024-09-27 18:46:18
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.
4 out of 4 hunks ignoredThere was a problem hiding this comment.
src/3rdparty/chromium/third_party/harfbuzz-ng/src/src/hb-ft.cc doesn’t look like the correct path (src/src seems unlikely) – it will probably work if you get the extraPrefix and stripLen values right.
There was a problem hiding this comment.
@emilazy I got confused and used fetchurl according to #358759 (comment) but now I use fetchpatch again with extraPrefix = "" and it finally works.
tricktron
force-pushed
the
fix-qt5-webengine-on-darwin
branch
from
4b7f37a to
24c494a
Compare
|
Successfully created backport PR for |
|
Git push to origin failed for staging-24.11 with exitcode 1 |
|
We can remove the stdenv override after this PR, right? --- a/pkgs/development/libraries/qt-5/5.15/default.nix
+++ b/pkgs/development/libraries/qt-5/5.15/default.nix
@@ -282,16 +282,6 @@ let
qtwayland = callPackage ../modules/qtwayland.nix {};
qtwebchannel = callPackage ../modules/qtwebchannel.nix {};
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; |
|
If |
3 participants
Things done
Supersedes #358601.
Fixes the build on clang / darwin. I mostly go the patches and ideas from https://trac.macports.org/ticket/70850.
Don't know if this needs to go to staging because all changes are gated behind
stdenv.cc.isClangand thus cannot break on other platforms.Needs 24.11 backport for ZHF #ZurichZHF
ZHF: #352882
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.