Opened 23 months ago
Closed 23 months 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 23 months ago by wurzelsand
Attachment: | main shortened.log added |
---|
comment:1 Changed 23 months ago by jmroot (Joshua Root)
Cc: | mcalhoun@… MarcusCalhoun-Lopez removed |
---|---|
Owner: | set to MarcusCalhoun-Lopez |
Status: | new → assigned |
comment:2 Changed 23 months ago by kencu (Ken)
comment:3 Changed 23 months 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 23 months 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 23 months 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 23 months ago by chrstphrchvz (Christopher Chavez)
Cc: | chrstphrchvz added |
---|
comment:7 Changed 23 months ago by mf2k (Frank Schima)
Cc: | mf2k added |
---|
comment:8 Changed 23 months 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 23 months 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 23 months ago by kencu (Ken)
Attachment: | patch-qt5-qtwebengine-RectF-ambiguous.diff added |
---|
comment:10 Changed 23 months ago by mf2k (Frank Schima)
Cc: | mf2k added |
---|
comment:12 Changed 23 months 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 23 months 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 23 months 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 23 months 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 23 months ago by devernay (Frédéric Devernay)
Cc: | devernay added |
---|
comment:18 Changed 23 months 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 23 months 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 23 months 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 23 months 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?