Feature request #1518

python binding does not link with libpython

Added by fundawang - almost 12 years ago. Updated over 11 years ago.

Status:Closed
Priority:Low
Assignee:nobody -
Category:Build/Install
Pull Request or Patch supplied: Resolution:wontfix
Easy fix?:No Copied to github as #:11578

Description

python binding of qgis does not have any direct link references to libpython, which will cause undefined references when linking. Furthermore, the linker does not obby cmake's linker flag.

Please consider merging following patch:
http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/qgis/current/SOURCES/qgis_1.0.0-linkage.patch

python_patch_QGIS_trunk.diff Magnifier - Patch to fix compile problem on Mandriva (in QGIS trunk) (1020 Bytes) Markus Neteler, 2009-08-15 10:45 AM

Associated revisions

Revision dc696527
Added by Jürgen Fischer over 11 years ago

use linker flags when compiling python bindings (fixes #1518)

git-svn-id: http://svn.osgeo.org/qgis/trunk/[email protected] c8812cc2-4d05-0410-92ff-de0c093fc19c

Revision 669b6940
Added by Jürgen Fischer over 11 years ago

use linker flags when compiling python bindings (fixes #1518)

git-svn-id: http://svn.osgeo.org/qgis/[email protected] c8812cc2-4d05-0410-92ff-de0c093fc19c

History

#1 Updated by Martin Dobias almost 12 years ago

Hi,

PyQGIS bindings are generated using makefiles produced by SIP. Looking at output of ldd, niether PyQt4 nor SIP (both using sipconfig) link directly to libpython on my ubuntu box, so I'm in doubt whether linking directly is desired. Do PyQt4 modules link to libpython on mandriva?

Martin

#2 Updated by fundawang - over 11 years ago

well, if it is not directly linked against libpython, then there is no signal that python bindings is using python. i.e., packagers will need to put "Depends: python" into python binding packagers, otherwise end users may leave a qgis-python but without python interpreter.

Linking directly against libpython will generate binary dependencies to python packages.

#3 Updated by Martin Dobias over 11 years ago

I think it's a reasonable requirement for packagers to add "depends: python" statement for the package for qgis python bindings, isn't it? :-)

My concern is that trying to change these linking things might actually break something, so I think we're better of with using default compilation parameters.

#4 Updated by Jean-Roc Morreale over 11 years ago

The proposed patch doesn't seem to be the final solution for Mandriva 2009.1 as even with qgis-python & others python deps installed, the python plugin manager doesn't show up. I've users who reports the same behaviour with ubuntu intrepid.

#5 Updated by Markus Neteler over 11 years ago

I am Mandriva 2009.1 user, the patch helps to compile QGIS (Version 1.0.x svn) and the Python plugins work.

#6 Updated by Jean-Roc Morreale over 11 years ago

Thank for the info Markus, I'll relay it to Buchan Milne (mdv's maintener for qgis) so we can ship a correct backport to 2009 and 2009.1

#7 Updated by Markus Neteler over 11 years ago

jrm: FYI, the original poster (Funda Wang) is now the maintainer: e.g. http://sophie.zarb.org/srpm/cooker,ia64/qgis

#8 Updated by Jürgen Fischer over 11 years ago

Replying to [comment:3 wonder]:

I think it's a reasonable requirement for packagers to add "depends: python" statement for the package for qgis python bindings, isn't it? :-)

My concern is that trying to change these linking things might actually break something, so I think we're better of with using default compilation parameters.

This patch breaks compiling on Windows - and I still don't see why the python bindings need to be linked with the python libraries - as they obviously don't need them.

What's the problem with adding a dependency to the mandriva package manually?

#9 Updated by fundawang - over 11 years ago

The "Depends: python" is a packaging issue. Without the patch, qgis even won't built with LDFLAGS="-Wl,--as-needed -Wl,--no-undefined".

#10 Updated by Giovanni Manghi over 11 years ago

Hi,

what is the status of this patch now we are closing in to feature freeze for qgis 1.2?

#11 Updated by Markus Neteler over 11 years ago

Did you try to compile with LDFLAGS="-Wl,--as-needed -Wl,--no-undefined" on an e.g. Ubuntu box?

#12 Updated by Giovanni Manghi over 11 years ago

Replying to [comment:11 neteler]:

Did you try to compile with LDFLAGS="-Wl,--as-needed -Wl,--no-undefined" on an e.g. Ubuntu box?

Hi,

yes, it compiles fine under Ubuntu 9.04

#13 Updated by Jürgen Fischer over 11 years ago

Replying to [comment:12 lutra]:

Replying to [comment:11 neteler]:

Did you try to compile with LDFLAGS="-Wl,--as-needed -Wl,--no-undefined" on an e.g. Ubuntu box?

Hi,

yes, it compiles fine under Ubuntu 9.04

And Debian unstable. And the resulting library is already linked with python:

$ ldd /usr/lib/libqgispython.so|grep pyth
    libpython2.5.so.1.0 => /usr/lib/libpython2.5.so.1.0 (0x00007f79d8fa3000)

Maybe this was fixed in PyQt4 or SIP - it would make sense to fix it there.

I'm using SIP 4.8.1 and PyQt4 4.5.1.

#14 Updated by Markus Neteler over 11 years ago

On Mandriva 2009.1, I have python-sip-4.7.9-3, python-qt4-4.4.4-6 and qt4-common-4.5.2-1.3. Your version seems to be more recent (but Debian unstable).

The resulting library is also here linked with python:

ldd /home/local/lib/qgis/lib/libqgispython.so |grep pyth
        libpython2.6.so.1.0 => /usr/lib64/libpython2.6.so.1.0 (0x00007f19e0f18000)

Not sure what to do.

#15 Updated by Jürgen Fischer over 11 years ago

Replying to [comment:14 neteler]:

On Mandriva 2009.1, I have python-sip-4.7.9-3, python-qt4-4.4.4-6 and qt4-common-4.5.2-1.3. Your version seems to be more recent (but Debian unstable).

The resulting library is also here linked with python:

> ldd /home/local/lib/qgis/lib/libqgispython.so |grep pyth
>         libpython2.6.so.1.0 => /usr/lib64/libpython2.6.so.1.0 (0x00007f19e0f18000)

Not sure what to do.

With or without the patch? Mine is without the patch. And I thought the patch's intent was to get that explicit link - which is there now. I guess the patch is obsolete now.

And this might have been a bug in PyQt4 (or probably even SIP) in the first place as that link is probably as useful for every other python binding as it is for qgis' and should have been - and probably was in the meantime - added there.

I'd opt for leaving things as the are in qgis and consider this a problem with older systems and declare it a packaging problem.

#16 Updated by Jean-Roc Morreale over 11 years ago

Hi, I did a try against mandriva's cooker (python-qt4-4.5.2, python-sip-4.8.2, qt4-common-4.5.2), qgis version 1.0 svn still need the patch to be built without undefined references and svn trunk is rejecting this part (python/configure.py.in.rej) :

  1. common settings for both core and gui libs
    for mk in [ makefile_core, makefile_gui ]:
    + mk.extra_lflags = [[CMAKE_MODULE_LINKER_FLAGS]]
    mk.extra_libs = qgis_core
    if geos_library!="":
    mk.extra_libs.append(geos_library)

#17 Updated by Jürgen Fischer over 11 years ago

Replying to [comment:16 jrm]:

Hi, I did a try against mandriva's cooker (python-qt4-4.5.2, python-sip-4.8.2, qt4-common-4.5.2), qgis version 1.0 svn still need the patch to be built without undefined references

Can you quote the error message you get?

#18 Updated by Jean-Roc Morreale over 11 years ago

$ rpmbuild -ba qgis_trunk.spec

Exécution_de(%prep): /bin/sh -e /home/jrm/rpm/tmp/rpm-tmp.z46fV4

+ umask 022

+ cd /home/jrm/rpm/BUILD

+ '[' 1 -eq 1 ']'

+ '[' 1 -eq 1 ']'

+ '[' 1 -eq 1 ']'

+ cd /home/jrm/rpm/BUILD

+ rm -rf qgis_1.2

+ /usr/bin/gzip -dc /home/jrm/rpm/SOURCES/qgis_1.2.tar.gz

+ /bin/tar -xf -

+ STATUS=0

+ '[' 0 -ne 0 ']'

+ cd qgis_1.2

+ echo 'Patch #3906 (qgis_1.0.0-linkage.patch):'

Patch #3906 (qgis_1.0.0-linkage.patch):

+ /usr/bin/patch -U -s -p0 -b --suffix .link --fuzz=0 -i /home/jrm/rpm/SOURCES
/qgis_1.0.0-linkage.patch

1 out of 2 hunks FAILED -- saving rejects to file python/configure.py.in.rej

#19 Updated by Jürgen Fischer over 11 years ago

  • Status changed from Open to Closed
  • Resolution set to fixed

Replying to [comment:18 jrm]:

+ echo 'Patch #3906 (qgis_1.0.0-linkage.patch):'

Patch #3906 (qgis_1.0.0-linkage.patch):

+ /usr/bin/patch -U -s -p0 -b --suffix .link --fuzz=0 -i /home/jrm/rpm/SOURCES

/qgis_1.0.0-linkage.patch

1 out of 2 hunks FAILED -- saving rejects to file python/configure.py.in.rej

<pre>

<pre>
# This made an appearence in Qt v4.4rc1 and breaks extension modules so
# remove it.  It was removed at my request but some stupid distros may
# have kept it.
self.LFLAGS.remove('-Wl,--no-undefined')
</pre>

<pre>

commit:669b6940 (SVN r11280) now passes on the linker flags in splitted form and there are no linker errors.

#20 Updated by Jean-Roc Morreale over 11 years ago

Sorry for the mistake, I did a try without the patch and with rev 11281 but still get the undefined references. I tryed to save cmake's output with '| tee errors.log' but it doesn't save the errors' lines so there is a part of my terminal's log : http://dl.free.fr/tAGYVq6e2

#21 Updated by Markus Neteler over 11 years ago

I have recompiled the current unpatched trunk and it now compiles on Mandriva 2009.1.

#22 Updated by fundawang - over 11 years ago

Replying to [comment:21 neteler]:

I have recompiled the current unpatched trunk and it now compiles on Mandriva 2009.1.

I think you should use %cmake macro when compiling, as it will produce correct compile flags, especially LDFLAGS.

#23 Updated by fundawang - over 11 years ago

  • Status changed from Closed to Feedback
  • Resolution deleted (fixed)

Patch against qgis 1.1 (I couldn't find any information on how to checkout svn tarball)

--- src/core/CMakeLists.txt~    2009-04-14 05:03:52.000000000 -0400
+++ src/core/CMakeLists.txt     2009-08-06 05:51:15.000000000 -0400
@@ -272,7 +272,7 @@
     TARGET_LINK_LIBRARIES(qgis_core ${ICONV_LIBRARY})
   ENDIF (WIN32 OR APPLE)
   IF (UNIX)
-    TARGET_LINK_LIBRARIES(qgis_core pthread)
+    TARGET_LINK_LIBRARIES(qgis_core pthread dl)
   ENDIF (UNIX)
 ELSE (WITH_INTERNAL_SPATIALITE)
   TARGET_LINK_LIBRARIES(qgis_core ${SQLITE3_LIBRARY})
--- python/qgisconfig.py.in~    2007-01-08 21:39:15.000000000 -0500
+++ python/qgisconfig.py.in     2009-08-06 05:57:33.000000000 -0400
@@ -1,4 +1,5 @@
 import [[PyQt]]4.pyqtconfig
+import sys

 # These are installation specific values created when QGIS was configured.
 # The following line will be replaced when this template is used to create
@@ -34,6 +35,7 @@
         # Make sure our C++ library is linked.
         self.extra_libs.append("qgis_core")
         self.extra_libs.append("qgis_gui")
+       self.extra_libs.append("python"+sys.version[0:3])

         # Let the super-class do what it needs to.
         pyqtconfig.QtModuleMakefile.finalise(self)
--- python/configure.py.in~     2009-02-25 14:15:23.000000000 -0500
+++ python/configure.py.in      2009-08-06 05:58:43.000000000 -0400
@@ -126,6 +126,7 @@

 # common settings for both core and gui libs
 for mk in [ makefile_core, makefile_gui ]:
+  mk.extra_lflags = [[@[email protected]]]
   mk.extra_libs = [[qgis_core]]
   if geos_library!="":
     mk.extra_libs.append(geos_library)
@@ -144,7 +145,8 @@
                            build_path, # qgsconfig.h, qgssvnversion.h
                            gdal_inc_dir,
                            geos_inc_dir]
-  mk.extra_cxxflags = [[-DCORE_EXPORT=]]
+  mk.extra_cxxflags = [[-DCORE_EXPORT="+export "@[email protected]]]
+  mk.extra_libs.append("python"+sys.version[0:3])

 # more settings for gui lib
 makefile_gui.extra_libs.append("qgis_gui")

if you wont' accept the patch, and think it is a packaging problem, just close this as won't fix, but not fixed, please.

#24 Updated by Giovanni Manghi over 11 years ago

  • Resolution set to wontfix
  • Status changed from Feedback to Closed

#25 Updated by Jürgen Fischer over 11 years ago

I'm revisiting this, because Markus seems to have hit this Mandriva specific problem again.

Replying to [comment:23 fundawang]:

Patch against qgis 1.1 (I couldn't find any information on how to checkout svn tarball)

1.1 isn't part of the unstable branch and there not maintained.

This patch still breaks the Windows build. I didn't find a portable way to get the name of the python library (ie. python25 on Windows, python2.5 and something else on MacOS). And I dislike hardcoding that without real need.

Python extensions are only useful when loaded into python and there the python library already exists. So there's no real reason why extension must be linked explicitly with the python library. So the usual case is, that the library isn't linked. And that's probably also why sipconfig filters out link options that try to make this trigger an error - because that would break each an every extension using sipconfig. See my comment above.

What I understand is that this could mask problems when extensions are not only not linked to the python library, but also not linked to other libraries they actually use and that are not always loaded into python (OTOH python might be able to cope with this - dunno).

Anyway, the applied change splits the link arguments up, so that the filter can actually remove the offending option. In that way this bug is actually fixed: The build doesn't break, if you pass pedantic link flags.

If this doesn't help on Mandriva, I suspect that the filter was removed there from sipconfig, but in that case I don't understand why sipconfig wasn't also modified to link with the python library by default. The Makefile class actually has a python parameter to do just that. Unfortunately it cannot be used through ModuleMakefile. But a modification to allow that and/or make that default should not have been difficult.

#26 Updated by William Kyngesburye over 11 years ago

Just a note from the OSX side. Python modules on OSX shouldn't link the Python library directly, since they are loaded by Python, the Python library is expected to be available to modules. Extra libraries directly used by the module DO need to be linked.

Linking flags are taken care of in both python distutils and SIP: no libpython, but including -bundle -undefined dynamic_lookup flags (bundle is an OSX special loadable module type of library, dynamic_lookup finds the python library symbols at runtime).

#27 Updated by Markus Neteler over 11 years ago

Hi, while I am unable to follow the technical details of this report I have attached a patch to fix compile problem on Mandriva (in QGIS trunk) based on the original patch for QGIS 1.0. If the patch os good or bad I cannot say, at least I can compile QGIS trunk.

Also available in: Atom PDF