Opened 2 years ago
Closed 2 years ago
#66165 closed defect (fixed)
qt5-qtwebengine @5.15.10_1 (aqua): Processing of port qt5-qtwebengine failed
Reported by: | wurzelsand | Owned by: | MarcusCalhoun-Lopez (Marcus Calhoun-Lopez) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.8.0 |
Keywords: | Cc: | chrstphrchvz (Christopher Chavez), mf2k (Frank Schima), devernay (Frédéric Devernay) | |
Port: | qt5-qtwebengine |
Description
Could not install qt5-qtwebengine on MacOS Ventura 13.0 (22A380) with Xcode Version 14.1 (14B47b). MacBook Pro Intel Core i5, 16 GB. Retried after sudo port clean qt5-qtwebengine
.
Attachments (2)
Change History (23)
Changed 2 years ago by wurzelsand
Attachment: | main shortened.log added |
---|
comment:1 Changed 2 years ago by jmroot (Joshua Root)
Cc: | mcalhoun@… MarcusCalhoun-Lopez removed |
---|---|
Owner: | set to MarcusCalhoun-Lopez |
Status: | new → assigned |
comment:2 Changed 2 years ago by kencu (Ken)
comment:3 Changed 2 years ago by kencu (Ken)
well, I'll embarass myself and post my last attempt, none of which worked:
diff --git a/src/3rdparty/chromium/third_party/blink/renderer/core/editing/finder/text_finder.cc b/src/3rdparty/chromium/third_party/blink/renderer/core/editing/finder/text_finder.cc index e41a894fc..a1db7d77f 100644 --- a/src/3rdparty/chromium/third_party/blink/renderer/core/editing/finder/text_finder.cc +++ b/src/3rdparty/chromium/third_party/blink/renderer/core/editing/finder/text_finder.cc @@ -629,13 +629,13 @@ gfx::RectF TextFinder::ActiveFindMatchRect() { if (!current_active_match_frame_ || !active_match_) return gfx::RectF(); - return gfx::RectF(FindInPageRectFromRange(EphemeralRange(ActiveMatch()))); + return gfx::RectF((const CGRect&)FindInPageRectFromRange(EphemeralRange(ActiveMatch()))); } Vector<gfx::RectF> TextFinder::FindMatchRects() { UpdateFindMatchRects(); - Vector<gfx::RectF> match_rects; + Vector<(const CGRect&)gfx::RectF> match_rects; match_rects.ReserveCapacity(match_rects.size() + find_matches_cache_.size()); for (const FindMatch& match : find_matches_cache_) { DCHECK(!match.rect_.IsEmpty());
perhaps someone who knows a little more c++ than I do can sort out how to do the cast right to remove the ambiguous call.
I'm going to try clang-14 next, as I have it installed... but it may have the same issue, as it's a similar vintage.
comment:4 Changed 2 years ago by kencu (Ken)
it turns out that forcing qt5-qtwebengine to use a specified compiler is not trivial, either. And then it wants to find other tools like dsymutil, that we also have to spec. Getting harder, not easier, down this path….
comment:5 Changed 2 years ago by chrstphrchvz (Christopher Chavez)
Likely not an ideal approach (and not something to propose to Qt without platform guards), but the only suggestion I currently have would be to try using a temporary CGRect
:
--- a/src/3rdparty/chromium/third_party/blink/renderer/core/editing/finder/text_finder.cc +++ b/src/3rdparty/chromium/third_party/blink/renderer/core/editing/finder/text_finder.cc @@ -629,7 +629,8 @@ gfx::RectF TextFinder::ActiveFindMatchRect() { if (!current_active_match_frame_ || !active_match_) return gfx::RectF(); - return gfx::RectF(FindInPageRectFromRange(EphemeralRange(ActiveMatch()))); + CGRect r = FindInPageRectFromRange(EphemeralRange(ActiveMatch())); + return gfx::RectF(r); } Vector<gfx::RectF> TextFinder::FindMatchRects() { @@ -639,7 +640,8 @@ match_rects.ReserveCapacity(match_rects.size() + find_matches_cache_.size()); for (const FindMatch& match : find_matches_cache_) { DCHECK(!match.rect_.IsEmpty()); - match_rects.push_back(match.rect_); + CGRect r = match.rect_; + match_rects.push_back(r); } return match_rects;
PS I would appreciate if a volunteer could try building the 5.15.11 update: https://github.com/macports/macports-ports/pull/16311
comment:6 Changed 2 years ago by chrstphrchvz (Christopher Chavez)
Cc: | chrstphrchvz added |
---|
comment:7 Changed 2 years ago by mf2k (Frank Schima)
Cc: | mf2k added |
---|
comment:8 Changed 2 years ago by kencu (Ken)
Cc: | mf2k removed |
---|
% port -v installed qt5-qtwebengine The following ports are currently installed: qt5-qtwebengine @5.15.11_0 (active) requested_variants='' platform='darwin 21' archs='x86_64' date='2022-11-06T12:59:07-0800'
comment:9 Changed 2 years ago by kencu (Ken)
The PR works, but the build on Monterey is also broken due to Xcode 14.1 -- Chris' idea worked. I hope it's the right bunch of fixes -- it seemed like they wanted to be CGRects. I will put up the patch for others to try.
A successful build on Ventura and we can PR it. Like Chris says, for upstream it will need __APPLE__
guards.
Changed 2 years ago by kencu (Ken)
Attachment: | patch-qt5-qtwebengine-RectF-ambiguous.diff added |
---|
comment:10 Changed 2 years ago by mf2k (Frank Schima)
Cc: | mf2k added |
---|
comment:12 Changed 2 years ago by kencu (Ken)
looks like this patch works on Ventura (Intel at least) as well:
% port -v installed qt5-qtwebengine The following ports are currently installed: qt5-qtwebengine @5.15.11_0 (active) requested_variants='' platform='darwin 22' archs='x86_64' date='2022-11-06T17:44:34-0800'
now to use it a bit and try to see if anything is broken.
comment:13 Changed 2 years ago by kencu (Ken)
I put a PR here, for anyone who wants to test it. Also included is Christopher's update, as it builds on that PR.
comment:14 Changed 2 years ago by chrstphrchvz (Christopher Chavez)
Ken, thanks for testing this and the update.
Looking again at float_rect.h and rect_f.h, I believe the intention was to rely on the gfx::RectF
method of FloatRect
for the conversion, and not the longer way of getting a CGRect
from a FloatRect
and then constructing a gfx::RectF
from a CGRect
; I would guess the existence of both approaches is what lead to the ambiguity error.
So a platform-agnostic alternative (probably still not ideal, but what I would prefer proposing to Qt) might be to duplicate what is done by the gfx::RectF
method of FloatRect
: use a temporary FloatRect
and pass its x/y/width/height to the gfx::RectF(float x, float y, float width, float height)
constructor, e.g.
- return gfx::RectF(FindInPageRectFromRange(EphemeralRange(ActiveMatch()))); + FloatRect r = FindInPageRectFromRange(EphemeralRange(ActiveMatch())); + return gfx::RectF(r.X(), r.Y(), r.Width(), r.Height());
comment:15 Changed 2 years ago by kencu (Ken)
Certainly both approaches indeed led to the ambiguity -- good for the compiler to fail to build it !
As it is a many-hours build, this is built, and I'm done for the time being, I'll test what I've got installed for a while here, while we see what upstream does with this.
Certainly 100% of current macOS systems are broken, Monterey and Ventura, so they'll have some heat on them to get it sorted properly once the "me toos" start piling on the upstream ticket.
comment:16 Changed 2 years ago by devernay (Frédéric Devernay)
Cc: | devernay added |
---|
comment:18 Changed 2 years ago by devernay (Frédéric Devernay)
I applied manually the patches from the PR to a failed build (didn't want to rebuild everything), and the build succeeded. Thank you for those fixes! Is there anything that prevents the PR from being merged?
comment:19 Changed 2 years ago by kencu (Ken)
I was going to merge the PR if we had a few users who found the final product worked OK.
CGRect or FloatRect both handle floats, so I think they're about equivalent. Upstream doesn't seem too interested in this issue, and tells people to move on to qt6.4 which uses a newer chromium without this issue, so we may not get any upstream guidance.
Does the qt5-qtwebengine seem to work right for you, Fred? I tried one port with it and it seemed to work OK, but I don't know if I hit the area this patch affected.
comment:20 Changed 2 years ago by devernay (Frédéric Devernay)
It builds, and the fix is simple enough and seems correct, but I don't have a text case for that. shipit
comment:21 Changed 2 years ago by kencu (Ken)
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
this issue is mentioned here, for a newer qtwebengine, but same error:
https://bugreports.qt.io/browse/QTBUG-108207?filter=-4&jql=text%20~%20%22qtwebengine%22%20order%20by%20created%20DESC
no fix as yet. I'll see what I can come up with. Some kind of cast might make the call unambiguous, but it's complicated c++ to work in...
upstream chromium has completely redone this and this call looks to no longer exist in this fashion upstream.
If all else fails, perhaps forcing some version of macports-clang-1[1-5] might work. Probably 14 would be a good try?