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

[cmake] Discover PyQt4 built with its new build system #1508

Closed
wants to merge 1 commit into from

Conversation

Ariki
Copy link

@Ariki Ariki commented Jul 16, 2014

Addresses bug reports #10596, #8857. The module pyqtconfig isn't available in PyQt4 built with configure-ng.py instead of configure.py, so changes were made in cmake/FindPyQt.py to support both.
Now can build without issues on Arch Linux x86_64.
Related links:

else:
import sipconfig
sipcfg = sipconfig.Configuration()
print("pyqt_mod_dir:%s" % sipcfg.default_mod_dir)
Copy link
Member

Choose a reason for hiding this comment

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

This should have the PyQt4 sub-directory appended.

@dakcarto
Copy link
Member

I don't think the following values are always equivalent when substituting sipconfig module for pyqtconfig:

sipcfg.default_mod_dir  for  pyqtcfg.pyqt_mod_dir  
sipcfg.default_sip_dir  for  pyqtcfg.pyqt_sip_dir
sipcfg.default_bin_dir  for  pyqtcfg.pyqt_bin_dir

The sipconfig module basically knows nothing of how PyQt was installed, excepting the sip flags.

BTW: you can still use configure.py (just don't do build), then configure again with configure-ng.py and build/install. This gives you the pyqtconfig module, while still allowing to use the new build system. See how this is done on Mac with the Homebrew project's pyqt formula.

@Ariki
Copy link
Author

Ariki commented Jul 17, 2014

At least on ArchLinux, all sip file directories such as QtCore, QtGui, etc. are now installed in /usr/share/sip/ instead of /usr/share/sip/PyQt4 (where configure.py installs them).

But anyway, my solution isn't good even for ArchLinux. As PyQt documentation says,

    SIP v5 will not include an extensible build system, i.e it will not provide
an equivalent of SIP v4’s sipconfig module. Consequently a version of PyQt
built with SIP v5 will not provide an equivalent of the pyqtconfig module.

@Ariki
Copy link
Author

Ariki commented Jul 17, 2014

In general, there exist ways to determine PyQt4 module path and the paths to Qt binaries and libraries. But there seems to be no way to reliably determine path to sip files if PyQt4 was built with non-default configuration options which is the case for Arch Linux. pyqtconfig-ng.py doesn't seem to store it anywhere.
And now I see there is a way to specify PYQT4_SIP_DIR in QGIS cmake configuration options (haven't tried yet), In that case there's no problem at all.

@leyan
Copy link
Contributor

leyan commented Jan 31, 2015

What is the reason why this pull request is not accepted ? It is still needed to compile QGis on ArchLinux.

Additionally, something similar is needed in https://github.com/qgis/QGIS/blob/master/python/console/console.py to replace the call to pyqtconfig.Configuration().qt_version by the QT_VERSION from PyQt4.QtCore

@Ariki
Copy link
Author

Ariki commented Jan 31, 2015

I must admit I completely forgot about this issue. I'l try to dive into it again.
My pull request is rather an evil one because it's based on incorrect assumptions. There already exist updated variants of FindPyQt.py which are more universal than mine. But I'm afraid they won't work on ArchLinux because the issue there seems to be not in QGIS build system but in misconfiguration of python2-pyqt4 package in ArchLinux. It installs its files in /usr/share/sip directory rather than in /usr/share/sip/PyQt4 where they used to be installed before transition to configure-ng. Just to compare, python2-pyqt5 installs its files to /usr/share/sip/PyQt5, so it's likely a bug. I'll try to investigate more and then provide the solution.

@okanisis
Copy link

okanisis commented Feb 3, 2015

It looks like they could be in the proper folders now, at least on my Arch box,

% ls -lah /usr/share/sip                                                                                                               =100%
total 20K
drwxr-xr-x   5 root root 4.0K Feb  3 13:37 .
drwxr-xr-x 226 root root 4.0K Jan 28 14:02 ..
drwxr-xr-x  21 root root 4.0K Feb  3 11:16 Py2-PyQt4
drwxr-xr-x  34 root root 4.0K Jan 12 08:06 Py2-PyQt5
drwxr-xr-x  22 root root 4.0K Jul 17  2014 PyQt4

There was a recent update to python2-pyqt4 (4.11.3-2) today and now there are two folders for it, though in my system there's been a PyQt4 folder since Jul 17 2014 and a new folder Py2-PyQt4 added with the latest update.

I tried building the QGIS 2.6 release branch using the PYQT4_SIP_DIR flag but it still needed this patch for some reason. Unless there are another set of options that need to go with the PYQT4_SIP_DIR flag...

@Ariki
Copy link
Author

Ariki commented Feb 4, 2015

Yes, PyQt4 SIP directory has been changed in Arch as a response to my bug report, but now it is Py2-PyQt4 rather than just PyQt4 (due to possible conflicts with Python 3 package). I suppose PyQt4 directory was created by old versions of the package (which used configure.py). I was out of home these days so haven't tried to build QGIS myself yet.

@okanisis
Copy link

okanisis commented Feb 5, 2015

It looks like some packages like qscintilla are using the old PyQt4/Qsci directory, so I opened a bug for qscintilla in Arch Linux to see if they can go into the new Py2-PyQt4 dir instead. Can't figure out how to build with qscintilla sip files in the old dir.

I've tried the cmake options -DPYQT4_SIP_DIR=/usr/share/sip/Py2-PyQt4 and -DQSCI_SIP_DIR=/usr/share/sip/PyQt4/Qsci but current stable release of QGIS 2.6.1 is failing to build. But I figure this is a separate issue. Asking on IRC if anyone has any insight.

@Ariki
Copy link
Author

Ariki commented Feb 5, 2015

It seems currently there is no option in QGIS to specify a directory for Qsci sip files separately. It can be added but I hope ArchLinux maintainers will make the packages more consistent.
I've managed to build QGIS without QScintilla using -DPYQT4_SIP_DIR=/usr/share/sip/Py2-PyQt4 and -DPYQT4_SIP_FLAGS="-x VendorID -t WS_X11 -t Qt_4_8_6 -x Py_v2". I'll try to modify FindPyQt script so as to eliminate the need for the latter option at least.

@Ariki
Copy link
Author

Ariki commented Feb 8, 2015

Finally in works both on Arch Linux and Ubuntu but needs to be tested on other platforms.
On Arch, -DQSCI_SIP_DIR=/usr/share/sip/PyQt4 should be used because QSci sip files are located outside PyQt sip directory (which is /usr/share/sip/Py2-PyQt4).
PyQt sip file directory is being searched among several popular locations inside default SIP directory returned by sipconfig. It differs from the approach adopted by QScintilla and KDE projects where /usr/share/sip/PyQt4 is the only default location and should be set explicitly if different directory is being used. I'm not sure which approach is better.
console.py is also fixed so as to work with the new PyQt4 build system as well as the old one.

@dakcarto dakcarto removed their assignment Oct 1, 2015
if os.path.exists(pyqt_sip_dir) and os.path.exists(
os.path.join(pyqt_sip_dir, 'QtCore')):
return pyqt_sip_dir
return default_sip_dir
Copy link
Member

Choose a reason for hiding this comment

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

This works on Mac, when using Homebrew for dependencies

@Ariki
Copy link
Author

Ariki commented Oct 2, 2015

It could be done if I knew utility name to search for. For example, my Linux system has no pyuic4 file in /usr/bin, it's named python2-pyuic4 (but if I had PyQt4 for Python 3 installed, the file pyuic4 would exist and point to Python 3 script).

The only place I found in QGIS repository that uses PYQT4_BIN_DIR is in Mac build system, and there's supposed the utility is named pyuic4.

May be it would be better not trying to guess non-default locations (the directory Py2-PyQt4 in the code above is of the same sort) but instead provide a sensible default and consider it user's responsibility to provide appropriate options for CMake in other cases?

@m-kuhn
Copy link
Member

m-kuhn commented Nov 19, 2015

The referenced issues #10596, #8857 are closed, so I guess this PR is obsolete?

Nevertheless, thank you for contributing!
If there is still an issue, please leave a note.

@m-kuhn m-kuhn closed this Nov 19, 2015
@okanisis
Copy link

It looks like some changes to python/console/console.py might be needed even though QGIS will build now that cmake/FindPyQt.py is updated.

There's still a pyqtconfig import error when trying to open the python console inside QGIS 2.8 and 2.12 and master (actually, master doesn't build on arch linux because of https://hub.qgis.org/issues/13686).

It looks like the 2 proposed changes to console.py in this pull get it working again.

@Scimmia22
Copy link
Contributor

Yes, console.py still needs fixed, but the fix in this PR isn't correct. PyQt4.QtCore.QT_VERSION isn't the same thing as PyQt4.pyqtconfig.Configuration().qt_version. The values are competely different.

I think the old build system includes PyQt4.QtCore.QT_VERSION_STR; if so, switching over to that instead of using pyqtconfig would solve this.

@okanisis
Copy link

It looks like a workaround was attempted with,

QT_VERSION = pyqtconfig.Configuration().qt_version

so that QT_VERSION is manually defined for the old pyqt4 and called later on (but you're saying this doesn't work if the user has an older version of pyqt4 installed?),

if QT_VERSION >= 0x40700:

Do you think that the old pyqt4 import should be what you've suggested?, PyQt4.QtCore import QT_VERSION_STR

And then do some kind of if/else to try/except test later on around line 472 depending on whether QT_VERSION or QT_VERSION_STR should be used?

PS: I'm a newb with python so learning stuff as I go along here =)

@okanisis
Copy link

QT_VERSION_STR exists in pyqt4 v4.11.4 and outputs "4.8.7" whereas QT_VERSION outputs "264199"

Both of these variables exist in pyqt4 v4.10.2 (tested on Windows 8 through the OSGeo4W installer) too, running against qt4 v4.8.5.

@Ariki
Copy link
Author

Ariki commented Nov 24, 2015

Scimmia22, what do you mean by "The values are completely different"? I cannot test with the old build system right now but it seemed to work. The documentation says qt_version is what I supposed it to be (an integer number which reflects Qt version when represented as a hexadecimal).

@Scimmia22
Copy link
Contributor

Ariki, yes, that's what PyQt4.pyqtconfig.Configuration().qt_version is, but if you look at sautdon's output, that's not what PyQt4.QtCore.QT_VERSION is.

Current workaround I'm using for Arch is:

  sed -e '/from PyQt4.QtCore/ s/$/, QT_VERSION_STR/' \
      -e '/import pyqtconfig/d' \
      -e 's/pyqtconfig.*qt_version/QT_VERSION_STR/' \
      -e 's/0x40700/"4.7.0"/' \
      -i python/console/console.py

I think this will work when using configure.py, but I haven't tried it.

@Scimmia22
Copy link
Contributor

OK, PyQt4.QtCore.QT_VERSION is being printed in decimal instead of hex. I'm no python expert, will the comparison take care of that? If so, this may be a non-issue and the patch would work

Is PyQt4.QtCore.QT_VERSION not present when using configure.py? If it is, it would be simpler to just switch to that completely instead of using try/except. Looking at the changelog, QT_VERSION was added in 12/2005, I'm assuming it would be safe to assume it's present?

@m-kuhn
Copy link
Member

m-kuhn commented Nov 25, 2015

I had tried to switch these two (among other changes to deduplicate the currently redundant code for pyqt4 and pyqt5) but had to revert it here because of troubles on some systems:

eeef84b

You could check if this works for your system and what needs to be done to get it pass on travis (and hope that it also will solve the issues on other (osx?) systems)

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

6 participants