Further refinement/discussion and the addition of some reference count 
management code has resulted in another version of this patch.
[[[
Add necessary plumbing to Python bindings to allow the callbacks defined 
in svn_client_ctx_t to function.
[ In subversion/bindings/swig ]
* include/proxy_apr.swg:
   (apr_pool_t::_add_owned_ref, apr_pool_t::_remove_owned_ref): New
   pool "owned reference" management functions.
   (apr_pool_t::destroy): Now clears all "owned reference" items.
* python/libsvn_swig_py/swigutil_py.c:
   (proxy_get_pool): New static function for getting the parent pool of
   a proxy object.  Logic extracted from svn_swig_MustGetPtr.
   (svn_swig_py_pool_set_owned_ref): New helper function to manage pool
   "owned references."
   (svn_swig_MustGetPtr): Extracted proxy_get_pool function from here.
* include/svn_types.swg:
   (PY_AS_VOID): New typemap for converting PyObjects into void* batons.
   Uses svn_swig_py_pool_set_owned_ref helper to manage reference counts.
* svn_client.i:
   Mark svn_client_ctx_t baton members as PY_AS_VOID
   (svn_swig_py_cancel_func): new opaque pointer constant
   (svn_swig_py_get_commit_log_func): new opaque pointer constant
   (svn_swig_py_notify_func): new opaque pointer constant
* python/tests/client.py:
   Fixed up doc comments throughout.
   (SubversionRepositoryTestCase): Renamed test case class to
   SubversionClientTestCase (original name is a probable typo)
   (test_mkdir_url, test_log3_url): New tests.
   (test_client_ctx_baton_lifetime): New test for PY_AS_VOID reference
   counting semantics.
]]]
Third time's the charm? ;)
-Walter
Index: subversion/bindings/swig/python/tests/client.py
===================================================================
--- subversion/bindings/swig/python/tests/client.py	(revision 19841)
+++ subversion/bindings/swig/python/tests/client.py	(working copy)
@@ -1,17 +1,35 @@
-import unittest, os
+import unittest, os, weakref
 
 from svn import core, repos, fs, delta, client
 
 from trac.versioncontrol.tests.svn_fs import SubversionRepositoryTestSetup, \
   REPOS_PATH
 from urllib import pathname2url
+from urlparse import urljoin
 
-class SubversionRepositoryTestCase(unittest.TestCase):
-  """Test cases for the Subversion repository layer"""
+class SubversionClientTestCase(unittest.TestCase):
+  """Test cases for the basic SWIG Subversion client layer"""
 
+  def log_message_func(self, items, pool):
+    """ Simple log message provider for unit tests. """
+    self.log_message_func_calls += 1
+    return "Test log message"
+
+  def log_receiver(self, changed_paths, revision, author, date, message, pool):
+    """ Function to recieve log messages retrieved by client.log3(). """
+    self.log_message = message
+    self.change_author = author
+    self.changed_paths = changed_paths
+
   def setUp(self):
-    """Load a Subversion repository"""
+    """Set up authentication and client context"""
     self.client_ctx = client.svn_client_create_context()
+    self.client_ctx.log_msg_func2 = client.svn_swig_py_get_commit_log_func
+    self.client_ctx.log_msg_baton2 = self.log_message_func
+    self.log_message_func_calls = 0
+    self.log_message = None
+    self.changed_paths = None
+    self.change_auther = None
 
     providers = [
        client.svn_client_get_simple_provider(),
@@ -19,6 +37,7 @@
     ]
 
     self.client_ctx.auth_baton = core.svn_auth_open(providers)
+    self.repos_url = "file://" + pathname2url(REPOS_PATH)
 
   def info_receiver(self, path, info, pool):
     """Squirrel away the output from 'svn info' so that the unit tests
@@ -26,28 +45,70 @@
     self.path = path
     self.info = info
 
+  def test_client_ctx_baton_lifetime(self):
+    pool = core.Pool()
+    temp_client_ctx = client.svn_client_create_context(pool)
+    # this is tricky because you can't get a PyObject back out of a PY_AS_VOID field
+    test_object1 = lambda *args: "message 1"
+    test_object2 = lambda *args: "message 2"
+    
+    temp_client_ctx.log_msg_baton2 = test_object1
+    
+    test_object1 = weakref.ref(test_object1)
+    self.assertNotEqual(test_object1(), None)
+    temp_client_ctx.log_msg_baton2 = test_object2
+    self.assertEqual(test_object1(), None)
+  
+    test_object2 = weakref.ref(test_object2)
+    self.assertNotEqual(test_object2(), None)
+    pool.destroy()
+    self.assertEqual(test_object2(), None)
+
   def test_info(self):
-    """Test scope of get_logs callbacks"""
+    """Test svn_client_info on an empty repository"""
 
     # Run info
     revt = core.svn_opt_revision_t()
     revt.kind = core.svn_opt_revision_head
-    repos_url = "file://" + pathname2url(REPOS_PATH)
-    client.info(repos_url, revt, revt, self.info_receiver,
+    client.info(self.repos_url, revt, revt, self.info_receiver,
                 False, self.client_ctx)
 
     # Check output from running info. This also serves to verify that
     # the internal 'info' object is still valid
     self.assertEqual(self.path, os.path.basename(REPOS_PATH))
     self.info.assert_valid()
-    self.assertEqual(self.info.URL, repos_url)
-    self.assertEqual(self.info.repos_root_URL, repos_url)
+    self.assertEqual(self.info.URL, self.repos_url)
+    self.assertEqual(self.info.repos_root_URL, self.repos_url)
 
+  def test_mkdir_url(self):
+    """Test svn_client_mkdir2 on a file:// URL"""
+    dir = urljoin(self.repos_url+"/", "dir1")
+    
+    commit_info = client.mkdir2((dir,), self.client_ctx)
+    self.assertEqual(commit_info.revision, 13)
+    self.assertEqual(self.log_message_func_calls, 1)
 
+  def test_log3_url(self):
+    """Test svn_client_log3 on a file:// URL"""
+    dir = urljoin(self.repos_url+"/", "trunk/dir1")
+
+    start = core.svn_opt_revision_t()
+    end = core.svn_opt_revision_t()
+    core.svn_opt_parse_revision(start, end, "4:0")
+    client.log3((dir,), start, start, end, 1, True, False, self.log_receiver,
+        self.client_ctx)
+    self.assertEqual(self.change_author, "john")
+    self.assertEqual(self.log_message, "More directories.")    
+    self.assertEqual(len(self.changed_paths), 3)
+    for dir in ('/trunk/dir1', '/trunk/dir2', '/trunk/dir3'):
+      self.assertTrue(self.changed_paths.has_key(dir))
+      self.assertEqual(self.changed_paths[dir].action, 'A')
+
 def suite():
-    return unittest.makeSuite(SubversionRepositoryTestCase, 'test',
+    return unittest.makeSuite(SubversionClientTestCase, 'test',
                               suiteClass=SubversionRepositoryTestSetup)
 
 if __name__ == '__main__':
     runner = unittest.TextTestRunner()
     runner.run(suite())
+
Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
===================================================================
--- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c	(revision 19841)
+++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c	(working copy)
@@ -104,6 +104,8 @@
 static PyObject *_global_svn_swig_py_pool = NULL;
 static char assertValid[] = "assert_valid";
 static char parentPool[] = "_parent_pool";
+static char addOwnedRef[] = "_add_owned_ref";
+static char removeOwnedRef[] = "_remove_owned_ref";
 static char wrap[] = "_wrap";
 static char unwrap[] = "_unwrap";
 static char setParentPool[] = "set_parent_pool";
@@ -166,6 +168,47 @@
 
   return 0;
 }
+
+/* Get the parent pool of a proxy object, or return the global application
+ * pool if one is not set.  Returns a BORROWED reference! */
+static PyObject *proxy_get_pool(PyObject *proxy)
+{
+  PyObject *result;
+  if (PyObject_HasAttrString(proxy, parentPool)) {
+    result = PyObject_GetAttrString(proxy, parentPool);
+    Py_DECREF(result);
+  } else {
+    result = _global_svn_swig_py_pool;
+  }
+  return result;
+}
+
+/* Change an 'owned reference' allocated in a pool from oldRef to newRef.
+ * If oldRef is non-NULL and present in the parent pool of proxy, it is removed.
+ */
+int svn_swig_py_pool_set_owned_ref(PyObject *proxy, PyObject *oldRef, PyObject *newRef)
+{
+  PyObject *temp;
+  PyObject *py_pool = proxy_get_pool(proxy);
+  
+  if (oldRef != NULL) {
+    temp = PyObject_CallMethod(py_pool, removeOwnedRef, objectTuple, oldRef);
+    if (temp == NULL) {
+      return 1;
+    } else {
+      Py_DECREF(temp);
+    }
+  }
+  if (newRef != NULL) {
+    temp = PyObject_CallMethod(py_pool, addOwnedRef, objectTuple, newRef);
+    if (temp == NULL) {
+      return 1;
+    } else {
+      Py_DECREF(temp);
+    }
+  }
+  return 0;
+}
 
 /* Wrapper for SWIG_TypeQuery */
 #define svn_swig_TypeQuery(x) SWIG_TypeQuery(x)
@@ -240,12 +283,7 @@
     Py_DECREF(result);
   }
   if (py_pool != NULL) {
-    if (PyObject_HasAttrString(input, parentPool)) {
-      *py_pool = PyObject_GetAttrString(input, parentPool);
-      Py_DECREF(*py_pool);
-    } else {
-      *py_pool = _global_svn_swig_py_pool;
-    }
+    *py_pool = proxy_get_pool((PyObject *) input);
   }
   if (PyObject_HasAttrString(input, unwrap)) {
     input = PyObject_CallMethod(input, unwrap, emptyTuple);
Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
===================================================================
--- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h	(revision 19841)
+++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h	(working copy)
@@ -79,6 +79,9 @@
 SVN_SWIG_SWIGUTIL_EXPORT
 void svn_swig_get_application_pool(PyObject **py_pool, apr_pool_t **pool);
 
+/* Set a Python 'owned' reference on the pool of the given proxy object */
+int svn_swig_py_pool_set_owned_ref(PyObject *proxy, PyObject *oldRef, PyObject *newRef);
+
 
 /*** SWIG Wrappers ***/
 
Index: subversion/bindings/swig/include/proxy_apr.swg
===================================================================
--- subversion/bindings/swig/include/proxy_apr.swg	(revision 19841)
+++ subversion/bindings/swig/include/proxy_apr.swg	(working copy)
@@ -159,6 +159,28 @@
           del self._parent_pool
         if hasattr(self, "_is_valid"):
           del self._is_valid
+        
+        # Clear out any pool-owned references inserted by typemaps
+        if hasattr(self, "_owned_refs"):
+          del self._owned_refs
+
+      def _add_owned_ref(self, ref):
+        """Add a new 'owned' reference -- i.e. a Python object contained in a C
+           structure allocated in this pool.  Used by the typemaps to manage
+           reference counting semantics."""
+        if not hasattr(self, "_owned_refs"):
+          self._owned_refs = {}
+        if self._owned_refs.has_key(ref):
+          self._owned_refs[ref] += 1
+        else:
+          self._owned_refs[ref] = 1
+      
+      def _remove_owned_ref(self, ref):
+        """Remove an existing 'owned' reference.  Also used by typemaps."""
+        if hasattr(self, "_owned_refs") and self._owned_refs.has_key(ref):
+          self._owned_refs[ref] -= 1
+          if self._owned_refs[ref] == 0:
+            del self._owned_refs[ref]
   
       def __del__(self):
         """Automatically destroy memory pools, if necessary"""
Index: subversion/bindings/swig/include/svn_types.swg
===================================================================
--- subversion/bindings/swig/include/svn_types.swg	(revision 19841)
+++ subversion/bindings/swig/include/svn_types.swg	(working copy)
@@ -526,6 +526,25 @@
 }
 
 /* -----------------------------------------------------------------------
+   Mapper to automatically turn Python objects into void* batons on assignment
+*/
+
+#ifdef SWIGPYTHON
+%typemap(in) void *PY_AS_VOID (PyObject *newRef) {
+  newRef = $input;
+  if ($input == Py_None) {
+    $1 = newRef = NULL;
+  } else {
+    newRef = $input;
+    $1 = (void *)$input;
+  }
+  if (svn_swig_py_pool_set_owned_ref(obj0, (PyObject *)arg1->$1_name, newRef)) {
+    SWIG_fail;
+  }
+}
+#endif
+
+/* -----------------------------------------------------------------------
    Wrap the digest output for functions populating digests.
 */
 
Index: subversion/bindings/swig/svn_client.i
===================================================================
--- subversion/bindings/swig/svn_client.i	(revision 19841)
+++ subversion/bindings/swig/svn_client.i	(working copy)
@@ -71,6 +71,16 @@
 
 #ifdef SWIGPYTHON
 %apply svn_stream_t *WRAPPED_STREAM { svn_stream_t * };
+
+/* members of svn_client_ctx_t */
+%apply void *PY_AS_VOID {
+    void *notify_baton,
+    void *log_msg_baton,
+    void *cancel_baton,
+    void *notify_baton2,
+    void *log_msg_baton2,
+    void *progress_baton
+};
 #endif
 
 /* -----------------------------------------------------------------------
@@ -498,6 +508,15 @@
 %include svn_time_h.swg
 %include svn_client_h.swg
 
+#ifdef SWIGPYTHON
+
+/* provide Python with access to some thunks. */
+%constant svn_cancel_func_t svn_swig_py_cancel_func;
+%constant svn_client_get_commit_log2_t svn_swig_py_get_commit_log_func;
+%constant svn_wc_notify_func2_t svn_swig_py_notify_func;
+
+#endif
+
 #ifdef SWIGRUBY
 %inline %{
 static VALUE
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jun  4 14:45:33 2006