On Thu, May 28, 2009 at 10:23:54PM -0700, Joe Swatosh wrote:
> This is not part of the Ruby bindings so it is not for me to commit
> without approval by a full committer. Is the patch acceptable with the
> log message amended below?
>
> --
> Joe
>
> [[[
>
> Partially revert r37331 "On Windows, shared libs for Python bindings
> should be *.pyd not *.dll." In addition to changing the shared libs for
> the Python bindings, r37331 inadvertently changed the extensions of the
> shared libraries produced for the Perl and Ruby bindings from .dll to
> .pyd. Change the extensions back to .dll for the Ruby and Perl
> bindings.
>
> * build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
> wrapper shared libs as type "pyd" not "lib".
>
> ]]]
>
> ===================================================================
> --- build/generator/gen_base.py (revision 37787)
> +++ build/generator/gen_base.py (working copy)
> @@ -550,12 +550,13 @@
> # Extract SWIG module name from .i file name
> module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
>
> - lib_extension = self.gen_obj._extension_map['pyd', 'target']
> + lib_extension = self.gen_obj._extension_map['lib', 'target']
> if self.lang == "ruby":
> lib_filename = module_name + lib_extension
> elif self.lang == "perl":
> lib_filename = '_' + module_name.capitalize() + lib_extension
> else:
> + lib_extension = self.gen_obj._extension_map['pyd', 'target']
Why put this in an else clause? This code assumes that we'll never
ever add more scripting languages to the bindings. I'd say it would
be safer to explicitly check for python before changing the extension
to ".pyd". We don't want the same problem to bite us again some time
later. We should rather make sure that this problem will never hit
us again. So I'd suggest doing something like this instead:
if self.lang == "ruby":
lib_filename = module_name + lib_extension
elif self.lang == "perl":
lib_filename = '_' + module_name.capitalize() + lib_extension
elif self.lang == "python":
lib_extension = self.gen_obj._extension_map['pyd', 'target']
else:
# throw an error, unknown language binding, please add proper
# library filename extension here
Stefan
Received on 2009-05-29 12:57:10 CEST