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

Jump to bottom

qt511.qtwebkit: fix on darwin #55983

Merged
merged 4 commits into from Mar 3, 2019
Merged

qt511.qtwebkit: fix on darwin #55983

merged 4 commits into from Mar 3, 2019

Conversation

veprbl
Copy link
Member

@veprbl veprbl commented Feb 18, 2019 •
edited

qt511.qtwebkit is currently broken on darwin. This also prevents qt5.full from building #55892

Things done
  • qt59.qtwebkit builds
  • qt511.qtwebkit builds
  • Tested

cc @alyssais who fixed qt59.qtwebkit earlier

@veprbl veprbl requested a review from ttuegel as a code owner February 18, 2019 03:54
@GrahamcOfBorg GrahamcOfBorg added the 6.topic: darwin Running or building packages on Darwin label Feb 18, 2019
veprbl
veprbl commented Feb 18, 2019
View reviewed changes
pkgs/development/libraries/qt-5/5.9/qtwebkit.patch
# Therefore WebKit provides adequate header files.
INCLUDEPATH = $${ROOT_WEBKIT_DIR}/Source/WTF/icu $$INCLUDEPATH
- LIBS += -licucore
+ LIBS += /usr/lib/libicucore.dylib
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@periklis Was there a specific reason you didn't use darwin.ICU instead?

@veprbl veprbl changed the title Pr/qt5 qtwebkit darwin fix qt511.qtwebkit: fix on darwin Feb 18, 2019
@veprbl
qt5.qtwebkit: link against libicucore.dylib from darwin.ICU instead o…
f212dd2 
…f /usr/lib

The impure library was introduced in

   775531c ('qt58: extend darwin compatibility')

cmake-based build can use a non-apple ICU if -DMACOS_USE_SYSTEM_ICU=OFF
is set.
@veprbl
qt5.qtwebkit: refactor version comparison
67186de 
This introduces usingAnnulenWebkitFork variable. Using version
check for qtbase.version is misleading because annulen's fork is not
tied to a specific qt5 release.
@veprbl
qt511.qtwebkit: fix on darwin
4bbd00e 
readline is used on macOS and iOS for some javascript console (JSC). It
seems to be easier to disable it than to fix it.
@veprbl
qt5.qtwebkit: don't set qmake variables for cmake build
518f997
@FRidh
Copy link
Member

FRidh commented Mar 3, 2019

Could you rebase and check again? If it builds for you on Darwin I think we should just go ahead and merge.

@veprbl
Copy link
Member Author

veprbl commented Mar 3, 2019

This builds with

nix-review pr  --build-args "--option build-use-sandbox false" 55983 -p qt511.qtwebkit -p qt59.qtwebkit

@veprbl veprbl merged commit 4a62576 into NixOS:master Mar 3, 2019
@veprbl veprbl mentioned this pull request Mar 12, 2019
Merged
10 tasks
@veprbl veprbl added the 8.has: port to stable A PR already has a backport to the stable release. label Mar 13, 2019
@veprbl
Copy link
Member Author

veprbl commented Mar 14, 2019

Okay, so qt511.qtwebkit is still broken on Darwin. I tested it with qutebrowser on release-18.09 and phantomjs2 on master and it crashes on start. Can't debug this as I don't have a fully working lldb and macOS refuses to generate core dumps. qt59.qtwebkit appears to work with phantomjs2.

@veprbl veprbl deleted the pr/qt5_qtwebkit_darwin_fix branch December 1, 2020 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttuegel ttuegel Awaiting requested review from ttuegel

@abbradar abbradar Awaiting requested review from abbradar

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: qt/kde 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@veprbl @FRidh @GrahamcOfBorg