Mapserver compilation flags should be added to the SWIG command line

Tamas Szekeres szekerest at GMAIL.COM
Fri Sep 1 18:14:11 EDT 2006 

For the sake of completeness we have problems with the following
defines and structures:

USE_POINT_Z_M 		pointObj
USE_GEOS		shapeObj
USE_PROJ		projectionObj

USE_PDF		PDFObj;
USE_MING_FLASH	SWFObj;


Tamas



2006/9/1, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
> I'm preparing to commit the GEOS patch this evening. I'll have a patch
> for Z_M ready for review tomorrow.
>
> Umberto
>
> On 9/1/06, Steve Lime <Steve.Lime at dnr.state.mn.us> wrote:
> > Initially pointObj's did have z and m members always but DM Solutions
> > testing
> > found a significant performance penalty for doing so. Perhaps that
> > performance
> > penalty can be addressed. However, when it comes to performance we
> > should
> > strive to keep that as high as possible. I think that trumps design
> > consistency in
> > a few instances.
> >
> > Steve
> >
> > >>> Tamas Szekeres <szekerest at GMAIL.COM> 8/31/2006 5:14:19 PM >>>
> > Steve,
> >
> > Currently some members of some structs are also affected by the
> > #defines (eg. shapeObj, pointObj) so if we want to accept this way for
> > handling the issue every interface item should be generated regardless
> > of the switches. For example shapeObj must always have a "void*
> > geometry" and pointObj must always have z and m members.
> >
> > The second option would be to pass the preprocessor defines to the
> > SWIG command line. (I could not get any reason why not to do it so
> > far)
> >
> > So I think we should choose the right way according to the decision of
> > the majority of the developers. The next IRC meeting would be a
> > sufficient place to make the decision.
> >
> > Unfortunately I cannot take part on it at the proposed time. 2 hours
> > later would be more suitable for me.
> >
> > I would have +0 on the first solution and +1 on the second one.
> >
> > Tamas
> >
> >
> > 2006/8/31, Steve Lime <Steve.Lime at dnr.state.mn.us>:
> > > I'm ok with the always having the geometry there and then hiding in
> > any
> > > case.
> > >
> > > Regarding the defines with SWIG. I guess it boils down to whether or
> > > not you
> > > want:
> > >
> > >   1) a consistent interface regardless of build environment, that
> > is,
> > > something like $shape->interects(...) always exists, but will throw
> > an
> > > error if the right supporting library isn't available
> > >
> > >   2) an inconsistent interface that has different syntax depending
> > on
> > > build environment
> > >
> > > Option 1 forces folks to practice good programming and actually
> > check
> > > returned values, while option 2 is far less subtle. You'll know
> > right
> > > away if something isn't available. Not sure which I prefer.
> > >
> > > Steve
> > >
> > > >>> Tamas Szekeres <szekerest at GMAIL.COM> 8/31/2006 6:49:48 AM >>>
> > > Umberto,
> > >
> > > At this point I cannot see considerable reason not to apply those
> > > changes. But I am still sure that adding the defines to SWIG and the
> > > mapscript C compilation ensures that the SWIG interface will be
> > > generated correctly and no runtime problems will happen (dedicated
> > to
> > > this issue).
> > >
> > > But again, I am +1 adding void* geometry to shapeObj regardless of
> > the
> > > USE_GEOS setting and hiding this member from the SWIG interface.
> > >
> > > Tamas
> > >
> > >
> > >
> > > 2006/8/31, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
> > > > Tamas,
> > > > I have reviewed all the .h files included in mapscript.i and I
> > have
> > > > found that there are only a few unsafe usages of defines (and they
> > > > refer to the usual suspects, that is z/m members for points and
> > > geos).
> > > >
> > > > In mapprimitive.h
> > > >
> > > > line 74 (Definitely UNSAFE!!)
> > > > #ifdef USE_POINT_Z_M
> > > >   double z;
> > > >   double m;
> > > > #endif
> > > >
> > > > line 100 (UNSAFE, but fixable):
> > > > #ifdef USE_GEOS
> > > >   void *geometry;
> > > > #endif
> > > >
> > > > Other suspicious usages could be in mapows.h, but at this time I
> > am
> > > > not sure whether they are actually posing a problem.
> > > >
> > > > I don't see an easy solution to the z/m defines, but I attach a
> > > patch
> > > > that removes the need for #ifdef USE_GEOS in mapprimitive.h. I
> > have
> > > > made a couple of tests and it seems ok to me, but I want your
> > > valuable
> > > > feedback and Steve's approval before I commit it.
> > > >
> > > > Umberto
> > > >
> > > > On 8/31/06, Tamas Szekeres <szekerest at gmail.com> wrote:
> > > > > Hi Umberto,
> > > > >
> > > > > How your proposal will prevent from the need of settings the
> > > defines to SWIG?
> > > > > We have ifdefs in some of the headers (eg. in map.h) will
> > continue
> > > to
> > > > > affect the interface generation.
> > > > >
> > > > > Tamas
> > > > >
> > > > >
> > > > >
> > > > > 2006/8/31, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
> > > > > > Hi Tamas,
> > > > > >
> > > > > > On 8/30/06, Tamas Szekeres <szekerest at gmail.com> wrote:
> > > > > > > Umberto,
> > > > > > >
> > > > > > > I guess your proposal was to replace the affected functions
> > > from the
> > > > > > > interface files to the core mapserver code, correct me if I
> > am
> > > wrong.
> > > > > >
> > > > > > You are right.
> > > > > >
> > > > > > > But I still can't understand what's the problem with adding
> > > the
> > > > > > > preprocessor defines to the SWIG command line. It requires
> > to
> > > change
> > > > > > > only a line in the makefile.
> > > > > >
> > > > > > Only Java and c# have makefiles. Perl, python, tcl don't.
> > > > > > Keeping preprocessor defines out of the interface files was a
> > > design
> > > > > > decision taken a long time ago and (almost) always enforced.
> > In
> > > fact
> > > > > > there aren't other defines in all the rest of the swig
> > interface
> > > > > > files.
> > > > > >
> > > > > > Umberto
> > > > > >
> > > > > > >
> > > > > > > Tamas
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > 2006/8/30, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
> > > > > > > > Hi Steve,
> > > > > > > > While doing the usual house keeping before 4.10 gets out I
> > > have looked
> > > > > > > > at the source of the swig interface files and found that
> > they
> > > contain
> > > > > > > > preprocessor defines (#ifdef USE_GEOS for example).
> > > > > > > > Most of the mapscript laguages will ignore these defines
> > > because swig
> > > > > > > > is not passed the list of options built by configure. Java
> > > mapscript
> > > > > > > > used to pass the list to swig, but this feature was removed
> > a
> > > long
> > > > > > > > time ago because Sean opted for removing the preprocessor
> > > defines from
> > > > > > > > the swig interface files to keep them tidier.
> > > > > > > > C# mapscript at this point is the only laguage still
> > passing
> > > defines to swig.
> > > > > > > > Perl, Ruby and Python invoke swig without passing the
> > > defines; also,
> > > > > > > > in the case of Perl and Python swig must be run manually
> > by
> > > the user
> > > > > > > > (by typing the right command in the shell).
> > > > > > > >
> > > > > > > > Now since most mapscript will ignore those preprocessor
> > > defines we are
> > > > > > > > facing a potentially critical issue and, in the case of
> > > GEOS,
> > > > > > > > mapscript won't use some GEOS functions.
> > > > > > > > Other possible errors could be memory errors due to the
> > use
> > > of non
> > > > > > > > initialized variables (this is the case of POINT_Z_M).
> > > > > > > >
> > > > > > > > I have reviewed the swig interface files and found these
> > > possibly
> > > > > > > > harmful defines:
> > > > > > > >
> > > > > > > > swiginc/image.i:#if GD2_VERS > 1
> > > > > > > >
> > > > > > > > Not sure about this one, maybe it is right. Dunno where
> > > GD2_VERS is defined.
> > > > > > > >
> > > > > > > > swiginc/image.i:#ifdef USE_GD_GIF
> > > > > > > > swiginc/image.i:#ifdef USE_GD_PNG
> > > > > > > > swiginc/image.i:#ifdef USE_GD_JPEG
> > > > > > > > swiginc/image.i:#ifdef USE_GD_WBMP
> > > > > > > >
> > > > > > > > These were added by Sean in July 04 and should be removed,
> > I
> > > guess.
> > > > > > > > They only affect TCL.
> > > > > > > >
> > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > >
> > > > > > > > These are *rarely* used and they are not a real problem,
> > at
> > > least for
> > > > > > > > 4.10. If used they crash Java mapscript for sure.
> > > > > > > >
> > > > > > > > swiginc/shape.i:#ifdef USE_GEOS
> > > > > > > > swiginc/shape.i:#ifdef USE_GEOS
> > > > > > > >
> > > > > > > > These were added by sdlime on his lates rework of geos
> > code
> > > this june.
> > > > > > > > They should be moved out of the swig interface file into
> > the
> > > c code of mapserver
> > > > > > > >
> > > > > > > > I hope I made myself clear,
> > > > > > > > Umberto
> > > > > > > >
> > > > > > > > On 8/28/06, Steve Lime <Steve.Lime at dnr.state.mn.us> wrote:
> > > > > > > > > Confused, what's the desired fix?
> > > > > > > > >
> > > > > > > > > >>> Umberto Nicoletti <umberto.nicoletti at GMAIL.COM>
> > > 8/28/2006 10:47:08
> > > > > > > > > AM >>>
> > > > > > > > > I have posted a similar mail today (search for Defines
> > in
> > > swig
> > > > > > > > > interface files in the subject) . Tamas, sorry for the
> > > double posting,
> > > > > > > > > I missed this one.
> > > > > > > > >
> > > > > > > > > I appear to recall that it was once agreed that swig
> > > interface files
> > > > > > > > > should not have defines in them, but I at the moment the
> > > only
> > > > > > > > > reference I found is here:
> > > > > > > > >
> > > > > > > > > http://mapserver.gis.umn.edu/bugs/show_bug.cgi?id=895#c2
> >
> > > > > > > > > (end of first para)
> > > > > > > > >
> > > > > > > > > Sean Gillies probably knows better.
> > > > > > > > >
> > > > > > > > > At this point C# is the only language that has 100%
> > > correct
> > > > > > > > > compilation today. Java mapscript had the flags passed
> > to
> > > swig, but
> > > > > > > > > they were removed a long time ago.
> > > > > > > > > Other laguages like ruby and python require the user to
> > > generate the
> > > > > > > > > interface by running swig by hand. This makes it much
> > > difficult to
> > > > > > > > > pass defines to swig as the user should have to copy
> > them
> > > from the
> > > > > > > > > Makefile or alternatively we should provide and mantain
> > > Makefiles for
> > > > > > > > > all mapscripts.
> > > > > > > > >
> > > > > > > > > This is the list of defines (still) in the swig
> > interface
> > > files:
> > > > > > > > >
> > > > > > > > > swiginc/image.i:#if GD2_VERS > 1
> > > > > > > > >
> > > > > > > > > Not sure about this one, maybe it is right.....
> > > > > > > > >
> > > > > > > > > swiginc/image.i:#ifdef USE_GD_GIF
> > > > > > > > > swiginc/image.i:#ifdef USE_GD_PNG
> > > > > > > > > swiginc/image.i:#ifdef USE_GD_JPEG
> > > > > > > > > swiginc/image.i:#ifdef USE_GD_WBMP
> > > > > > > > >
> > > > > > > > > These were added by Sean in July 04.
> > > > > > > > >
> > > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > > swiginc/point.i:#ifdef USE_POINT_Z_M
> > > > > > > > >
> > > > > > > > > These are rarely used and they are not a real problem,
> > at
> > > least for
> > > > > > > > > 4.10
> > > > > > > > >
> > > > > > > > > swiginc/shape.i:#ifdef USE_GEOS
> > > > > > > > > swiginc/shape.i:#ifdef USE_GEOS
> > > > > > > > >
> > > > > > > > > These were added by sdlime on his lates rework of geos
> > code
> > > this june.
> > > > > > > > >
> > > > > > > > > I can try to take care of the defines in image.i and
> > maybe
> > > steve of
> > > > > > > > > those in shape.i, but I dunno if I'm going or if we
> > should
> > > do it for
> > > > > > > > > for 4.10?
> > > > > > > > >
> > > > > > > > > Umberto
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On 8/28/06, Daniel Morissette <dmorissette at mapgears.com>
> > > wrote:
> > > > > > > > > > Tamas Szekeres wrote:
> > > > > > > > > > > Folks,
> > > > > > > > > > >
> > > > > > > > > > > Currently most of the SWIG bindings does not add
> > > mapserver
> > > > > > > > > compilation
> > > > > > > > > > > flags to the
> > > > > > > > > > > SWIG command line and therefore the interface might
> > not
> > > be
> > > > > > > > > generated
> > > > > > > > > > > properly.
> > > > > > > > > > > For example missing -DUSE_POINT_Z_M will prevent
> > from
> > > generating z
> > > > > > > > > and m
> > > > > > > > > > > members
> > > > > > > > > > > for pointObj etc.
> > > > > > > > > > >
> > > > > > > > > > > I've added a bug for this issue:
> > > > > > > > > > >
> > http://mapserver.gis.umn.edu/bugs/show_bug.cgi?id=1881
> > >
> > > > > > > > > > >
> > > > > > > > > > > And fixed for C#
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Does something need to be done for this for the other
> > > wrappers?
> > > > > > > > > There
> > > > > > > > > > used to be a 'mapscriptvars' file with all the compile
> > > flags used by
> > > > > > > > > the
> > > > > > > > > > Perl Makefile. Is this not used by the other bindings?
> > > > > > > > > >
> > > > > > > > > > Daniel
> > > > > > > > > > --
> > > > > > > > > > Daniel Morissette
> > > > > > > > > > http://www.mapgears.com/
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > >
> >
>

More information about the mapserver-dev mailing list