Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#1518 closed patch (wontfix)

python binding does not link with libpython

Reported by: fundawang Owned by: nobody
Priority: minor: annoyance Milestone: Version 1.0.3
Component: Build/Install Version: 1.0.0
Keywords: Cc: neteler
Must Fix for Release: No Platform: All
Platform Version: Awaiting user input: no

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

Attachments (1)

python_patch_QGIS_trunk.diff (1020 bytes ) - added by neteler 15 years ago.
Patch to fix compile problem on Mandriva (in QGIS trunk)

Download all attachments as: .zip

Change History (28)

comment:1 by wonder, 15 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

comment:2 by fundawang, 15 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.

comment:3 by wonder, 15 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.

comment:4 by jrm, 15 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.

comment:5 by neteler, 15 years ago

Cc: neteler added

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

in reply to:  5 comment:6 by jrm, 15 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

comment:7 by neteler, 15 years ago

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

in reply to:  3 comment:8 by jef, 15 years ago

Replying to 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?

comment:9 by fundawang, 15 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".

comment:10 by lutra, 15 years ago

Hi,

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

comment:11 by neteler, 15 years ago

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

in reply to:  11 ; comment:12 by lutra, 15 years ago

Replying to 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

in reply to:  12 comment:13 by jef, 15 years ago

Replying to lutra:

Replying to 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.

comment:14 by neteler, 15 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.

in reply to:  14 comment:15 by jef, 15 years ago

Replying to 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.

comment:16 by jrm, 15 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) :

# 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)

in reply to:  16 comment:17 by jef, 15 years ago

Replying to 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?

comment:18 by jrm, 15 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 #2 (qgis_1.0.0-linkage.patch):'
Patch #2 (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

in reply to:  18 comment:19 by jef, 15 years ago

Resolution: fixed
Status: newclosed

Replying to jrm:

+ echo 'Patch #2 (qgis_1.0.0-linkage.patch):'
Patch #2 (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

sorry, I meant the link problems you were experiencing before. That error message just tells us that the patch doesn't apply cleanly to trunk anymore because configure.py.in was changed in the meantime.

Looking for a portable way to find the name of the python library I came across following comment in sipconfig:

# 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')

Seems like sipconfig which is used to create the Makefiles for the python bindings tries to explictly filter out the link option that causes the link errors. It probably just fails to because extra_lflags are not split in the patch and therefore there is no such element.

r11279 now passes on the linker flags in splitted form and there are no linker errors.

comment:20 by jrm, 15 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

comment:21 by neteler, 15 years ago

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

in reply to:  21 comment:22 by fundawang, 15 years ago

Replying to 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.

comment:23 by fundawang, 15 years ago

Resolution: fixed
Status: closedreopened

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 PyQt4.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 = ["@CMAKE_MODULE_LINKER_FLAGS@"]
   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="+export]
+  mk.extra_cxxflags = ["-DCORE_EXPORT="+export, "@CMAKE_CXX_FLAGS@"]
+  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.

comment:24 by lutra, 15 years ago

Resolution: wontfix
Status: reopenedclosed

in reply to:  23 comment:25 by jef, 15 years ago

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

Replying to 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.

comment:26 by kyngchaos, 15 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).

by neteler, 15 years ago

Patch to fix compile problem on Mandriva (in QGIS trunk)

comment:27 by neteler, 15 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.

Note: See TracTickets for help on using tickets.