Opened 12 years ago

Closed 11 years ago

#61 closed defect (fixed)

vdigit makefile uses hardwired compile and link flags

Reported by: kyngchaos Owned by: grass-dev@…
Priority: major Milestone: 6.4.0
Component: wxGUI Version: 6.3.0 RCs
Keywords: Cc:
CPU: Unspecified Platform: Unspecified

Description

The vdigit makefile has some compile and link flags that should be left up to configuration.

CXXFLAGS has "-c -fpic". fpic is fPIC on OSX (fpic is invalid for OSX). -c is handled in the GRASS object make rules.

LDFLAGS has "-shared -fpic". -shared is platform-specific, and is not the correct flag for OSX.

If the GRASS make rules can't be used for vdigit, then at least its compile and link rules should more closely mirror the GRASS default rules to pick up platform configuration values.

Change History (13)

comment:1 Changed 12 years ago by neteler

I have changed -fpic to ${SHLIB_CFLAGS} (see r30332).

Still -shared is there.

Markus

comment:2 Changed 12 years ago by kyngchaos

Some more notes:

On OSX, the proper linking for a python extension is -bundle, not -dynamiclib. So "-shared" should be replaced by some configured value in Platform.make.

Shouldn't COMPILE_FLAGS_CXX be in CXXFLAGS, and LINK_FLAGS in LDFLAGS?

Also, with COMPILE_FLAGS_CXX and LINK_FLAGS, on OSX that's where the arch flags end up. On 64bit OSX Leopard, wxPython is not available in 64bits due to limitations in the system, so if one is trying to build 64bits, vdigit linking will fail. This will strip out any 64bit flags (if they are the only arch flags, it's OK because the default arch is 32bits on OSX):

CXXFLAGS = -c ${SHLIB_CFLAGS} $(subst -arch ppc64,,$(subst -arch x86_64,,${COMPILE_FLAGS_CXX})) \
-I$(ARCH_DISTDIR)/include $(GDALCFLAGS) $(PYTHONCFLAGS) $(WXWIDGETSCXXFLAGS)

LDFLAGS = -bundle ${SHLIB_CFLAGS} $(subst -arch ppc64,,$(subst -arch x86_64,,${LINK_FLAGS})) \
-L$(ARCH_LIBDIR) $(VECTLIB) $(GISLIB) $(GDALLIBS) $(VEDITLIB) $(WXWIDGETSLIB) $(PYTHONLDFLAGS) -lgdi

See ticket #58 for info about linking _gdi_.so.

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

Replying to kyngchaos:

On OSX, the proper linking for a python extension is -bundle, not -dynamiclib. So "-shared" should be replaced by some configured value in Platform.make.

Shouldn't COMPILE_FLAGS_CXX be in CXXFLAGS, and LINK_FLAGS in LDFLAGS?

I removed from Makefile some hardcoded flags and also added COMPILE_FLAGS_CXX and LINK_FLAGS (which are contained in CXXFLAGS and LDFLAGS, see Grass.make) -- r30523.

See ticket #58 for info about linking _gdi_.so.

_gdi_.so is still directly linked into _grass6_wxvdigit.so. When I remove -lgdi from LD_FLAGS 'import grass6_wxvdigit' fails

_grass6_wxvdigit.so: undefined symbol: _ZN10wxPseudoDC9AddToListEP5pdcOp

Martin

comment:4 Changed 12 years ago by kyngchaos

SHLIB_LDFLAGS is probably meant to be SHLIB_LD_FLAGS? Though this is empty for OSX.

Configure still needs to determine proper linking flags for a python extension (-shared/-bundle), which may be different from library linking, and set some platform.make variable, or add it to PYTHONLDFLAGS.

comment:5 Changed 12 years ago by hamish

fyi removed (or at least shifted) hardcoded SWIG=swig back into ./configure, r30525.

Hamish

comment:6 Changed 12 years ago by kyngchaos

Just taking a look at the latest incarnation of the makefile - SHLIB_LD can't be used for linking the module.so. This contains the dynamic/shared lib flag, which is not correct for a python extension on OSX, and contains the version and install_name flags meant for a GRASS library and are not valid for the -bundle flag needed here.

Would it be possible to configure a PYTHON_LD for use in this case?

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

Replying to kyngchaos:

Just taking a look at the latest incarnation of the makefile - SHLIB_LD can't be used for linking the module.so. This contains the dynamic/shared lib flag, which is not correct for a python extension on OSX, and contains the version and install_name flags meant for a GRASS library and are not valid for the -bundle flag needed here.

Would it be possible to configure a PYTHON_LD for use in this case?

Possible? Yes. Straightforward? No.

Look at the SC_CONFIG_CFLAGS macro in aclocal.m4, which defines SHLIB_LD. You would need to add a corresponding PYTHON_LD macro for each platform.

We can't just use "$(CC) $(PYTHONLDFLAGS)", as "python-config --ldflags" doesn't include the -shared flag on Linux (where it is definitely required).

comment:8 Changed 12 years ago by kyngchaos

There is distutils. Build it with distutils, and it will be compiled and linked correctly for each platform. The problem with this is that it builds locally into a folder with a platform-specific name chosen by distutils (ie build/lib.macosx-10.5-i386-2.5), so the makefile would not know where it is to install it in the GRASS dist folder. But it looks like there are build options to force a build dir name, so it would something known to the makefile.

That would mean creating a setup.py for the compile step.

comment:9 in reply to:  8 ; Changed 12 years ago by glynn

Replying to kyngchaos:

There is distutils. Build it with distutils, and it will be compiled and linked correctly for each platform. The problem with this is that it builds locally into a folder with a platform-specific name chosen by distutils (ie build/lib.macosx-10.5-i386-2.5), so the makefile would not know where it is to install it in the GRASS dist folder. But it looks like there are build options to force a build dir name, so it would something known to the makefile.

There are other Makefile integration issues, e.g. specifying the compiler, compilation switches, etc. Also, we don't want to re-compile files which haven't changed; that could become a significant issue for developers as the code grows.

More generally, I don't like the idea of invoking commands where no-one understands what's going on under the hood (this is why we don't use libtool). And, AFAIK, no-one here understands the internals of distutils.

If there is some way to get distutils to tell us which flags we should use (i.e. act like a working version of python-config), that would be preferable.

Also, as it stands, we currently don't know how to invoke Python on the setup.py file (configure currently detects python-config, not python).

comment:10 Changed 12 years ago by kyngchaos

You could look at GDAL for an example of configuring and running distutils. Also, the old python bindings in GDAL (ogpython, in the pymod dir) used a manual-compile method that could be of use. But distutils is standard stuff for python, so we shouldn't be afraid to use it.

If we don't use distutils, for whatever reason, we're stuck with trying to configure some form of PYTHON_LD, like in the GDAL ogpython.

comment:11 in reply to:  9 Changed 12 years ago by hamish

Replying to glynn:

If there is some way to get distutils to tell us which flags we should use (i.e. act like a working version of python-config), that would be preferable.

Also, as it stands, we currently don't know how to invoke Python on the setup.py file (configure currently detects python-config, not python).

maybe some hints here:

source:grass/trunk/swig/python/NumPtr/setup.py

Hamish

comment:12 Changed 11 years ago by martinl

Component: PythonwxGUI

comment:13 Changed 11 years ago by martinl

CPU: Unspecified
Platform: Unspecified
Resolution: fixed
Status: newclosed

Currently are wxGUI C++ extensions built with setup.py. Closing the ticket.

Martin

Note: See TracTickets for help on using tickets.