[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Mismatched backwards compat callbacks in libsvn_wc

From: Hyrum K. Wright <hyrum_at_hyrumwright.org>
Date: Thu, 2 Jul 2009 10:44:44 -0500

On Jul 1, 2009, at 6:26 PM, Роман Донченко wrote:

> Роман Донченко <DXDragon_at_yandex.ru> писал в
> своём письме Sat, 27 Jun 2009
> 20:27:25 +0400:
>
>> The immediate fix is obviously to write new wrappers for (3) and
>> put them
>> in callbacks3_wrapper.
>
> The immediate fix is attached. It's rather trivial. 8=] Also of note
> is
> that it fixes the Python test I was struggling with.
>
> [[[
> Create proper wrapper functions for svn_wc_diff_callbacks3_t and use
> them.
> Also change the misleading comments on the old wrappers.
>
> * subversion/libsvn_wc/deprecated.c
> (file_changed, file_deleted, dir_deleted, dir_props_changed,
> dir_opened,
> dir_closed): Relabel as svn_wc_diff_callbacks3_t functions, since
> that's
> what they're used as.
> (file_changed3, file_deleted3, dir_deleted3, dir_props_changed3,
> dir_opened2, dir_closed2): New functions.
> (callbacks3_wrapper): Use the new functions above.
> ]]]

I applied the patch locally, and the python tests no longer segfault.
Thanks very much for tracking this down! With the patch, I now get an
error in the tests, but not a crash:

hwright_at_orac:~/dev/svn-trunk$ make check-swig-py
cd /home/hwright/dev/svn-trunk/subversion/bindings/swig/python; \
          /usr/bin/python /home/hwright/dev/svn-trunk/subversion/bindings/
swig/python/tests/run_all.py
/home/hwright/dev/svn-trunk/subversion/bindings/swig/python/svn/
core.py:47: DeprecationWarning: BaseException.message has been
deprecated as of Python 2.6
   self.message = message
/home/hwright/dev/svn-trunk/subversion/bindings/swig/python/tests/
core.py:17: DeprecationWarning: BaseException.message has been
deprecated as of Python 2.6
   self.assertEqual(svn.core.SubversionException('error message',
1).message,
...............................................................E
..........................................
======================================================================
ERROR: test_lock (wc.SubversionWorkingCopyTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/home/hwright/dev/svn-trunk/subversion/bindings/swig/python/
tests/wc.py", line 48, in test_lock
     lock = wc.add_lock(self.path, core.svn_lock_create(core.Pool()),
self.wc)
   File "/home/hwright/dev/svn-trunk/subversion/bindings/swig/python/
libsvn/wc.py", line 2814, in svn_wc_add_lock
     return apply(_wc.svn_wc_add_lock, args)
SubversionException: ('traced call', 200030)

----------------------------------------------------------------------
Ran 106 tests in 55.597s

FAILED (errors=1)
make: *** [check-swig-py] Error 1

(Note that that this doesn't *not* solve the segfault in the ruby
tests.)

Committed to trunk in r38307.

>> B. The naming scheme for wrappers is, well, poor. First, the names
>> don't
>> contain an indication of "wrapperness", so the purpose isn't
>> immediately
>> obvious; plus one may confuse, say, (1)::file_changed with
>> ::file_changed.
>> Second, the names don't convey which generation of callbacks are they
>> wrapping to and from. Combined with A, this makes the names quite
>> unhelpful.
>
> I'd also post a patch renaming the wrappers to something more
> telling — if
> someone approves.

Any improvements to documentation, including function naming are
definitely encouraged. Feel free to do a followup patch.

-Hyrum

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2367413
Received on 2009-07-02 17:45:07 CEST

This is an archived mail posted to the Subversion Dev mailing list.