New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python MapScript broken with SWIG 2.0.5 #4325

Merged
merged 1 commit into from Oct 2, 2012

Conversation

Projects
None yet
2 participants
@tbonfort
Member

tbonfort commented Sep 30, 2012

When using SWIG 2.0.5 for building Python MapScript it reports "AttributeError: No constructor defined" when trying to initialize a new mapObj. Using SWIG 2.0.4 works though.

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Jun 20, 2012

Member

seems like a swig issue, nothing much we can do about it aside from keeping this available as reference if someone has the same issue. For info, I've tested these versions:

  • 2.0.4 works
  • 2.0.6 works
  • 2.0.7 segfaults
Member

tbonfort commented Jun 20, 2012

seems like a swig issue, nothing much we can do about it aside from keeping this available as reference if someone has the same issue. For info, I've tested these versions:

  • 2.0.4 works
  • 2.0.6 works
  • 2.0.7 segfaults

@tbonfort tbonfort closed this Jun 20, 2012

@tbonfort tbonfort reopened this Jun 20, 2012

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Jun 20, 2012

Member

reopening, seems to be another issue

Member

tbonfort commented Jun 20, 2012

reopening, seems to be another issue

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 6, 2012

Member

demilestoning since there's a workaround (use 2.0.4) and no one wants to tackle the issue right away.

Member

tbonfort commented Aug 6, 2012

demilestoning since there's a workaround (use 2.0.4) and no one wants to tackle the issue right away.

@tbonfort tbonfort closed this Sep 8, 2012

@tbonfort tbonfort reopened this Sep 8, 2012

tbonfort added a commit to tbonfort/mapserver that referenced this pull request Sep 30, 2012

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Sep 30, 2012

Member

The error was arising because swig in recent versions is ignoring constructors who's name doesn't match the original c-struct name, namely for mapObj, layerObj, classObj and errorObj which are defined as map_obj, layer_obj, class_obj and error_obj.
This can be seen by this kind of warning message when running swig:

../swiginc/class.i:66: Warning 504: Function class_obj::classObj(layerObj *) must have a return type. Ignored.
../swiginc/class.i:77: Warning 521: Illegal destructor name class_obj::~classObj(). Ignored.

The proposed fix changes our use of map_obj into mapObj everywhere, which fixes this swig issue. However I fear that this is going to have some side effects as some compilers might be upset by this change. For reference, the commit that first introduced this notation is a332c37 (yeah, that's old :) )

@szekerest @jmckenna : can you please check that this new code still compiles on your windows buildkits ?

git remote add tbonfort git://github.com/tbonfort/mapserver.git
git fetch tbonfort
git checkout swig-fixes
#normal windows build as if it was master

maybe @dmorissette has some insight as to why this syntax was used ?

Member

tbonfort commented Sep 30, 2012

The error was arising because swig in recent versions is ignoring constructors who's name doesn't match the original c-struct name, namely for mapObj, layerObj, classObj and errorObj which are defined as map_obj, layer_obj, class_obj and error_obj.
This can be seen by this kind of warning message when running swig:

../swiginc/class.i:66: Warning 504: Function class_obj::classObj(layerObj *) must have a return type. Ignored.
../swiginc/class.i:77: Warning 521: Illegal destructor name class_obj::~classObj(). Ignored.

The proposed fix changes our use of map_obj into mapObj everywhere, which fixes this swig issue. However I fear that this is going to have some side effects as some compilers might be upset by this change. For reference, the commit that first introduced this notation is a332c37 (yeah, that's old :) )

@szekerest @jmckenna : can you please check that this new code still compiles on your windows buildkits ?

git remote add tbonfort git://github.com/tbonfort/mapserver.git
git fetch tbonfort
git checkout swig-fixes
#normal windows build as if it was master

maybe @dmorissette has some insight as to why this syntax was used ?

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Oct 1, 2012

Member

tested the proposed fix on an old compiler (gcc 3.4.6 from centos 4) and it compiles fine. waiting on the windows guys...

Member

tbonfort commented Oct 1, 2012

tested the proposed fix on an old compiler (gcc 3.4.6 from centos 4) and it compiles fine. waiting on the windows guys...

@Schpidi

This comment has been minimized.

Show comment
Hide comment
@Schpidi

Schpidi Oct 1, 2012

Member

can confirm this works on debian 6.0 with swig 2.0.7

Member

Schpidi commented Oct 1, 2012

can confirm this works on debian 6.0 with swig 2.0.7

@tbonfort tbonfort merged commit 3e6be47 into mapserver:branch-6-2 Oct 2, 2012

1 check was pending

default Build triggered.
@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Oct 2, 2012

Member

applied to 6.2, recent swig versions produce buildable/runnable code, along with some build fixes for java and ruby

Member

tbonfort commented Oct 2, 2012

applied to 6.2, recent swig versions produce buildable/runnable code, along with some build fixes for java and ruby

@ghost ghost assigned tbonfort Oct 2, 2012

mkofahl pushed a commit to faegi/mapserver that referenced this pull request Apr 9, 2013

mkofahl pushed a commit to faegi/mapserver that referenced this pull request Apr 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment