Mapserver compilation flags should be added to the SWIG
command line
Steve Lime
Steve.Lime at DNR.STATE.MN.US
Fri Sep 1 11:53:08 EDT 2006
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