You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2006/03/12 21:07:06 UTC
Ruby test failure
Hi,
I wanted to vote for the r17280 backport group in 1.3.x/STATUS, but got a Ruby
test failure.
I am using SWIG 1.3.28 and Ruby 1.8.4 on Debian testing.
I checked out a fresh WC, merged the integration branch and ran:
make all swig-py swig-pl swig-rb
make check-swig-pl check-swig-py check-swig-rb
The last target gave the following output:
cd /home/pl/src/svn/13x/subversion/bindings/swig/ruby; \
/usr/bin/ruby -I /home/pl/src/svn/13x/subversion/bindings/swig/ruby \
/home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/run-test.rb \
--verbose=normal
Loaded suite test
Started
................................................................................................................F......
Finished in 299.491669 seconds.
1) Failure:
test_locked(SvnWcTest)
[/home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/test_wc.rb:333:in `test_locked'
/home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/util.rb:55:in `change_gc_status'
/home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/util.rb:70:in `gc_enable'
/home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/test_wc.rb:331:in `test_locked']:
<false> is not true.
119 tests, 616 assertions, 1 failures, 0 errors
The tests all pass using SWIG 1.3.25. Is this a known problem, or does it
look like something that could be wrong on my system?
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by David James <dj...@collab.net>.
On 3/17/06, Kouhei Sutou <ko...@cozmixng.org> wrote:
> Hi,
>
> In <17...@localhost.localdomain>
> "Re: Ruby test failure" on Fri, 17 Mar 2006 09:48:26 +0100,
> "Peter N. Lundblad" <pe...@famlundblad.se> wrote:
>
> > > I fixed the problem in r18924.
> >
> > Does this mean r18924 should be proposed for backport together with
> > the other swig 1.3.28 proposals and nominated for 1.3.2?
>
> I nominated r18924 for 1.3.1 and added reliance on r18924 to
> r17280 family.
>
> Should this move to 1.3.2 section? Is it too late?
Yes, all changes that were not merged into 1.3.1 should be moved into
the 1.3.2 section. It is too late for 1.3.1 because 1.3.1 has already
been rolled
Cheers,
David
--
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
In <17...@localhost.localdomain>
"Re: Ruby test failure" on Fri, 17 Mar 2006 09:48:26 +0100,
"Peter N. Lundblad" <pe...@famlundblad.se> wrote:
> > I fixed the problem in r18924.
>
> Does this mean r18924 should be proposed for backport together with
> the other swig 1.3.28 proposals and nominated for 1.3.2?
I nominated r18924 for 1.3.1 and added reliance on r18924 to
r17280 family.
Should this move to 1.3.2 section? Is it too late?
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Tue, Mar 21, 2006 at 02:38:16PM +0900, Kouhei Sutou wrote:
> We don't need to check any error for r2c_swig_type2() in
> svn_swig_rb_get_pool(). Because r2c_swig_type2() raises an
> exception when an error is occurred. Ruby's exception works
> like longjmp() in C. So, the r2c_swig_type2() call don't
> return to svn_swig_rb_get_pool() if an error occurred in
> r2c_swig_type2().
>
Ah, thanks for the explanation. I thought it was probably something
along those lines, but SWIG's documentation is a little lacking.
> # I'm sorry for my poor English...
>
Not at all; it's significantly better than my (non-existent) Japanese!
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
In <20...@lorenz.farside.org.uk>
"Re: Ruby test failure" on Mon, 20 Mar 2006 13:39:28 +0000,
Malcolm Rowe <ma...@farside.org.uk> wrote:
> > > Okay, so now if r2c_swig_type2() fails, we will now raise an error,
> > > but svn_swig_rb_get_pool() will still dereference pool_wrapper regardless,
> > > won't it?
> >
> > Yes. Because svn_swig_rb_get_pool() uses r2c_swig_type2(),
> > svn_swig_rb_get_pool() will raises an error without checking
> > in svn_swig_rb_get_pool().
> >
>
> I'm not quite sure if you're agreeing with me (I'm not sure what 'raises
> an error without checking' means); if you're not, could you explain a
> bit more?
>
> Shouldn't we be checking for a failure from calling r2c_swig_type2()?
> Or if it's 'never going to happen', at least set *result to NULL after
> we call SWIG_Error(), so that we'll get a more predictable crash later?
I'm sorry. I understood your comment by mistake.
We don't need to check any error for r2c_swig_type2() in
svn_swig_rb_get_pool(). Because r2c_swig_type2() raises an
exception when an error is occurred. Ruby's exception works
like longjmp() in C. So, the r2c_swig_type2() call don't
return to svn_swig_rb_get_pool() if an error occurred in
r2c_swig_type2().
# I'm sorry for my poor English...
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Mon, Mar 20, 2006 at 09:32:34AM +0900, Kouhei Sutou wrote:
> > Okay, so now if r2c_swig_type2() fails, we will now raise an error,
> > but svn_swig_rb_get_pool() will still dereference pool_wrapper regardless,
> > won't it?
>
> Yes. Because svn_swig_rb_get_pool() uses r2c_swig_type2(),
> svn_swig_rb_get_pool() will raises an error without checking
> in svn_swig_rb_get_pool().
>
I'm not quite sure if you're agreeing with me (I'm not sure what 'raises
an error without checking' means); if you're not, could you explain a
bit more?
Shouldn't we be checking for a failure from calling r2c_swig_type2()?
Or if it's 'never going to happen', at least set *result to NULL after
we call SWIG_Error(), so that we'll get a more predictable crash later?
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
In <20...@lorenz.farside.org.uk>
"Re: Ruby test failure" on Sun, 19 Mar 2006 11:36:08 +0000,
Malcolm Rowe <ma...@farside.org.uk> wrote:
> Okay, so now if r2c_swig_type2() fails, we will now raise an error,
> but svn_swig_rb_get_pool() will still dereference pool_wrapper regardless,
> won't it?
Yes. Because svn_swig_rb_get_pool() uses r2c_swig_type2(),
svn_swig_rb_get_pool() will raises an error without checking
in svn_swig_rb_get_pool().
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Sun, Mar 19, 2006 at 10:38:19AM +0900, Kouhei Sutou wrote:
> This document is old for SWIG 1.3.28. For SWIG 1.3.28, if
> flags is 1, free function of the Ruby object is removed; if
> flags is 0, free function of the Ruby object isn't removed.
>
> The cause of the problem is that free function of Ruby's
> pool object is removed by SWIG_ConvertPtr() of SWIG 1.3.28.
> And the free function cleans up a lock in working copy.
>
> I fixed the problem by preventing a free function deletion.
>
Ah, I see. That makes more sense now, thanks!
> We don't have. We should check returned value.
> I added the code in r18951.
>
Okay, so now if r2c_swig_type2() fails, we will now raise an error,
but svn_swig_rb_get_pool() will still dereference pool_wrapper regardless,
won't it?
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
In <20...@lorenz.farside.org.uk>
"Re: Ruby test failure" on Sat, 18 Mar 2006 13:43:45 +0000,
Malcolm Rowe <ma...@farside.org.uk> wrote:
> So the change you made in r18924 was just to change the 'flags' parameter
> passed to SWIG_ConvertPtr() from non-zero to zero. That parameter is
> explained as follows in the SWIG documentation:
>
> int SWIG_ConvertPtr(VALUE obj, void **ptr, swig_type_info *ty, int flags)
>
> Converts a Ruby object obj to a C pointer whose address is ptr (i.e. ptr
> is a pointer to a pointer). The third argument, ty, is a pointer to a
> SWIG type descriptor structure. If ty is not NULL, that type information
> is used to validate type compatibility and other aspects of the type
> conversion. If flags is non-zero, any type errors encountered during
> this validation result in a Ruby TypeError exception being raised;
> if flags is zero, such type errors will cause SWIG_ConvertPtr() to
> return -1 but not raise an exception. If ty is NULL, no type-checking
> is performed.
This document is old for SWIG 1.3.28. For SWIG 1.3.28, if
flags is 1, free function of the Ruby object is removed; if
flags is 0, free function of the Ruby object isn't removed.
> My questions are:
>
> * What guarantee do we have that pool_wrapper is dereferencable at all,
> when we aren't checking the return value of the function that sets it?
> I don't see any guarantee that SWIG_ConvertPtr() ensures that the
> output pointer is set to anything valid on error.
We don't have. We should check returned value.
I added the code in r18951.
> * What problem is this fixing? The log message says "Support SWIG 1.3.28.
> Keep owning pool.", but apart from fixing the Ruby test failures
> (somehow), I don't understand what this change is trying to achieve.
The cause of the problem is that free function of Ruby's
pool object is removed by SWIG_ConvertPtr() of SWIG 1.3.28.
And the free function cleans up a lock in working copy.
I fixed the problem by preventing a free function deletion.
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Peter Samuelson <pe...@p12n.org>.
[Peter N. Lundblad]
> Does this mean r18924 should be proposed for backport together with
> the other swig 1.3.28 proposals and nominated for 1.3.2?
Yes (I've already added it to our Debian builds), but I'd wait before
voting +1 until someone figures out why svk breaks
(http://bugs.debian.org/356894). clkao and I are both trying to find
time to look at this. Unfortunately, I'm not very qualified, not being
an expert in either libsvn or swig, so I can't promise much.
\\Peter
Re: Ruby test failure
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Kouhei Sutou writes:
> Hi,
>
> I'm sorry for my late response.
No problem:-)
> I fixed the problem in r18924.
Does this mean r18924 should be proposed for backport together with
the other swig 1.3.28 proposals and nominated for 1.3.2?
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Fri, Mar 17, 2006 at 03:16:38PM +0900, Kouhei Sutou wrote:
> > OK, thanks for the answer. This makes me think that backporting the 1.3.28
> > support into 1.3.1 is a bit scary, at least for Ruby. Shouldn't we
> > either disallow Ruby support for Swig 1.3.28 or do something about the
> > test failure?
>
> I fixed the problem in r18924.
>
Hi Kou,
I was trying to review r18924 so that I could approve it for backport to
1.3.2, but I'm not sure I understand what it's doing.
The relevant change was in svn_swig_rb_get_pool(), which (as far as I
can infer) is responsible for converting a (possibly implicitly created)
Ruby pool object to an apr_pool_t*.
The relevant parts of the function look like this:
354 void
355 svn_swig_rb_get_pool(int argc, VALUE *argv, VALUE self,
356 VALUE *rb_pool, apr_pool_t **pool)
357 {
358 VALUE target;
359 apr_pool_wrapper_t *pool_wrapper;
360 apr_pool_wrapper_t **pool_wrapper_p;
...
*rb_pool = {various}
...
389
390 pool_wrapper_p = &pool_wrapper;
391 SWIG_ConvertPtr(*rb_pool, (void **)pool_wrapper_p,
392 SWIG_TypeQuery("apr_pool_wrapper_t *"), 0);
393 *pool = pool_wrapper->pool;
394 }
So the change you made in r18924 was just to change the 'flags' parameter
passed to SWIG_ConvertPtr() from non-zero to zero. That parameter is
explained as follows in the SWIG documentation:
int SWIG_ConvertPtr(VALUE obj, void **ptr, swig_type_info *ty, int flags)
Converts a Ruby object obj to a C pointer whose address is ptr (i.e. ptr
is a pointer to a pointer). The third argument, ty, is a pointer to a
SWIG type descriptor structure. If ty is not NULL, that type information
is used to validate type compatibility and other aspects of the type
conversion. If flags is non-zero, any type errors encountered during
this validation result in a Ruby TypeError exception being raised;
if flags is zero, such type errors will cause SWIG_ConvertPtr() to
return -1 but not raise an exception. If ty is NULL, no type-checking
is performed.
So by changing 'flags' to zero, we change the behaviour of
SWIG_ConvertPtr() when passed a Ruby object that's not a Ruby pool,
from raising an exception (of some sort; is this like a C++ exception
that skips the remainder of the function?) to just returning -1. As we
don't check the return value, we'll essentially ignore the problem.
My questions are:
* What guarantee do we have that pool_wrapper is dereferencable at all,
when we aren't checking the return value of the function that sets it?
I don't see any guarantee that SWIG_ConvertPtr() ensures that the
output pointer is set to anything valid on error.
* What problem is this fixing? The log message says "Support SWIG 1.3.28.
Keep owning pool.", but apart from fixing the Ruby test failures
(somehow), I don't understand what this change is trying to achieve.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Fri, Mar 17, 2006 at 03:16:38PM +0900, Kouhei Sutou wrote:
> > OK, thanks for the answer. This makes me think that backporting the 1.3.28
> > support into 1.3.1 is a bit scary, at least for Ruby. Shouldn't we
> > either disallow Ruby support for Swig 1.3.28 or do something about the
> > test failure?
>
> I fixed the problem in r18924.
>
Presumably that should be nominated for 1.3.2 along with the rest of
the r17280 group?
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
I'm sorry for my late response.
In <17...@localhost.localdomain>
"Re: Ruby test failure" on Mon, 13 Mar 2006 09:23:09 +0100,
"Peter N. Lundblad" <pe...@famlundblad.se> wrote:
> OK, thanks for the answer. This makes me think that backporting the 1.3.28
> support into 1.3.1 is a bit scary, at least for Ruby. Shouldn't we
> either disallow Ruby support for Swig 1.3.28 or do something about the
> test failure?
I fixed the problem in r18924.
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by Peter Samuelson <pe...@p12n.org>.
[Peter N. Lundblad]
> David James writes:
> OK, thanks for the answer. This makes me think that backporting the 1.3.28
> support into 1.3.1 is a bit scary, at least for Ruby.
I just got a bug report against the Perl bindings, right after
releasing a Debian build of 1.3.0 with swig 1.3.28 (details at
http://bugs.debian.org/356894):
> TypeError in method 'svn_ra_reporter2_invoke_set_path', argument 6 of type 'char const *'
If this too is swig-related -- and I'm not certain it is, I haven't
investigated yet -- that's another reason to be wary of swig 1.3.28.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
David James writes:
> On 3/12/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
[...]
> > /home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/test_wc.rb:331:in `test_locked']:
> > <false> is not true.
> >
> > 119 tests, 616 assertions, 1 failures, 0 errors
> >
> It is a known problem with SWIG 1.3.28 and the Ruby bindings, I have
> always gotten the same error. We have not looked into what causes it.
>
OK, thanks for the answer. This makes me think that backporting the 1.3.28
support into 1.3.1 is a bit scary, at least for Ruby. Shouldn't we
either disallow Ruby support for Swig 1.3.28 or do something about the
test failure?
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Ruby test failure
Posted by David James <dj...@collab.net>.
On 3/12/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> Hi,
>
> I wanted to vote for the r17280 backport group in 1.3.x/STATUS, but got a Ruby
> test failure.
>
> I am using SWIG 1.3.28 and Ruby 1.8.4 on Debian testing.
>
> I checked out a fresh WC, merged the integration branch and ran:
> make all swig-py swig-pl swig-rb
> make check-swig-pl check-swig-py check-swig-rb
>
> The last target gave the following output:
> cd /home/pl/src/svn/13x/subversion/bindings/swig/ruby; \
> /usr/bin/ruby -I /home/pl/src/svn/13x/subversion/bindings/swig/ruby \
> /home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/run-test.rb \
> --verbose=normal
> Loaded suite test
> Started
> ................................................................................................................F......
> Finished in 299.491669 seconds.
>
> 1) Failure:
> test_locked(SvnWcTest)
> [/home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/test_wc.rb:333:in `test_locked'
> /home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/util.rb:55:in `change_gc_status'
> /home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/util.rb:70:in `gc_enable'
> /home/pl/src/svn/13x/subversion/bindings/swig/ruby/test/test_wc.rb:331:in `test_locked']:
> <false> is not true.
>
> 119 tests, 616 assertions, 1 failures, 0 errors
>
> The tests all pass using SWIG 1.3.25. Is this a known problem, or does it
> look like something that could be wrong on my system?
It is a known problem with SWIG 1.3.28 and the Ruby bindings, I have
always gotten the same error. We have not looked into what causes it.
Cheers,
David
--
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org