Without my first two patches ("Cancellation editor discards
abort_edit" and "Call abort_edit upon editor error in repos
reporter"), this test fails because abort_edit is never called:
FAIL: test_do_diff2_abort_edit (__main__.SubversionRepositoryAccessTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "subversion/bindings/swig/python/tests/ra.py", line 268, in test_do_diff2_abort_edit
self.assertEqual(editor.aborted, True)
AssertionError: False != True
Applying only the first doesn't change anything in this case.
After applying only the second patch, the reporter calls the
default editor's abort_edit.
After applying both, the reporter calls my abort_edit, but then
we get a new error:
ERROR: test_do_diff2_abort_edit (__main__.SubversionRepositoryAccessTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "subversion/bindings/swig/python/tests/ra.py", line 265, in test_do_diff2
_abort_edit
self.assertRaises(TestException, reporter.finish_report, reporter_baton)
File "/usr/lib/python2.4/unittest.py", line 320, in failUnlessRaises
callableObj(*args, **kwargs)
File "/home/epg/work/svn/subversion2/subversion/bindings/swig/python/libsvn/ra
.py", line 212, in finish_report
return svn_ra_reporter2_invoke_finish_report(self, *args)
File "/home/epg/work/svn/subversion2/subversion/bindings/swig/python/libsvn/ra
.py", line 1212, in svn_ra_reporter2_invoke_finish_report
return apply(_ra.svn_ra_reporter2_invoke_finish_report, args)
SystemError: NULL result without error in PyObject_Call
What happens here is complicated:
1. reporter.c:finish_report gets an error
(SVN_ERR_SWIG_PY_EXCEPTION_SET, from my apply_textdelta
throwing an exception) and calls abort_edit.
2. abort_edit has its own exceptions to deal with (as real
abort_edit implementations do) *thereby clearing out the
earlier Python exception from apply_textdelta*.
3. reporter.c:finish_report discards any error from abort_edit
and returns the SVN_ERR_SWIG_PY_EXCEPTION_SET error it got in
step 1.
4. The swig-generated wrapper _wrap_svn_ra_reporter3_invoke_finish_report
gets the SVN_ERR_SWIG_PY_EXCEPTION_SET error and returns NULL,
knowing that an exception is there.
Except it isn't! It was clobbered at step 2. So Python sees a
NULL return without an exception, and raises the above SystemError.
Whew!
[[[
Preserve exception state in the Python binding of the abort_edit editor
interface, so that when finish_report returns NULL the exception is still
there, rather than a 'SystemError: NULL result without error in
PyObject_Call' error.
* subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(abort_edit): This needs to be different from all the other editor
callback wrappers that use close_baton, so copy a simplified version
of that in here. But, take care to preserve any exception that may
have already been raised (likely the reason abort_edit is being called
in the first place).
* subversion/bindings/swig/python/tests/ra.py
(test_do_diff2_abort_edit): Add test that raising an exception in an
editor function causes abort_edit to be called, and that abort_edit
can handle exceptions of its own without clobbering the original.
]]]
Index: subversion/bindings/swig/python/tests/ra.py
===================================================================
--- subversion/bindings/swig/python/tests/ra.py (revision 30158)
+++ subversion/bindings/swig/python/tests/ra.py (working copy)
@@ -234,6 +234,39 @@ class SubversionRepositoryAccessTestCase(unittest.
self.assertEqual("A test.\n", editor.textdeltas[0].new_data)
self.assertEqual(1, len(editor.textdeltas))
+ def test_do_diff2_abort_edit(self):
+ class TestException(Exception):
+ pass
+ class ChangeReceiver(delta.Editor):
+ def __init__(self):
+ self.aborted = False
+ def apply_textdelta(self, file_baton, base_checksum):
+ raise TestException
+ def abort_edit(self, pool=None):
+ try:
+ raise Exception
+ except:
+ pass
+ self.aborted = True
+ editor = ChangeReceiver()
+
+ e_ptr, e_baton = delta.make_editor(editor)
+
+ fs_revnum = fs.youngest_rev(self.fs)
+
+ sess_url = ra.get_session_url(self.ra_ctx)
+ try:
+ ra.reparent(self.ra_ctx, REPOS_URL+"/trunk")
+ reporter, reporter_baton = ra.do_diff2(self.ra_ctx, fs_revnum,
+ "README.txt", 0, 0, 1,
+ REPOS_URL+"/trunk/README.txt",
+ e_ptr, e_baton)
+ reporter.set_path(reporter_baton, "", 0, True, None)
+ self.assertRaises(TestException, reporter.finish_report, reporter_baton)
+ finally:
+ ra.reparent(self.ra_ctx, sess_url)
+ self.assertEqual(editor.aborted, True)
+
def test_get_locations(self):
locations = ra.get_locations(self.ra_ctx, "trunk/README.txt", 2, range(1,5))
self.assertEqual(locations, {
Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
===================================================================
--- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (revision 30158)
+++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (working copy)
@@ -1808,7 +1808,49 @@ static svn_error_t *close_edit(void *edit_baton,
static svn_error_t *abort_edit(void *edit_baton,
apr_pool_t *pool)
{
- return close_baton(edit_baton, "abort_edit");
+ item_baton *ib = edit_baton;
+ PyObject *result;
+ svn_error_t *err;
+ PyObject *type, *value, *traceback;
+
+ svn_swig_py_acquire_py_lock();
+
+ /* Save exception state. */
+ PyErr_Fetch(&type, &value, &traceback);
+
+ /* Call abort_edit callback. */
+ result = PyObject_CallMethod(ib->editor, (char *)"abort_edit", NULL);
+ if (type == NULL && result == NULL)
+ {
+ /* We had no exception before abort and callback raised one; let the
+ callback's out. */
+ err = callback_exception_error();
+ goto finished;
+ }
+
+ /* there is no return value, so just toss this object (probably Py_None) */
+ Py_XDECREF(result);
+
+ /* 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. */
+ Py_XDECREF(ib->baton);
+
+#ifdef SVN_DEBUG
+ ib->editor = ib->baton = NULL;
+#endif
+
+ if (type != NULL)
+ {
+ /* We had an exception before abort; discard any exception the
+ callback may have raised and restore the saved exception. */
+ PyErr_Restore(type, value, traceback);
+ }
+ err = SVN_NO_ERROR;
+
+ finished:
+ svn_swig_py_release_py_lock();
+ return err;
}
void svn_swig_py_make_editor(const svn_delta_editor_t **editor,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-02 21:58:34 CEST