Opened 12 years ago

Closed 11 years ago

#58 closed defect (fixed)

wx version of v.digit relies upon non-portable hacks

Reported by: marisn Owned by: martinl
Priority: critical Milestone: 6.4.0
Component: Default Version: 6.3.0 RCs
Keywords: wxGUI, v.digit Cc: grass-dev@…
CPU: Unspecified Platform: Unspecified

Description

Installing GRASS on Gentoo fails during compilation altough configure stage passes fine.

OS: 32bit Gentoo ~x86 GRASS: 6.3.0RC5 GCC: 4.2.2 (Gentoo 4.2.2 p1.0) wxWidgets: 2.8.7.1

c++ -shared -fpic -L/var/tmp/portage/sci-geosciences/grass-6.3.0_rc5/work/grass-6.3.0RC5/dist.i686-pc-linux-gnu/lib -lgrass_vect -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_dbmiclient -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_dgl -lgrass_dig2 -lgrass_gis -lgrass_datetime -lz -lgrass_rtree -lgrass_gis -lgrass_datetime -lz -lgrass_linkm -lgrass_rtree -lgrass_dig2 -lgrass_gis -lgrass_datetime -lz -lgrass_rtree -lgrass_dgl -lgrass_rtree -lgrass_linkm -lgrass_dbmiclient -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -L/usr/lib -lgdal -lgrass_gis -lgrass_datetime -lz -L/usr/lib -lgdal -lgrass_vedit -lgrass_gis -lgrass_datetime -lz -lgrass_vect -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_dbmiclient -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_dgl -lgrass_dig2 -lgrass_gis -lgrass_datetime -lz -lgrass_rtree -lgrass_gis -lgrass_datetime -lz -lgrass_linkm -lgrass_rtree -pthread -L/usr/X11R6/lib -lwx_gtk2u_richtext-2.8 -lwx_gtk2u_aui-2.8 -lwx_gtk2u_xrc-2.8 -lwx_gtk2u_qa-2.8 -lwx_gtk2u_html-2.8 -lwx_gtk2u_adv-2.8 -lwx_gtk2u_core-2.8 -lwx_baseu_xml-2.8 -lwx_baseu_net-2.8 -lwx_baseu-2.8 -lgdi OBJ.i686-pc-linux-gnu/driver.o OBJ.i686-pc-linux-gnu/digit.o OBJ.i686-pc-linux-gnu/cats.o OBJ.i686-pc-linux-gnu/line.o OBJ.i686-pc-linux-gnu/vertex.o OBJ.i686-pc-linux-gnu/select.o OBJ.i686-pc-linux-gnu/grass6_wxvdigit_wrap.o -o _grass6_wxvdigit.so /usr/lib/gcc/i686-pc-linux-gnu/4.2.2/../../../../i686-pc-linux-gnu/bin/ld: cannot find -lgdi collect2: ld returned 1 exit status make: * [grass6_wxvdigit.so] Error 1

Change History (19)

comment:1 Changed 12 years ago by martinl

Cc: grass-dev@… added
Keywords: wxGUI added
Owner: changed from grass-dev@… to martinl
Priority: blockercritical
Status: newassigned

This is known issue, since digitizer uses wxPseudoDC class which is not part of wxWidgets but only wxPython related, you need to create manually link to Python extension _gdi_.so. See source:grass/trunk/gui/wxpython/README.

Martin

comment:2 Changed 12 years ago by marisn

Solution provided by README is not an option - installation procedure CAN NOT modify directly stuff outside of sandbox. I can create symlinks within GRASS source ($WORKDIR), but not in /usr/*

comment:3 Changed 12 years ago by kyngchaos

Same problem here on OSX. At least on OSX I can directly link in the _gdi_.so binary with a full path. Or you can create the symlink in some place you have permissions to write. BUT, this is still not a good option - directly linking binaries to an internal wxpython library just doesn't sound right. Aren't Python binary extensions something that are normally loaded by python extension code?

comment:4 in reply to:  3 Changed 12 years ago by martinl

Replying to kyngchaos:

Same problem here on OSX. At least on OSX I can directly link in the _gdi_.so binary with a full path. Or you can create the symlink in some place you have permissions to write. BUT, this is still not a good option - directly linking binaries to an internal wxpython library just doesn't sound right. Aren't Python binary extensions something that are normally loaded by python extension code?

Right, this solution is unacceptable, for now just a temporal one. I am trying to find better solution, to avoid replacing wxPseudoDC by wxDC in C++ code since in wxPython code are used methods of PseudoDC (e.g. TranslateId() for moving selected objects) which are not available for wxDC. Suggestions/ideas welcomed. Martin

comment:5 Changed 12 years ago by kyngchaos

Well, looks like it's a no-go for OSX. I think it's because on OSX there is a distinction between dynamic libraries and loadable modules. Dynamic libraries are loaded by ld and linked during the build (I think some call this statically linked), loadable modules are loaded by programs (and other libraries) at runtime and are not linked (or as some would say, dynamically linked). Dynamic libraries can also be loaded as modules.

Extensions to libraries and programs, such as Python, PHP, Apache, ..., are built as modules on OSX. So, _gdi_.so can't be linked as a library, and you get missing symbols errors.

comment:6 in reply to:  5 Changed 12 years ago by glynn

Summary: v.digit fails to build on Gentoov.digit relies upon non-portable hacks

Replying to kyngchaos:

Extensions to libraries and programs, such as Python, PHP, Apache, ..., are built as modules on OSX. So, _gdi_.so can't be linked as a library, and you get missing symbols errors.

In that case, use of wxPseudoDC must be eliminated from the vdigit module.

comment:7 Changed 12 years ago by kyngchaos

I got it to build and run on OSX. The key is that _gdi_.so is loaded by python when wx is imported, so there really is no need to directly link it into _grass6_wxvdigit.so. To get around the missing symbols then, on OSX use the "-undefined dynamic_lookup" flag (note: OSX 10.3 and above only), instead of the "-lgdi" hack.

So, replace "-lgdi" with a configured variable in Platform.make. If there is an equivalent dynamic_lookup flag for other platforms, that should be used instead of directly linking _gdi_.so.

See ticket #61 for other notes. (should these tickets be merged?)

comment:8 Changed 12 years ago by hamish

Summary: v.digit relies upon non-portable hackswx version of v.digit relies upon non-portable hacks

comment:9 Changed 12 years ago by martinl

Milestone: 6.3.06.4.0

comment:10 Changed 11 years ago by kyngchaos

I've been getting a lot of questions about compiling vdigit on OSX, so, until a better make setup is worked out, I added a conditional in the makefile to apply my OSX-specific EXTRA_LIBS and SHLIB_LD as needed. I also changed the vdigit extension to .so because that's what is used for python extensions on OSX, even though the system library exension is dylib (many other packages also do this on OSX, like Apache and PHP).

comment:11 Changed 11 years ago by martinl

CPU: Unspecified
Platform: Unspecified

I added local copies of pseudodc.cpp and pseudodc.h to trunk (r35446). Please let me know if it works for you. Then I will backport it to all active branches. Anyway it's just workaround how to avoid libgdi link. Should be solved in the future in better way...

Martin

comment:12 Changed 11 years ago by kyngchaos

Those are definitely not needed for OSX, so they should be conditionally compiled, even if this is a temporary solution. The current method of "-bundle -undefined dynamic_lookup" for OSX is the proper way to handle python extensions in general, which coincidentally takes care of the GDI problem.

So, a wildcard couldn't be used for SOURCES. Something like:

SOURCES := cats.cpp digit.cpp driver_draw.cpp driver.cpp \
	line.cpp message.cpp select.cpp undo.cpp vertex.cpp \
	$(LIB_NAME)_wrap.cpp
ifneq ($(findstring darwin,$(ARCH)),darwin)
SOURCES = pseudodc.cpp $(SOURCES)
endif

comment:13 in reply to:  12 Changed 11 years ago by martinl

Replying to kyngchaos:

Those are definitely not needed for OSX, so they should be conditionally compiled, even if this is a temporary solution. The current method of "-bundle -undefined dynamic_lookup" for OSX is the proper way to handle python extensions in general, which coincidentally takes care of the GDI problem.

That's right, but compiling against local copy of pseudodc should work also on Mac and should be harmless. Just to avoid other platform dependent conditionalization.

comment:14 in reply to:  11 ; Changed 11 years ago by glynn

Replying to martinl:

I added local copies of pseudodc.cpp and pseudodc.h to trunk (r35446). Please let me know if it works for you.

It certainly won't work in general, as the Python code (toolbar.py) is creating a wx.PseduoDC, not the local version. I daresay that it will work if the local copies happen to exactly match the installed version of wxPython, but not otherwise.

IOW, any local copy needs to be SWIG'd and used from Python in place of wx.PseudoDC.

I was looking into this, and got it mostly working, except for the Python-ised version of GetIdBounds?(). For some reason, SWIG insists upon returning an opaque wrapper for wxRect* rather than a wxRect proxy.

If you can tolerate modifying the code to pass in a new wx.Rect rather than expecting GetIdBounds? to create and return one, then it's basically working.

comment:15 in reply to:  14 Changed 11 years ago by glynn

Replying to glynn:

It certainly won't work in general

For this reason, I have reverted it.

comment:16 in reply to:  14 ; Changed 11 years ago by glynn

Replying to glynn:

It certainly won't work in general, as the Python code (toolbar.py) is creating a wx.PseduoDC, not the local version. I daresay that it will work if the local copies happen to exactly match the installed version of wxPython, but not otherwise.

IOW, any local copy needs to be SWIG'd and used from Python in place of wx.PseudoDC.

I've committed this in r35537.

Drawbacks and caveats:

  1. I only SWIG'd the methods which were actually being called.
  1. I had to bypass the type-checking for wxDC*. The SWIG'd function is a wrapper which takes a void* instead of a wxDC*, casts it, and passes that to DrawToDC[Clipped]. If you pass some other pointer, you'll get a segfault rather than a Python exception. The alternative was to require that wxPython's .i files were installed, and figure out how to use them (without them, SWIG doesn't know how to cast e.g. wxClientDC* to wxDC*).
  1. DrawToDCClipped() requires a wx.Rect; it won't accept a tuple or list.

comment:17 in reply to:  16 ; Changed 11 years ago by martinl

Replying to glynn:

I've committed this in r35537.

Thanks Glynn!, any objections to backport it to devbr6 and relbr64?

comment:18 in reply to:  17 ; Changed 11 years ago by glynn

Replying to martinl:

I've committed this in r35537.

Thanks Glynn!, any objections to backport it to devbr6 and relbr64?

I can't think of any reason not to.

comment:19 in reply to:  18 Changed 11 years ago by martinl

Resolution: fixed
Status: assignedclosed

Replying to glynn:

Replying to martinl:

I've committed this in r35537.

Thanks Glynn!, any objections to backport it to devbr6 and relbr64?

I can't think of any reason not to.

Done in r35554 and r35555.

Note: See TracTickets for help on using tickets.