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

Qt5py3 Mac fixes #3790

Merged
merged 9 commits into from
Nov 23, 2016
Merged

Qt5py3 Mac fixes #3790

merged 9 commits into from
Nov 23, 2016

Conversation

dakcarto
Copy link
Member

Some Mac-specific fixes (and a couple others) for getting QGIS 3 master branch to compile with Homebrew.

@m-kuhn, if you have some time, please review.

Instead of finding Python interpreter, library and framework on Mac,
rely upon the reported paths of the interpreter (executable) to decipher
whether a framework is being used, then ensure any such framework has
its versioned subdirectory Headers used for includes and the base
Python library used directly in linking. This removes ambiguity in
framework searching, allowing just the PYTHON_EXECUTABLE (user-defined
or from FindPythonInterp module) to control which Python is used.
@@ -78,7 +78,9 @@ QgsIdentifyResultsWebView::QgsIdentifyResultsWebView( QWidget *parent ) : QgsWeb
setSizePolicy( QSizePolicy::MinimumExpanding, QSizePolicy::Minimum );
page()->setNetworkAccessManager( QgsNetworkAccessManager::instance() );
// page()->setLinkDelegationPolicy( QWebPage::DelegateAllLinks );
#ifdef WITH_QTWEBKIT
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is QWebKit not available via homebrew?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

The Homebrew qt5 formula does have the --with-qtwebkit option, but this requires source building of qt5 and probably pyqt5 as well (though the latter does not have a --with-qtwebkit option, so must opportunistically generate the bindings it QWebKit is available). While this is OK for working with general builds, it would take way too long for Travis CI.

Another option might be to host a qt5-webkit formula that builds against an installed qt5, though I have no idea if the CMake and PyQt5 setups will work right with the module built outside of the Qt5 build chain. Worth trying, as it would be the simplest solution.

Yet another option is to do a PR for Homebrew's qt5 formula to include QWebKit as default now with 5.7+ versions.

Copy link
Member

Choose a reason for hiding this comment

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

QtWebKit is no longer in the main Qt repository, so I guess it's fine to build it in a separate formula.
For reference:
https://github.com/opengisch/osgeo4travis/blob/master/docker/qt5/scripts/build-qtwebkit.sh

@m-kuhn
Copy link
Member

m-kuhn commented Nov 21, 2016

Hi Larry, I have just read through all the changes and as far as I can tell they look reasonable to me. Unfortunately it didn't build with my self-created qgis3 formula (looks like C++11 is not enabled), so I'm unable to verify the build process.
I would just merge this as-is and fixup possible problems if they appear.

@dakcarto
Copy link
Member Author

Hi Larry, I have just read through all the changes and as far as I can tell they look reasonable to me. Unfortunately it didn't build with my self-created qgis3 formula (looks like C++11 is not enabled), so I'm unable to verify the build process.
I would just merge this as-is and fixup possible problems if they appear.

OK, I'll merge tonight and immediately work on the qgis3-dev formula, so as to ensure additional commits are timely.

Not sure when the Travis CI setup can be updated, as we need to solve the QWebKit issue, decide on a newer macOS to build on (10.9 no longer has bottles from Homebrew), and create any ancillary bottles needed for full testing. Also, I'm getting quite a few test fails. Need to check into that, or at a minimum allow failed tests for Mac in the meantime.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 21, 2016

FYI, We have failing tests with Qt 5.7 on Linux as well, see #3400 .

Concerning PyQtWebKit, the Qt 4.8 version of these bindings are built as part of QGIS 2.18 on some platforms, you might have a look at that: https://github.com/qgis/QGIS/tree/release-2_18/python/QtWebKit

edit: I don't think we should have that in QGIS 3.0 but this might help to build a separate formula.

@dakcarto dakcarto merged commit d712b49 into qgis:master Nov 23, 2016
@dakcarto
Copy link
Member Author

@m-kuhn, @nyalldawson, I've added a separate formula for QtWebKit/QtWebKitWidgets.

That formula builds OK, but it seems to have an issue with /usr/local/Cellar/qt5/5.7.0/plugins/bearer/libqcorewlanbearer.dylib. If I pull that plugin out, the test passes just fine. I am trying a QGIS master build against this setup. If the problem persists at QGIS runtime, I may just rebuild and bottle via brew install qt5 --with-webkit and see if that helps.

@dakcarto
Copy link
Member Author

Hmm. QtWebKit (by way of separate formula) seems to work fine in QGIS. So, I'll try to complete a qgis3-dev formula ASAP.

@nyalldawson
Copy link
Collaborator

@dakcarto I'm getting:

In file included from /usr/include/qt5/QtCore/qchar.h:43:0,
                 from /usr/include/qt5/QtCore/qstring.h:48,
                 from /usr/include/qt5/QtCore/QString:1,
                 from /usr/include/qt5/QtCrypto/qca_core.h:36,
                 from /usr/include/qt5/QtCrypto/qca.h:36,
                 from /usr/include/qt5/QtCrypto/QtCrypto:1,
                 from qcaossl.cpp:2:
/usr/include/qt5/QtCore/qglobal.h:1113:4: error: #error "You must build your code with position independent code if Qt was built with -reduce-relocations. " "Compile your code with -fPIC (-fPIE is not enough)."
 #  error "You must build your code with position independent code if Qt was built with -reduce-relocations. "\

on master. Any ideas?

@nirvn
Copy link
Contributor

nirvn commented Nov 23, 2016

Same here, can't build master ATM.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 23, 2016

I have partially reverted this for now in 9bb3235 (basically the one line that prevented it from even checking for the qca_ossl plugin)

It's a bit puzzling that -DCMAKE_POSITION_INDEPENDENT_CODE=ON results in -fPIE and not -fPIC. Is this a compiler-dependent result?

@m-kuhn
Copy link
Member

m-kuhn commented Nov 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants