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

Re: [PATCH] Fix abort_edit in swig-py

From: Eric Gillespie <epg_at_pretzelnet.org>
Date: Thu, 03 Apr 2008 15:41:15 -0700

"David James" <james_at_cs.toronto.edu> writes:

> Patch looks mostly good. I have one question: What happens if the
> abort_edit callback raises an exception which is not caught anywhere?
> In this case, I think the abort_edit exception should win and clobber
> the old exception.

Oh, good catch. I fixed this and expanded the test to cover both
raise-and-discard and just raise. How's this?

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,54 @@ 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 TestAbortException(Exception):
+ pass
+ class ChangeReceiver(delta.Editor):
+ def __init__(self, catch_abort_exception):
+ self.aborted = False
+ self.catch_abort_exception = catch_abort_exception
+ def apply_textdelta(self, file_baton, base_checksum):
+ raise TestException
+ def abort_edit(self, pool=None):
+ self.aborted = True
+ try:
+ raise TestAbortException
+ except:
+ if not self.catch_abort_exception:
+ raise
+
+ 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")
+
+ editor = ChangeReceiver(catch_abort_exception=True)
+ e_ptr, e_baton = delta.make_editor(editor)
+ 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)
+ self.assertEqual(editor.aborted, True)
+
+ editor = ChangeReceiver(catch_abort_exception=False)
+ e_ptr, e_baton = delta.make_editor(editor)
+ 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(TestAbortException,
+ reporter.finish_report, reporter_baton)
+ self.assertEqual(editor.aborted, True)
+ finally:
+ ra.reparent(self.ra_ctx, sess_url)
+
   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,52 @@ 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 (result == NULL)
+ {
+ /* Discard earlier exception, if any. */
+ Py_XDECREF(type);
+ Py_XDECREF(value);
+ Py_XDECREF(traceback);
+ /* Pass callback's exception along. */
+ err = callback_exception_error();
+ goto finished;
+ }
+
+ /* there is no return value, so just toss this object (probably Py_None) */
+ Py_DECREF(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-04 00:41:29 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.