In the ctypes-python bindings, when I try to use the Stream class
(either via the RemoteRepository.cat() method, or just on its own), I
find that the program crashes at the moment the C code in
svn_stream_write() calls the callback function which is provided by
subversion/bindings/ctypes-python/csvn/types.py:240:

> class Stream(object):
> 
>     def __init__(self, buffer, disown=False):
>         [...]
>         svn_stream_set_read(self.stream, svn_read_fn_t(self._read))
>     
>     def _write(self, baton, data, l):
>         [...]

So, using the attached patch (with or without the experimental edits it
contains in _read() and _write()), I find the tests suite bombs out
without reporting any results.  My usage of GDB and the Python debugger
pdb indicates that _write() is never reached.

If I write a test using a ctypes callback type with the C standard
library's qsort(), as shown in the Python ctypes module documentation
<http://docs.python.org/library/ctypes.html#callback-functions>, that
works fine for me.

I wonder if something is wrong with my building and linking of ctypesgen
or the ctypes-python bindings, but on the other hand the obvious bug in
Stream._read() where 'string' is referenced instead of 's' (see my
patch) makes me wonder if this class has ever worked.

Another data point: the log message receiver callback seems to work fine
for me, as tested in
subversion/bindings/ctypes-python/test/remoterepos.py:test_log().

- Julian

In the ctypes-python bindings, fix the Stream class and add tests and doc
strings.
### This is just an attempt, and not yet correct.

* subversion/bindings/ctypes-python/csvn/types.py
  (Stream._read): This was totally broken: at the very least it should say
    's' instead of 'string'. Try using plain Python code instead of
    'memmove'; this might be totally wrong. Another guess: maybe it should
    be 'memmove(buffer.raw, create_string_buffer(s, len(s)), len(s))'.
  (Stream._write): Try using plain Python code instead of special ctypes
    code, for symmetry and simplicity; this might be totally wrong.
  (Stream): Add doc strings to the methods, as their argument types are a
    little unclear, especially 'buffer' which has two different meanings.

* subversion/bindings/ctypes-python/test/svntypes.py
  (StreamTestCase): New test case class containing one simple test.
  (suite): Include it.

* subversion/bindings/ctypes-python/test/remoterepos.py
  (RemoteRepositoryTestCase): Add a test for 'cat'.
  (suite): ### Run it instead of the normal tests, just for debugging.
--This line, and those below, will be ignored--

Index: subversion/bindings/ctypes-python/csvn/types.py
===================================================================
--- subversion/bindings/ctypes-python/csvn/types.py	(revision 1157788)
+++ subversion/bindings/ctypes-python/csvn/types.py	(working copy)
@@ -246,13 +262,13 @@ class Stream(object):
 
     def _read(self, baton, buffer, l):
         s = self.buffer.read(l[0])
-        memmove(buffer, string, len(s))
+        buffer.data = s
         l[0] = len(s)
         return SVN_NO_ERROR
 
     def _write(self, baton, data, l):
-        s = string_at(data.raw, l[0])
-        self.buffer.write(s)
+        s = str(data)
+        self.buffer.write(s[:l[0]])
         return SVN_NO_ERROR
 
     def _close(self, baton):
Index: subversion/bindings/ctypes-python/csvn/types.py
===================================================================
--- subversion/bindings/ctypes-python/csvn/types.py	(revision 1157734)
+++ subversion/bindings/ctypes-python/csvn/types.py	(working copy)
@@ -232,7 +232,9 @@ class APRFile(object):
 class Stream(object):
 
     def __init__(self, buffer, disown=False):
-        """Create a stream which wraps a Python file or file-like object"""
+        """Create a stream which wraps the Python file or file-like object
+           BUFFER. If DISOWN is true, closing this stream won't close
+           BUFFER."""
 
         self.pool = Pool()
         self.buffer = buffer
@@ -245,17 +247,23 @@ class Stream(object):
     _as_parameter_ = property(fget=lambda self: self.stream)
 
     def _read(self, baton, buffer, l):
+        """Read (*L) bytes from self.buffer into BUFFER. Implements svn_read_fn_t.
+           BATON is unused; BUFFER is a String; L is a POINTER(apr_size_t)."""
         s = self.buffer.read(l[0])
         memmove(buffer, string, len(s))
         l[0] = len(s)
         return SVN_NO_ERROR
 
     def _write(self, baton, data, l):
+        """Write (*L) bytes into self.buffer from DATA. Implements svn_write_fn_t.
+           BATON is unused; DATA is a String; L is a POINTER(apr_size_t)."""
         s = string_at(data.raw, l[0])
         self.buffer.write(s)
         return SVN_NO_ERROR
 
     def _close(self, baton):
+        """Close this stream and the underlying file-like object
+           'self.buffer'. Implements svn_close_fn_t."""
         self.buffer.close()
         return SVN_NO_ERROR
 
Index: subversion/bindings/ctypes-python/test/svntypes.py
===================================================================
--- subversion/bindings/ctypes-python/test/svntypes.py	(revision 1157904)
+++ subversion/bindings/ctypes-python/test/svntypes.py	(working copy)
@@ -23,6 +23,15 @@ from csvn.core import *
 import csvn.types as _types
 from csvn.types import SvnDate, Hash, Array, APRFile, Stream, SvnStringPtr
 
+from sys import version_info # For Python version check
+if version_info[0] >= 3:
+  # Python >=3.0
+  from io import StringIO
+else:
+  # Python <3.0
+  from StringIO import StringIO
+
+
 class SvnDateTestCase(unittest.TestCase):
 
     def test_as_apr_time_t(self):
@@ -86,11 +95,23 @@ class ArrayTestCase(unittest.TestCase):
         self.assertEqual(self.svnarray[2], "vici")
         self.assertEqual(self.svnarray[1], "vidi")
 
+class StreamTestCase(unittest.TestCase):
+
+    def test_stream(self):
+        buf = StringIO()
+        s = Stream(buf, disown=True)
+        l = apr_size_t(5)
+        svn_stream_write(s, 'Hello', l)
+        self.assertEqual(l, 5)
+        buf.seek(0)
+        self.assertEqual(buf.read(), 'Hello')
+
 def suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(SvnDateTestCase, 'test'))
     suite.addTest(unittest.makeSuite(HashTestCase, 'test'))
     suite.addTest(unittest.makeSuite(ArrayTestCase, 'test'))
+    suite.addTest(unittest.makeSuite(StreamTestCase, 'test'))
     return suite
 
 if __name__ == '__main__':
Index: subversion/bindings/ctypes-python/test/remoterepos.py
===================================================================
--- subversion/bindings/ctypes-python/test/remoterepos.py	(revision 1157904)
+++ subversion/bindings/ctypes-python/test/remoterepos.py	(working copy)
@@ -106,6 +106,15 @@ class RemoteRepositoryTestCase(unittest.
         for path in found:
             self.svn_dirent_t_assert_equal(found[path], expected[path])
 
+    ### This test does not merely fail, it bombs right out of the test suite
+    ### at the point where the C svn_stream_write() calls the Python
+    ### callback.
+    def zzz_test_cat(self):
+        buf = StringIO()
+        self.repos.cat(buf, "trunk/README.txt")
+        buf.seek(0)
+        self.assertEqual(buf.read(), "")
+
     def test_info(self):
         e = svn_dirent_t(kind=svn_node_file, size=159, has_props=True,
                          created_rev=9, last_author=String('bruce'))
@@ -203,7 +212,7 @@ class RemoteRepositoryTestCase(unittest.
         self.assertEqual(commit_info.revision, 10)
 
 def suite():
-    return unittest.makeSuite(RemoteRepositoryTestCase, 'test')
+    return unittest.makeSuite(RemoteRepositoryTestCase, 'zzz_test')
 
 if __name__ == '__main__':
     runner = unittest.TextTestRunner()

Reply via email to