From: William S F. <ws...@fu...> - 2007-05-03 10:23:23
|
Gonzalo is going great guns on the Ruby module with plenty of commits. I'm sure Ruby users will appreciate the improvement. As the commits are a lot more than the original bugs he filed, can I just ask Gonzalo to update the list giving a high level summary of what he has been doing and what his plans are? Thanks William |
From: Olly B. <ol...@su...> - 2007-05-08 02:43:34
|
On 2007-05-03, William S Fulton <ws...@fu...> wrote: > Gonzalo is going great guns on the Ruby module with plenty of commits. I'm sure > Ruby users will appreciate the improvement. As the commits are a lot more than > the original bugs he filed, can I just ask Gonzalo to update the list giving a > high level summary of what he has been doing and what his plans are? Seconded! There are just too many commits for me to follow what's happening here. It's great that SWIG's ruby support is being improved but we need to try to avoid problems being introduced in the process. Unfortunately current SWIG SVN HEAD fails to build xapian's ruby bindings. The first problem is that the SWIG -feature option has been renamed to -init_name. I don't think options should just change name like this without us continuing to accept the old name for compatibility, as otherwise users can't write a makefile (or other build script) which works with SWIG <= 1.3.31 and SWIG >= 1.3.32. Anyway, if I rerun SWIG with -init_name instead of -feature, I get bad C++ code in xapian_wrap.cc: fail: Ruby_Format_OverloadedError( argc, 3, "ConstIterator.incr", " swig::ConstIterator * ConstIterator.incr(size_t n Error, "1)\n" " swig::ConstIterator * ConstIterator.incr()\n"); It starts to go wrong at `"1)\n"' on the third line. Looking at ruby.cxx there's a call to Printf with format "%s %s" but only one string being passed. This patch fixes this, and then I can build and "make check" on xapian-bindings passes for Ruby again: http://oligarchy.co.uk/xapian/patches/swig-ruby-bad-code-fix.patch I would just apply it, but I wonder if you intended to pass a second string here which matters in some cases? And is there a way to suppress this new `ConstIterator' stuff? I don't think I want it in there for my purposes, as we already have hand written ruby wrappers for the C++ iterators. The Ruby documentation doesn't seem to say how to turn this off. Cheers, Olly |
From: William S F. <ws...@fu...> - 2007-05-08 19:07:49
|
Olly Betts wrote: > On 2007-05-03, William S Fulton <ws...@fu...> wrote: >> Gonzalo is going great guns on the Ruby module with plenty of commits. I'm sure >> Ruby users will appreciate the improvement. As the commits are a lot more than >> the original bugs he filed, can I just ask Gonzalo to update the list giving a >> high level summary of what he has been doing and what his plans are? > > Seconded! There are just too many commits for me to follow what's > happening here. It's great that SWIG's ruby support is being improved > but we need to try to avoid problems being introduced in the process. > > Unfortunately current SWIG SVN HEAD fails to build xapian's ruby > bindings. > > The first problem is that the SWIG -feature option has been renamed to > -init_name. I don't think options should just change name like this without > us continuing to accept the old name for compatibility, as otherwise users > can't write a makefile (or other build script) which works with SWIG <= 1.3.31 > and SWIG >= 1.3.32. > > Anyway, if I rerun SWIG with -init_name instead of -feature, I get bad C++ > code in xapian_wrap.cc: > > fail: > Ruby_Format_OverloadedError( argc, 3, "ConstIterator.incr", > " swig::ConstIterator * ConstIterator.incr(size_t n Error, "1)\n" > " swig::ConstIterator * ConstIterator.incr()\n"); > > It starts to go wrong at `"1)\n"' on the third line. Looking at ruby.cxx > there's a call to Printf with format "%s %s" but only one string being > passed. This patch fixes this, and then I can build and "make check" > on xapian-bindings passes for Ruby again: > > http://oligarchy.co.uk/xapian/patches/swig-ruby-bad-code-fix.patch > > I would just apply it, but I wonder if you intended to pass a second string > here which matters in some cases? > > And is there a way to suppress this new `ConstIterator' stuff? I don't think I > want it in there for my purposes, as we already have hand written ruby wrappers > for the C++ iterators. The Ruby documentation doesn't seem to say how to turn > this off. > Gonzalo, can you get your mailing list subscription sorted out so we can all follow what is going on please? Thanks William |
From: Olly B. <ol...@su...> - 2007-05-09 01:24:52
|
On 2007-05-08, Olly Betts <ol...@su...> wrote: > It starts to go wrong at `"1)\n"' on the third line. Looking at ruby.cxx > there's a call to Printf with format "%s %s" but only one string being > passed. I wondered how commonly we call Printf() with the wrong number of parameters or with a bad format string, so I hacked a GCC specific attribute onto the Printf and vPrintf prototypes and filtered out the noise with grep (since Printf() is very liberal in what can be passed for %s!) This has found rather a lot of mistakes! Most involve passing too many parameters, and often the extra parameter is `NIL' (presumably left over from changing a call to PrintV() to Printf()) or it's obviously come from copying and modifying a nearby Printf() call. These are just code cruft on many platforms, but on some passing extra arguments messes up the stack so fixing this is a portability fix. It also avoids heartache when someone modifies a Printf() call and adds a new parameter to the end of the list... There are also a few cases of a literal `%' in a message not being escaped. These cases have an obvious fix... I've been going through the list of warnings and I'm checking in fixes where I'm confident that I'm doing the right thing. It would be prudent to look over the diffs for files in backends you maintain just to make sure though. Once I've done all the easy ones, I'll produce patches for any cases I'm unsure about. Cheers, Olly |