Re: [PATCH] Linking the Ruby bindings on windows

On Sat, 30 Dec 2006, Joe Swatosh wrote:

> On 12/29/06, Joe Swatosh <joe.swatosh@gmail.com> wrote:
> >On 12/29/06, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> >> On 12/29/06, Joe Swatosh <joe.swatosh@gmail.com> wrote:
> >
> >>
> >> Shouldn't _find_ruby also set the library and include paths (like
> >> _find_python does)? Do you set these manually in Visual Studio in
> >> order to get the bindings to build?
> >>
> >
> >I've been setting them manually. Since the Perl processing didn't set
> >the library and include paths, I didn't even consider it. It looks
> >like it could be done fairly easily using
> >
> >Config::CONFIG['libdir']
> >Config::CONFIG['archdir']
> >
> >I'm still not a Python programmer, but if consensus is that is how it
> >should be, I'll prepare another patch.
...
> [[[
> Generate project files for VC6 that will link the Ruby bindings with
> the Ruby library.
>
> * build/generator/gen_win.py
> Mimicked the way that Perl was determining what lib to link with when
> building the bindings. Mimicked the way that Python was determining
> the include and library paths.
> ]]]
>
>
> Index: build/generator/gen_win.py
> ===================================================================
> --- build/generator/gen_win.py (revision 22838)
> +++ build/generator/gen_win.py (working copy)
> @@ -145,6 +145,9 @@
> # Find the right Perl library name to link SWIG bindings with
> self._find_perl()
>
> + # Find the right Ruby library name to link SWIG bindings with
> + self._find_ruby()
> +
> # Find the right Python include and libraries dirs for SWIG bindings
> self._find_python()
>
> @@ -707,6 +710,8 @@
> fakeincludes.append(self.swig_libdir)
> if target.lang == "python":
> fakeincludes.extend(self.python_includes)
> + if target.lang == "ruby":
> + fakeincludes.extend(self.ruby_includes)

The indentation level appears to be off here for both the "if"
statement and its enclosed code block.

>
> fakeincludes.append(self.apath(self.zlib_path))
>
> @@ -738,6 +743,8 @@
> or isinstance(target, gen_base.TargetSWIGLib)):
> if target.lang == "python" and self.python_libdir:
> fakelibdirs.append(self.python_libdir)
> + if target.lang == "ruby" and self.ruby_libdir:
> + fakelibdirs.append(self.ruby_libdir)

Ditto. Joe, do you need to tweak some GMail setting to make it more
friendly with leading whitespace? I've noticed this issue with some
of your other patches as well.

> return fakelibdirs
>
> @@ -782,6 +789,11 @@
> and target.lang == 'perl'):
> nondeplibs.append(self.perl_lib)
>
> + if ((isinstance(target, gen_base.TargetSWIG)
> + or isinstance(target, gen_base.TargetSWIGLib))
> + and target.lang == 'ruby'):
> + nondeplibs.append(self.ruby_lib)
> +
> for dep in self.get_win_depends(target, FILTER_LIBS):
> nondeplibs.extend(dep.msvc_libs)
>
> @@ -953,6 +965,30 @@
> finally:
> fp.close()
>
> + def _find_ruby(self):
> + "Find the right Ruby library name to link swig bindings with"
> + self.ruby_includes = []
> + self.ruby_libdir = ""

We can initialize ruby_libdir to None instead of "".

> + proc = os.popen('ruby -rrbconfig -e ' + escape_shell_arg(
> + "puts Config::CONFIG['LIBRUBY'];"
> + "puts Config::CONFIG['archdir'];"
> + "puts Config::CONFIG['libdir'];"), 'r')
> + try:
> + libruby = proc.readline()[:-1]
> + if libruby:
> + msg = 'Found installed ruby.'
> + self.ruby_lib = libruby
> + self.ruby_includes.append(proc.readline()[:-1])
> + self.ruby_libdir = proc.readline()[:-1]
> + else:
> + msg = 'Could not detect Ruby version.'
> + self.ruby_lib = 'msvcrt-ruby18.lib'
> + sys.stderr.write('%s\n Ruby bindings will be linked with %s\n'
> + % (msg, self.ruby_lib))
> + finally:
> + proc.close()
> +
> +
> def _find_python(self):
> "Find the appropriate options for creating SWIG-based Python modules"
> self.python_includes = []

Looks good.