Re: [PATCH] Fix abort_edit in swig-py

My patch is wrong, and David James's point is part of a much
larger issue. David Glasser and I talked about it, and have a
series of proposals for the list to consider. I'll start a new
thread about that, but it will take me a few minutes to get it
all into an email. This one really needs a whiteboard... :(

Here's a sneak preview of the issue:

> 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=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)

This should fail. The repos reporter's editor driver clears any
error from abort_edit, just like nearly all other editor
drivers. Yet it's unable to clear the Python exception...

in C:

my_apply_textdelta()
{
    return svn_error_t("delta error");
}
my_abort_edit()
{
    return svn_error_t("abort error");
}
my_diff() {
    ...
    err = reporter->finish_report();
    // err is the the "delta error", not "abort error"
    // the reporter discarded the "abort error"
}

in Python:

def my_apply_textdelta():
    raise Exception("delta error")
def my_abort_edit():
    raise Exception("abort error")
def my_diff():
    ...
    try:
        reporter.finish_report()
    except Exception, err:
        # err is the the "abort error", not "delta error"
        # the reporter discarded the SVN_ERR_SWIG_PY_EXCEPTION_SET svn_error_t
        # but DID NOT clear the Python exception!

Of course it didn't! How could libsvn_repos know anything about
Python? Join me in a few minutes in a new thread with your
ideas, and with an open mind about ours...

-- 
Eric Gillespie <*> epg_at_pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org