Hi all, Please review/comment.
[[[ Implement Python bindings for dump stream parser. * subversion/bindings/swig/include/svn_types.swg (): Fix a typo in svn_repos_parser_fns2_t type name, add svn_repos_parser_fns2_t type to the argout typemap. * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (item_baton): Add pool; needed for certain methods in dump stream parser that do not take pool argument. (make_baton): Increment refcount on `editor'; remove incorrect the comment stating the current usage is safe. Save pool pointer. (close_baton): Decrement refcount on `editor'. (svn_swig_py_make_parse_fns3): New function; creates vtable/baton to be used in a later call to repos.parse_dumpstream3() * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h: (svn_swig_py_make_parse_fns3): New function. * subversion/bindings/swig/python/svn/core.py (Stream.close): Guard against double-close. * subversion/bindings/swig/python/svn/repos.py (ParseFns3): New class; vtable for dump stream parser. * subversion/bindings/swig/python/tests/repository.py (test_parse_fns3): Test for dump stream parser bindings. (test_unnamed_editor): Test delta.Editor usage where the interpreter does not have a named variable for the instance and thus does not hold a reference for it. Fails without the make_baton/close_baton fix in swigutil_py.c. * subversion/bindings/swig/svn_repos.i (svn_swig_py_make_parse_fns3): Wrap it. ]]] Regards, Alexey.
Index: subversion/bindings/swig/include/svn_types.swg =================================================================== --- subversion/bindings/swig/include/svn_types.swg (revision 1642639) +++ subversion/bindings/swig/include/svn_types.swg (working copy) @@ -146,7 +146,8 @@ /* svn_repos */ svn_authz_t **, svn_repos_t **, - const svn_repos_parse_fns2_t **, + const svn_repos_parse_fns3_t **, + const svn_repos_parser_fns2_t **, const svn_repos_parser_fns_t **, void **parse_baton, void **revision_baton, Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c =================================================================== --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (revision 1642639) +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (working copy) @@ -1540,6 +1540,7 @@ typedef struct item_baton { PyObject *editor; /* the editor handling the callbacks */ PyObject *baton; /* the dir/file baton (or NULL for edit baton) */ + apr_pool_t *pool; /* top-level pool */ } item_baton; static item_baton *make_baton(apr_pool_t *pool, @@ -1548,13 +1549,11 @@ { item_baton *newb = apr_palloc(pool, sizeof(*newb)); - /* Note: We steal the caller's reference to 'baton'. Also, to avoid - memory leaks, we borrow the caller's reference to 'editor'. In this - case, borrowing the reference to 'editor' is safe because the contents - of an item_baton struct are only used by function calls which operate on - the editor itself. */ + /* Note: We steal the caller's reference to 'baton'. */ + Py_INCREF(editor); newb->editor = editor; newb->baton = baton; + newb->pool = pool; return newb; } @@ -1583,6 +1582,9 @@ /* there is no return value, so just toss this object (probably Py_None) */ Py_DECREF(result); + /* Release the editor object */ + Py_DECREF(ib->editor); + /* We're now done with the baton. Since there isn't really a free, all we need to do is note that its objects are no longer referenced by the baton. */ @@ -2044,7 +2046,374 @@ *edit_baton = make_baton(pool, py_editor, NULL); } + +/* Wrappers for dump stream parser */ +static svn_error_t *parse_fn3_magic_header_record(int version, + void *parse_baton, + apr_pool_t *pool) +{ + item_baton *ib = parse_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"magic_header_record", + (char *)"lO&", version, + make_ob_pool, pool)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* there is no return value, so just toss this object (probably Py_None) */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_uuid_record(const char *uuid, + void *parse_baton, + apr_pool_t *pool) +{ + item_baton *ib = parse_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"uuid_record", + (char *)"sO&", uuid, + make_ob_pool, pool)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* there is no return value, so just toss this object (probably Py_None) */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_new_revision_record(void **revision_baton, + apr_hash_t *headers, + void *parse_baton, + apr_pool_t *pool) +{ + item_baton *ib = parse_baton; + PyObject *result; + PyObject *tmp; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + if ((result = PyObject_CallMethod(ib->editor, (char *)"new_revision_record", + (char *)"O&O&", + svn_swig_py_stringhash_to_dict, headers, + make_ob_pool, pool)) == NULL) { + err = callback_exception_error(); + goto finished; + } + + /* make_baton takes our 'result' reference */ + *revision_baton = make_baton(pool, ib->editor, result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_new_node_record(void **node_baton, + apr_hash_t *headers, + void *revision_baton, + apr_pool_t *pool) +{ + item_baton *ib = revision_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + if ((result = PyObject_CallMethod(ib->editor, (char *)"new_node_record", + (char *)"O&OO&", + svn_swig_py_stringhash_to_dict, headers, + ib->baton, + make_ob_pool, pool)) == NULL) { + err = callback_exception_error(); + goto finished; + } + + /* make_baton takes our 'result' reference */ + *node_baton = make_baton(pool, ib->editor, result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_set_revision_property(void *revision_baton, + const char *name, + const svn_string_t *value) +{ + item_baton *ib = revision_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"set_revision_property", + (char *)"Oss#", ib->baton, name, + value ? value->data : NULL, + value ? value->len : 0)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* there is no return value, so just toss this object (probably Py_None) */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_set_node_property(void *node_baton, + const char *name, + const svn_string_t *value) +{ + item_baton *ib = node_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"set_node_property", + (char *)"Oss#", ib->baton, name, + value ? value->data : NULL, + value ? value->len : 0)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* there is no return value, so just toss this object (probably Py_None) */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_delete_node_property(void *node_baton, + const char *name) +{ + item_baton *ib = node_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"delete_node_property", + (char *)"Os", ib->baton, name)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* there is no return value, so just toss this object (probably Py_None) */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_remove_node_props(void *node_baton) +{ + item_baton *ib = node_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"remove_node_props", + (char *)"(O)", ib->baton)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* there is no return value, so just toss this object (probably Py_None) */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_set_fulltext(svn_stream_t **stream, + void *node_baton) +{ + item_baton *ib = node_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"set_fulltext", + (char *)"(O)", ib->baton)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* Interpret None to mean NULL - no text is desired */ + if (result == Py_None) + { + *stream = NULL; + } + else + { + /* create a stream from the IO object. it will increment the + reference on the 'result'. */ + *stream = svn_swig_py_make_stream(result, ib->pool); + } + + /* if the handler returned an IO object, svn_swig_py_make_stream() has + incremented its reference counter. If it was None, it is discarded. */ + Py_DECREF(result); + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_apply_textdelta(svn_txdelta_window_handler_t *handler, + void **handler_baton, + void *node_baton) +{ + item_baton *ib = node_baton; + PyObject *result; + svn_error_t *err; + + svn_swig_py_acquire_py_lock(); + + /* ### python doesn't have 'const' on the method name and format */ + if ((result = PyObject_CallMethod(ib->editor, (char *)"apply_textdelta", + (char *)"(O)", ib->baton)) == NULL) + { + err = callback_exception_error(); + goto finished; + } + + /* Interpret None to mean svn_delta_noop_window_handler. This is much + easier/faster than making code always have to write a NOOP handler + in Python. */ + if (result == Py_None) + { + Py_DECREF(result); + + *handler = svn_delta_noop_window_handler; + *handler_baton = NULL; + } + else + { + /* return the thunk for invoking the handler. the baton takes our + 'result' reference, which is the handler. */ + *handler = window_handler; + *handler_baton = result; + } + + err = SVN_NO_ERROR; + + finished: + svn_swig_py_release_py_lock(); + return err; +} + + +static svn_error_t *parse_fn3_close_node(void *node_baton) +{ + return close_baton(node_baton, "close_node"); +} + + +static svn_error_t *parse_fn3_close_revision(void *revision_baton) +{ + return close_baton(revision_baton, "close_revision"); +} + + +static const svn_repos_parse_fns3_t thunk_parse_fns3_vtable = + { + parse_fn3_magic_header_record, + parse_fn3_uuid_record, + parse_fn3_new_revision_record, + parse_fn3_new_node_record, + parse_fn3_set_revision_property, + parse_fn3_set_node_property, + parse_fn3_delete_node_property, + parse_fn3_remove_node_props, + parse_fn3_set_fulltext, + parse_fn3_apply_textdelta, + parse_fn3_close_node, + parse_fn3_close_revision + }; + +static apr_status_t +svn_swig_py_parse_fns3_destroy(void *parse_baton) +{ + close_baton(parse_baton, "_close_dumpstream"); + return APR_SUCCESS; +} + +void svn_swig_py_make_parse_fns3(const svn_repos_parse_fns3_t **parse_fns3, + void **parse_baton, + PyObject *py_parse_fns3, + apr_pool_t *pool) +{ + *parse_fns3 = &thunk_parse_fns3_vtable; + *parse_baton = make_baton(pool, py_parse_fns3, NULL); + + /* Dump stream vtable does not provide a method which is called right before + the end of the parsing (similar to close_edit/abort_edit in delta editor). + Thus, register a pool clean-up routine to release this parse baton. */ + apr_pool_cleanup_register(pool, *parse_baton, svn_swig_py_parse_fns3_destroy, + apr_pool_cleanup_null); +} + /*** Other Wrappers for SVN Functions ***/ Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h =================================================================== --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h (revision 1642639) +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h (working copy) @@ -252,6 +252,12 @@ PyObject *py_editor, apr_pool_t *pool); +/* make a parse vtable that "thunks" from C callbacks up to Python */ +void svn_swig_py_make_parse_fns3(const svn_repos_parse_fns3_t **parse_fns3, + void **parse_baton, + PyObject *py_parse_fns3, + apr_pool_t *pool); + apr_file_t *svn_swig_py_make_file(PyObject *py_file, apr_pool_t *pool); Index: subversion/bindings/swig/python/svn/core.py =================================================================== --- subversion/bindings/swig/python/svn/core.py (revision 1642639) +++ subversion/bindings/swig/python/svn/core.py (working copy) @@ -191,8 +191,9 @@ svn_stream_write(self._stream, buf) def close(self): - svn_stream_close(self._stream) - self._stream = None + if self._stream is not None: + svn_stream_close(self._stream) + self._stream = None def secs_from_timestr(svn_datetime, pool=None): """Convert a Subversion datetime string into seconds since the Epoch.""" Index: subversion/bindings/swig/python/svn/repos.py =================================================================== --- subversion/bindings/swig/python/svn/repos.py (revision 1642639) +++ subversion/bindings/swig/python/svn/repos.py (working copy) @@ -285,3 +285,56 @@ if idx == -1: return parent_path + '/' + path return parent_path + path[idx:] + + +class ParseFns3: + def __init__(self): + pass + + def __del__(self): + pass + + def _close_dumpstream(self): + # Does not correspond to a C method - called before finishing the + # parsing of the dump stream. + pass + + def magic_header_record(self, version, pool=None): + pass + + def uuid_record(self, uuid, pool=None): + pass + + def new_revision_record(self, headers, pool=None): + return None # Returns revision_baton + + def new_node_record(self, headers, revision_baton, pool=None): + return None # Returns node_baton + + def set_revision_property(self, revision_baton, name, value): + pass + + def set_node_property(self, node_baton, name, value): + pass + + def delete_node_property(self, node_baton, name): + pass + + def remove_node_props(self, node_baton): + pass + + def set_fulltext(self, node_baton): + return None # Returns a writable stream + + def apply_textdelta(self, node_baton): + return None # Returns delta window handler + + def close_node(self, node_baton): + pass + + def close_revision(self, revision_baton): + pass + + +def make_parse_fns3(parse_fns3, pool=None): + return svn_swig_py_make_parse_fns3(parse_fns3, pool) Index: subversion/bindings/swig/python/tests/repository.py =================================================================== --- subversion/bindings/swig/python/tests/repository.py (revision 1642639) +++ subversion/bindings/swig/python/tests/repository.py (working copy) @@ -18,7 +18,7 @@ # under the License. # # -import unittest, setup_path +import unittest, setup_path, os, sys from sys import version_info # For Python version check if version_info[0] >= 3: # Python >=3.0 @@ -44,6 +44,42 @@ self.textdeltas.append(textdelta) return textdelta_handler +class DumpStreamParser(repos.ParseFns3): + def __init__(self): + repos.ParseFns3.__init__(self) + self.ops = [] + def magic_header_record(self, version, pool=None): + self.ops.append(("magic-header", version)) + def uuid_record(self, uuid, pool=None): + self.ops.append(("uuid", uuid)) + def new_revision_record(self, headers, pool=None): + rev = int(headers[repos.DUMPFILE_REVISION_NUMBER]) + self.ops.append(("new-revision", rev)) + return rev + def close_revision(self, revision_baton): + self.ops.append(("close-revision", revision_baton)) + def new_node_record(self, headers, revision_baton, pool=None): + node = headers[repos.DUMPFILE_NODE_PATH] + self.ops.append(("new-node", revision_baton, node)) + return (revision_baton, node) + def close_node(self, node_baton): + self.ops.append(("close-node", node_baton[0], node_baton[1])) + def set_revision_property(self, revision_baton, name, value): + self.ops.append(("set-revision-prop", revision_baton, name, value)) + def set_node_property(self, node_baton, name, value): + self.ops.append(("set-node-prop", node_baton[0], node_baton[1], name, value)) + def remove_node_props(self, node_baton): + self.ops.append(("remove-node-props", node_baton[0], node_baton[1])) + def delete_node_property(self, node_baton, name): + self.ops.append(("delete-node-prop", node_baton[0], node_baton[1], name)) + def apply_textdelta(self, node_baton): + self.ops.append(("apply-textdelta", node_baton[0], node_baton[1])) + return None + def set_fulltext(self, node_baton): + self.ops.append(("set-fulltext", node_baton[0], node_baton[1])) + return None + + def _authz_callback(root, path, pool): "A dummy authz callback which always returns success." return 1 @@ -139,6 +175,62 @@ # svn_repos_t objects, so the following call segfaults #repos.dump_fs2(None, None, None, 0, self.rev, 0, 0, None) + def test_parse_fns3(self): + self.cancel_calls = 0 + def is_cancelled(): + self.cancel_calls += 1 + return None + dump_path = os.path.join(os.path.dirname(sys.argv[0]), + "trac/versioncontrol/tests/svnrepos.dump") + stream = open(dump_path) + dsp = DumpStreamParser() + ptr, baton = repos.make_parse_fns3(dsp) + repos.parse_dumpstream3(stream, ptr, baton, False, is_cancelled) + stream.close() + self.assertEqual(self.cancel_calls, 76) + expected_list = [ + ("magic-header", 2), + ('uuid', '92ea810a-adf3-0310-b540-bef912dcf5ba'), + ('new-revision', 0), + ('set-revision-prop', 0, 'svn:date', '2005-04-01T09:57:41.312767Z'), + ('close-revision', 0), + ('new-revision', 1), + ('set-revision-prop', 1, 'svn:log', 'Initial directory layout.'), + ('set-revision-prop', 1, 'svn:author', 'john'), + ('set-revision-prop', 1, 'svn:date', '2005-04-01T10:00:52.353248Z'), + ('new-node', 1, 'branches'), + ('remove-node-props', 1, 'branches'), + ('close-node', 1, 'branches'), + ('new-node', 1, 'tags'), + ('remove-node-props', 1, 'tags'), + ('close-node', 1, 'tags'), + ('new-node', 1, 'trunk'), + ('remove-node-props', 1, 'trunk'), + ('close-node', 1, 'trunk'), + ('close-revision', 1), + ('new-revision', 2), + ('set-revision-prop', 2, 'svn:log', 'Added README.'), + ('set-revision-prop', 2, 'svn:author', 'john'), + ('set-revision-prop', 2, 'svn:date', '2005-04-01T13:12:18.216267Z'), + ('new-node', 2, 'trunk/README.txt'), + ('remove-node-props', 2, 'trunk/README.txt'), + ('set-fulltext', 2, 'trunk/README.txt'), + ('close-node', 2, 'trunk/README.txt'), + ('close-revision', 2), ('new-revision', 3), + ('set-revision-prop', 3, 'svn:log', 'Fixed README.\n'), + ('set-revision-prop', 3, 'svn:author', 'kate'), + ('set-revision-prop', 3, 'svn:date', '2005-04-01T13:24:58.234643Z'), + ('new-node', 3, 'trunk/README.txt'), + ('remove-node-props', 3, 'trunk/README.txt'), + ('set-node-prop', 3, 'trunk/README.txt', 'svn:mime-type', 'text/plain'), + ('set-node-prop', 3, 'trunk/README.txt', 'svn:eol-style', 'native'), + ('set-fulltext', 3, 'trunk/README.txt'), + ('close-node', 3, 'trunk/README.txt'), ('close-revision', 3), + ] + # Compare only the first X nodes described in the expected list - otherwise + # the comparison list gets too long. + self.assertListEqual(dsp.ops[:len(expected_list)], expected_list) + def test_get_logs(self): """Test scope of get_logs callbacks""" logs = [] @@ -177,6 +269,14 @@ set(["This is a test.\n", "A test.\n"])) self.assertEqual(len(editor.textdeltas), 2) + def test_unnamed_editor(self): + """Test editor object without reference from interpreter""" + this_root = fs.revision_root(self.fs, self.rev) + prev_root = fs.revision_root(self.fs, self.rev-1) + e_ptr, e_baton = delta.make_editor(ChangeReceiver(this_root, prev_root)) + repos.dir_delta(prev_root, '', '', this_root, '', e_ptr, e_baton, + _authz_callback, 1, 1, 0, 0) + def test_retrieve_and_change_rev_prop(self): """Test playing with revprops""" self.assertEqual(repos.fs_revision_prop(self.repos, self.rev, "svn:log", Index: subversion/bindings/swig/svn_repos.i =================================================================== --- subversion/bindings/swig/svn_repos.i (revision 1642639) +++ subversion/bindings/swig/svn_repos.i (working copy) @@ -147,8 +147,18 @@ %ignore svn_repos_dump_fs2; #endif +/* ----------------------------------------------------------------------- */ +#ifdef SWIGPYTHON +/* Make swig wrap this function for us, to allow making a vtable in python */ +void svn_swig_py_make_parse_fns3(const svn_repos_parse_fns3_t **parse_fns3, + void **parse_baton, + PyObject *py_parse_fns3, + apr_pool_t *pool); +#endif + %include svn_repos_h.swg #ifdef SWIGRUBY %define_close_related_methods(repos) #endif +