You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexey Neyman <st...@att.net> on 2010/03/16 21:05:01 UTC
[PATCH] Allow "cease invocation" return from Python callback for repos.svn_repos_history2()
Hi all,
The svn_repos_history2() function allows the history_func() to return a
special error, SVN_ERR_CEASE_INVOCATION, to stop the search. This is not
supported in Python bindings, though: attempt to return
core.SVN_ERR_CEASE_INVOCATION from the history receiver results in an
exception:
def history_lookup(path, rev, pool):
return core.SVN_ERR_CEASE_INVOCATION
repos.svn_repos_history2(..., history_lookup, ...)
svn.core.SubversionException: ('Python callback returned an invalid
object', 20014)
Indeed, svn_swig_py_repos_history_func() only expects a return of None.
This patch allows the callback to return core.SVN_ERR_CEASE_INVOCATION.
Currently, this is only used in svn_repos_history2() - apparently, it's
the only function that supports SVN_ERR_CEASE_INVOCATION. However, there
is a FIXME at least in copyfrom_info_receiver() in libsvn_client/log.c,
stating that other callbacks may eventually be able to return
SVN_ERR_CEASE_INVOCATION as well.
Regards,
Alexey.
Re: [PATCH] Allow "cease invocation" return from Python callback for repos.svn_repos_history2()
Posted by Alexey Neyman <st...@att.net>.
On Thursday 18 March 2010 05:31:25 pm Роман Донченко wrote:
> Alexey Neyman <st...@att.net> писал в своём письме Fri, 19 Mar 2010
>
> 02:26:28 +0300:
> >> Hmm. How are you testing this? I'd write a test script, but I'm
> >> short on time, and you probably already have one. 8=]
> >
> > Yes, please use the attached pre-commit script. Then:
> >
> > ...
> >
> >
> > Changed to the following code (difference from previous version is
> > that it
> > Py_DECREF's everything):
> >
> > ...
> >
> > if (PyErr_GivenExceptionMatches(exc_class, err))
>
> I will investigate more carefully later, but you've got this the wrong
> way around. The exception should be the first argument, and the class
> should be the second. The doc for PyErr_GivenExceptionMatches is kind
> of ambiguous, but see the doc for PyErr_ExceptionMatches.
Thanks for catching it. However, it does not change anything.
Regards,
Alexey.
Re: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Роман Донченко <DX...@yandex.ru>.
Alexey Neyman <st...@att.net> писал в своём письме Wed, 24 Mar 2010
03:05:29 +0300:
> Is this going to be checked to trunk only, or could it be checked in to
> upcoming 1.6.10 as well? I tried the same code with SVN I am currently
> using (1.6.6), it works as well.
You know, I was going to nominate this for backporting, but when writing
the justification realized that I'm essentially introducing a significant
change into the callback exception handling logic, which (while useful)
may be surprising to 1.6 users, and enabling fuller use of one function
does not justify that. Plus, I should really translate the full exception
chain, and that's not done yet. So I'm going to leave it for 1.7.
Roman.
Re: [PATCH] Allow "cease invocation" return from Python callback for repos.svn_repos_history2()
Posted by Alexey Neyman <st...@att.net>.
On Tuesday 23 March 2010 03:38:06 pm Роман Донченко wrote:
> Alexey Neyman <st...@att.net> писал в своём письме Fri, 19 Mar 2010
>
> 02:26:28 +0300:
> > ...
> >
> > With stock 1.6.6, it produces the following traceback:
> >
> > ---------------------------------------------------------------------
> >---- svn: Commit blocked by pre-commit hook (exit code 1) with output:
> > Traceback (most recent call last):
> > File "/tmp/c.repo/hooks/pre-commit", line 35, in <module>
> > core.run_app(verify, sys.argv[1], sys.argv[2])
> > File "/usr/local/lib/svn-python/svn/core.py", line 281, in run_app
> > return func(application_pool, *args, **kw)
> > File "/tmp/c.repo/hooks/pre-commit", line 31, in verify
> > repos.svn_repos_replay2(txn_root, "", -1, True, e_ptr, e_baton,
> > None, pool)
> > File "/usr/local/lib/svn-python/libsvn/repos.py", line 311, in
> > svn_repos_replay2
> > return apply(_repos.svn_repos_replay2, args)
> > File "/tmp/c.repo/hooks/pre-commit", line 22, in open_directory
> > repos.svn_repos_history2(self.fs_ptr, path, history_lookup, None,
> > 0, self.base_rev, True, self.pool)
> > File "/usr/local/lib/svn-python/libsvn/repos.py", line 407, in
> > svn_repos_history2
> > return apply(_repos.svn_repos_history2, args)
> > File "/tmp/c.repo/hooks/pre-commit", line 21, in history_lookup
> > raise
> > core.SubversionException(apr_err=core.SVN_ERR_CEASE_INVOCATION,
> > message="Hi from history_lookup")
> > svn.core.SubversionException: ('Hi from history_lookup', 200021)
> > ---------------------------------------------------------------------
> >----
> >
> > Changed to the following code (difference from previous version is
> > that it
> > Py_DECREF's everything):
> >
> > ---------------------------------------------------------------------
> >---- static svn_error_t *callback_exception_error(void)
> > {
> > PyObject *err = PyErr_Occurred();
> > PyObject *svn_module = NULL, *exc_class = NULL;
> > PyObject *message = NULL, *apr_err = NULL;
> > svn_error_t *rv = NULL;
> >
> > if ((svn_module = PyImport_ImportModule((char *)"svn.core")) ==
> > NULL) goto finished;
> > if ((exc_class = PyObject_GetAttrString(svn_module,
> > (char *)"SubversionException")) == NULL)
> > goto finished;
> > if (PyErr_GivenExceptionMatches(exc_class, err))
> > {
> > apr_err = PyObject_GetAttrString(err, (char *)"apr_err");
> > message = PyObject_GetAttrString(err, (char *)"message");
> > if (message && apr_err && PyString_Check(message) &&
> > PyInt_Check(apr_err))
> > rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
> > PyString_AsString(message));
> > }
> > finished:
> > Py_XDECREF(svn_module);
> > Py_XDECREF(exc_class);
> > Py_XDECREF(apr_err);
> > Py_XDECREF(message);
> > return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET,
> > NULL, "Python callback raised an exception"); }
> > ---------------------------------------------------------------------
> >----
> >
> > With this, pre-commit produces this traceback:
> >
> > ---------------------------------------------------------------------
> >---- svn: Commit blocked by pre-commit hook (exit code 1) with output:
> > Traceback (most recent call last):
> > File "/tmp/c.repo/hooks/pre-commit", line 35, in <module>
> > core.run_app(verify, sys.argv[1], sys.argv[2])
> > File "/usr/local/lib/svn-python/svn/core.py", line 281, in run_app
> > return func(application_pool, *args, **kw)
> > File "/tmp/c.repo/hooks/pre-commit", line 31, in verify
> > repos.svn_repos_replay2(txn_root, "", -1, True, e_ptr, e_baton,
> > None, pool)
> > File "/usr/local/lib/svn-python/libsvn/repos.py", line 311, in
> > svn_repos_replay2
> > return apply(_repos.svn_repos_replay2, args)
> > SystemError: NULL result without error in PyObject_Call
> > ---------------------------------------------------------------------
> >----
>
> OK, I think I got it. It couldn't find "apr_err", because
> PyErr_Occurred returns the exception _type_, and I'm pretty sure that
> the SystemError is due to not clearing the exception (although I don't
> know for sure, since I switched to PyErr_Fetch, and that automatically
> clears it). Please test with this version, which works for me:
>
> static svn_error_t *callback_exception_error(void)
> {
> PyObject *svn_module = NULL, *svn_exc = NULL;
> PyObject *message = NULL, *apr_err = NULL;
> PyObject *exc, *exc_type, *exc_traceback;
> svn_error_t *rv = NULL;
>
> PyErr_Fetch(&exc_type, &exc, &exc_traceback);
>
> if ((svn_module = PyImport_ImportModule("svn.core")) == NULL)
> goto finished;
> if ((svn_exc = PyObject_GetAttrString(svn_module,
> "SubversionException")) == NULL)
> goto finished;
>
> if (PyErr_GivenExceptionMatches(exc_type, svn_exc))
> {
> Py_DECREF(exc_type);
> Py_DECREF(exc_traceback);
> }
> else
> {
> PyErr_Restore(exc_type, exc, exc_traceback);
> goto finished;
> }
>
> if ((apr_err = PyObject_GetAttrString(exc, "apr_err")) == NULL)
> goto finished;
> if ((message = PyObject_GetAttrString(exc, "message")) == NULL)
> goto finished;
>
> Py_DECREF(exc);
>
> /* A possible improvement here would be to convert the whole
> SubversionException chain. */
> if (PyString_Check(message) && PyInt_Check(apr_err))
> rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
> PyString_AsString(message));
>
> finished:
> Py_XDECREF(svn_module);
> Py_XDECREF(svn_exc);
> Py_XDECREF(apr_err);
> Py_XDECREF(message);
> return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET,
> NULL, "Python callback raised an exception"); }
> }
Works for me, thanks! I'll try to write a test case for 'make
check-swig-py'.
Is this going to be checked to trunk only, or could it be checked in to
upcoming 1.6.10 as well? I tried the same code with SVN I am currently
using (1.6.6), it works as well.
Regards,
Alexey.
Re: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Роман Донченко <DX...@yandex.ru>.
Alexey Neyman <st...@att.net> писал в своём письме Fri, 19 Mar 2010
02:26:28 +0300:
>> Hmm. How are you testing this? I'd write a test script, but I'm short
>> on time, and you probably already have one. 8=]
>
> Yes, please use the attached pre-commit script. Then:
> ...
>
> Changed to the following code (difference from previous version is that
> it
> Py_DECREF's everything):
> ...
> if (PyErr_GivenExceptionMatches(exc_class, err))
I will investigate more carefully later, but you've got this the wrong way
around. The exception should be the first argument, and the class should
be the second. The doc for PyErr_GivenExceptionMatches is kind of
ambiguous, but see the doc for PyErr_ExceptionMatches.
> ...
Roman.
Re: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Роман Донченко <DX...@yandex.ru>.
Jon Foster <Jo...@cabot.co.uk> писал в своём письме Wed, 24 Mar 2010
15:41:47 +0300:
> Hi,
>
> [ completely valid observations about rdonch's poor reference counting
> skills ]
>
> Kind regards,
>
> Jon
Thanks. I must've been really tired when I wrote that... I've now
committed a (hopefully) better version in r927222.
Roman.
RE: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Jon Foster <Jo...@cabot.co.uk>.
Hi,
Роман Донченко wrote:
> static svn_error_t *callback_exception_error(void)
> {
> PyObject *svn_module = NULL, *svn_exc = NULL;
> PyObject *message = NULL, *apr_err = NULL;
> PyObject *exc, *exc_type, *exc_traceback;
> svn_error_t *rv = NULL;
>
> PyErr_Fetch(&exc_type, &exc, &exc_traceback);
>
> if ((svn_module = PyImport_ImportModule("svn.core")) == NULL)
> goto finished;
If we take this goto, does this code leak references to EXC_TYPE,
EXC, and EXC_TRACEBACK?
> if ((svn_exc = PyObject_GetAttrString(svn_module, "SubversionException"))
> == NULL)
> goto finished;
Ditto.
>
> if (PyErr_GivenExceptionMatches(exc_type, svn_exc))
> {
> Py_DECREF(exc_type);
> Py_DECREF(exc_traceback);
Why not move these two lines after the if() statement?
That way, you can negate the sense of the if() statement
and avoid having an ELSE block. I.e.:
if (!PyErr_GivenExceptionMatches(exc_type, svn_exc))
{
PyErr_Restore(exc_type, exc, exc_traceback);
goto finished;
}
Py_DECREF(exc_type);
Py_DECREF(exc_traceback);
> }
> else
> {
> PyErr_Restore(exc_type, exc, exc_traceback);
> goto finished;
> }
>
> if ((apr_err = PyObject_GetAttrString(exc, "apr_err")) == NULL)
> goto finished;
Here, if we take the goto, is EXC leaked?
> if ((message = PyObject_GetAttrString(exc, "message")) == NULL)
> goto finished;
Ditto.
>
> Py_DECREF(exc);
>
> /* A possible improvement here would be to convert the whole
> SubversionException chain. */
> if (PyString_Check(message) && PyInt_Check(apr_err))
> rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
> PyString_AsString(message));
If this if() statement is false, what happens? Does this
raise the SVN error SVN_ERR_SWIG_PY_EXCEPTION_SET,
but without there being a Python exception set? (Looking in
the Python 2.6.2 source code, it looks like PyInt_Check() is a
macro that expands to some bitfield-checking, it doesn't set
a Python exception for you).
>
> finished:
> Py_XDECREF(svn_module);
> Py_XDECREF(svn_exc);
> Py_XDECREF(apr_err);
> Py_XDECREF(message);
> return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
> "Python callback raised an exception");
> }
Kind regards,
Jon
**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Re: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Роман Донченко <DX...@yandex.ru>.
Alexey Neyman <st...@att.net> писал в своём письме Fri, 19 Mar 2010
02:26:28 +0300:
> ...
> With stock 1.6.6, it produces the following traceback:
>
> -------------------------------------------------------------------------
> svn: Commit blocked by pre-commit hook (exit code 1) with output:
> Traceback (most recent call last):
> File "/tmp/c.repo/hooks/pre-commit", line 35, in <module>
> core.run_app(verify, sys.argv[1], sys.argv[2])
> File "/usr/local/lib/svn-python/svn/core.py", line 281, in run_app
> return func(application_pool, *args, **kw)
> File "/tmp/c.repo/hooks/pre-commit", line 31, in verify
> repos.svn_repos_replay2(txn_root, "", -1, True, e_ptr, e_baton, None,
> pool)
> File "/usr/local/lib/svn-python/libsvn/repos.py", line 311, in
> svn_repos_replay2
> return apply(_repos.svn_repos_replay2, args)
> File "/tmp/c.repo/hooks/pre-commit", line 22, in open_directory
> repos.svn_repos_history2(self.fs_ptr, path, history_lookup, None, 0,
> self.base_rev, True, self.pool)
> File "/usr/local/lib/svn-python/libsvn/repos.py", line 407, in
> svn_repos_history2
> return apply(_repos.svn_repos_history2, args)
> File "/tmp/c.repo/hooks/pre-commit", line 21, in history_lookup
> raise core.SubversionException(apr_err=core.SVN_ERR_CEASE_INVOCATION,
> message="Hi from history_lookup")
> svn.core.SubversionException: ('Hi from history_lookup', 200021)
> -------------------------------------------------------------------------
>
> Changed to the following code (difference from previous version is that
> it
> Py_DECREF's everything):
>
> -------------------------------------------------------------------------
> static svn_error_t *callback_exception_error(void)
> {
> PyObject *err = PyErr_Occurred();
> PyObject *svn_module = NULL, *exc_class = NULL;
> PyObject *message = NULL, *apr_err = NULL;
> svn_error_t *rv = NULL;
>
> if ((svn_module = PyImport_ImportModule((char *)"svn.core")) == NULL)
> goto finished;
> if ((exc_class = PyObject_GetAttrString(svn_module,
> (char *)"SubversionException")) == NULL)
> goto finished;
> if (PyErr_GivenExceptionMatches(exc_class, err))
> {
> apr_err = PyObject_GetAttrString(err, (char *)"apr_err");
> message = PyObject_GetAttrString(err, (char *)"message");
> if (message && apr_err && PyString_Check(message) &&
> PyInt_Check(apr_err))
> rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
> PyString_AsString(message));
> }
> finished:
> Py_XDECREF(svn_module);
> Py_XDECREF(exc_class);
> Py_XDECREF(apr_err);
> Py_XDECREF(message);
> return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
> "Python callback raised an exception");
> }
> -------------------------------------------------------------------------
>
> With this, pre-commit produces this traceback:
>
> -------------------------------------------------------------------------
> svn: Commit blocked by pre-commit hook (exit code 1) with output:
> Traceback (most recent call last):
> File "/tmp/c.repo/hooks/pre-commit", line 35, in <module>
> core.run_app(verify, sys.argv[1], sys.argv[2])
> File "/usr/local/lib/svn-python/svn/core.py", line 281, in run_app
> return func(application_pool, *args, **kw)
> File "/tmp/c.repo/hooks/pre-commit", line 31, in verify
> repos.svn_repos_replay2(txn_root, "", -1, True, e_ptr, e_baton, None,
> pool)
> File "/usr/local/lib/svn-python/libsvn/repos.py", line 311, in
> svn_repos_replay2
> return apply(_repos.svn_repos_replay2, args)
> SystemError: NULL result without error in PyObject_Call
> -------------------------------------------------------------------------
OK, I think I got it. It couldn't find "apr_err", because PyErr_Occurred
returns the exception _type_, and I'm pretty sure that the SystemError is
due to not clearing the exception (although I don't know for sure, since I
switched to PyErr_Fetch, and that automatically clears it). Please test
with this version, which works for me:
static svn_error_t *callback_exception_error(void)
{
PyObject *svn_module = NULL, *svn_exc = NULL;
PyObject *message = NULL, *apr_err = NULL;
PyObject *exc, *exc_type, *exc_traceback;
svn_error_t *rv = NULL;
PyErr_Fetch(&exc_type, &exc, &exc_traceback);
if ((svn_module = PyImport_ImportModule("svn.core")) == NULL)
goto finished;
if ((svn_exc = PyObject_GetAttrString(svn_module, "SubversionException"))
== NULL)
goto finished;
if (PyErr_GivenExceptionMatches(exc_type, svn_exc))
{
Py_DECREF(exc_type);
Py_DECREF(exc_traceback);
}
else
{
PyErr_Restore(exc_type, exc, exc_traceback);
goto finished;
}
if ((apr_err = PyObject_GetAttrString(exc, "apr_err")) == NULL)
goto finished;
if ((message = PyObject_GetAttrString(exc, "message")) == NULL)
goto finished;
Py_DECREF(exc);
/* A possible improvement here would be to convert the whole
SubversionException chain. */
if (PyString_Check(message) && PyInt_Check(apr_err))
rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
PyString_AsString(message));
finished:
Py_XDECREF(svn_module);
Py_XDECREF(svn_exc);
Py_XDECREF(apr_err);
Py_XDECREF(message);
return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
"Python callback raised an exception");
}
Cheers,
Roman.
Re: [PATCH] Allow "cease invocation" return from Python callback for repos.svn_repos_history2()
Posted by Alexey Neyman <st...@att.net>.
On Thursday 18 March 2010 03:50:35 pm Роман Донченко wrote:
> Alexey Neyman <st...@att.net> писал в своём письме Thu, 18 Mar 2010
>
> 21:51:53 +0300:
> > On Wednesday 17 March 2010 04:59:34 pm Роман Донченко wrote:
> >> Alexey Neyman <st...@att.net> писал в своём письме Wed, 17 Mar 2010
> >>
> >> 00:05:01 +0300:
> >> > Hi all,
> >> >
> >> > The svn_repos_history2() function allows the history_func() to
> >> > return a special error, SVN_ERR_CEASE_INVOCATION, to stop the
> >> > search. This is not supported in Python bindings, though: attempt
> >> > to return core.SVN_ERR_CEASE_INVOCATION from the history receiver
> >> > results in an exception:
> >> >
> >> > def history_lookup(path, rev, pool):
> >> > return core.SVN_ERR_CEASE_INVOCATION
> >> >
> >> > repos.svn_repos_history2(..., history_lookup, ...)
> >> >
> >> > svn.core.SubversionException: ('Python callback returned an
> >> > invalid object', 20014)
> >> >
> >> > Indeed, svn_swig_py_repos_history_func() only expects a return of
> >> > None. This patch allows the callback to return
> >> > core.SVN_ERR_CEASE_INVOCATION. Currently, this is only used in
> >> > svn_repos_history2() - apparently, it's the only function that
> >> > supports SVN_ERR_CEASE_INVOCATION. However, there is a FIXME at
> >> > least in copyfrom_info_receiver() in libsvn_client/log.c, stating
> >> > that other callbacks may eventually be able to return
> >> > SVN_ERR_CEASE_INVOCATION as well.
> >>
> >> Good idea, but I think that the callback should signal an error the
> >> same way the Subversion functions do it - namely, by throwing
> >> core.SubversionException. The wrapper would then translate the
> >> exception's fields into an svn_error_t. It looks like tweaking
> >> callback_exception_error is the most obvious way to do that. You
> >> don't even have to special-case SVN_ERR_CEASE_INVOCATION, just copy
> >> whatever code the exception had into the error.
> >
> > I think that may be beyond my current knowledge of Python's C API...
> > I tried that:
> >
> > /* Return a Subversion error about a failed callback. */
> > static svn_error_t *callback_exception_error(void)
> > {
> > PyObject *err = PyErr_Occurred();
> > PyObject *svn_module, *exc_class;
> > PyObject *message, *apr_err;
> >
> > if ((svn_module = PyImport_ImportModule((char *)"svn.core")) ==
> > NULL) goto finished;
> > if ((exc_class = PyObject_GetAttrString(svn_module,
> > (char *)"SubversionException")) == NULL)
> > goto finished;
> > if (PyErr_GivenExceptionMatches(exc_class, err))
> > {
> > message = PyObject_GetAttrString(err, (char *)"message");
> > apr_err = PyObject_GetAttrString(err, (char *)"apr_err");
> > if (message && apr_err && PyString_Check(message)
> > && PyInt_Check(apr_err))
> > {
> > return svn_error_create(PyInt_AsLong(apr_err), NULL,
> > PyString_AsString(message));
> > }
> > }
> >
> > finished:
> > return svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
> > "Python callback raised an exception");
> > }
> >
> > First, it cannot find "apr_err" in the instance of
> > core.Subversion_Exception; PyObject_GetAttrString(err, (char
> > *)"apr_err") returns NULL.
> >
> > And, it produces the following error:
> >
> > SystemError: NULL result without error in PyObject_Call
> >
> > Can you help? What's wrong with that code?
>
> Hmm. How are you testing this? I'd write a test script, but I'm short
> on time, and you probably already have one. 8=]
Yes, please use the attached pre-commit script. Then:
$ svnadmin create /tmp/c.repo
$ cp <...this attachment...> /tmp/c.repo/hooks/pre-commit
$ svn mkdir -m "1" file:///tmp/c.repo/trunk
$ svn cp -m "2" file:///tmp/c.repo/trunk file:///tmp/c.repo/copied-dir
$ svn mkdir -m "3" file:///tmp/c.repo/copied-dir/newdir
With stock 1.6.6, it produces the following traceback:
-------------------------------------------------------------------------
svn: Commit blocked by pre-commit hook (exit code 1) with output:
Traceback (most recent call last):
File "/tmp/c.repo/hooks/pre-commit", line 35, in <module>
core.run_app(verify, sys.argv[1], sys.argv[2])
File "/usr/local/lib/svn-python/svn/core.py", line 281, in run_app
return func(application_pool, *args, **kw)
File "/tmp/c.repo/hooks/pre-commit", line 31, in verify
repos.svn_repos_replay2(txn_root, "", -1, True, e_ptr, e_baton, None,
pool)
File "/usr/local/lib/svn-python/libsvn/repos.py", line 311, in
svn_repos_replay2
return apply(_repos.svn_repos_replay2, args)
File "/tmp/c.repo/hooks/pre-commit", line 22, in open_directory
repos.svn_repos_history2(self.fs_ptr, path, history_lookup, None, 0,
self.base_rev, True, self.pool)
File "/usr/local/lib/svn-python/libsvn/repos.py", line 407, in
svn_repos_history2
return apply(_repos.svn_repos_history2, args)
File "/tmp/c.repo/hooks/pre-commit", line 21, in history_lookup
raise core.SubversionException(apr_err=core.SVN_ERR_CEASE_INVOCATION,
message="Hi from history_lookup")
svn.core.SubversionException: ('Hi from history_lookup', 200021)
-------------------------------------------------------------------------
Changed to the following code (difference from previous version is that it
Py_DECREF's everything):
-------------------------------------------------------------------------
static svn_error_t *callback_exception_error(void)
{
PyObject *err = PyErr_Occurred();
PyObject *svn_module = NULL, *exc_class = NULL;
PyObject *message = NULL, *apr_err = NULL;
svn_error_t *rv = NULL;
if ((svn_module = PyImport_ImportModule((char *)"svn.core")) == NULL)
goto finished;
if ((exc_class = PyObject_GetAttrString(svn_module,
(char *)"SubversionException")) == NULL)
goto finished;
if (PyErr_GivenExceptionMatches(exc_class, err))
{
apr_err = PyObject_GetAttrString(err, (char *)"apr_err");
message = PyObject_GetAttrString(err, (char *)"message");
if (message && apr_err && PyString_Check(message) &&
PyInt_Check(apr_err))
rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
PyString_AsString(message));
}
finished:
Py_XDECREF(svn_module);
Py_XDECREF(exc_class);
Py_XDECREF(apr_err);
Py_XDECREF(message);
return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
"Python callback raised an exception");
}
-------------------------------------------------------------------------
With this, pre-commit produces this traceback:
-------------------------------------------------------------------------
svn: Commit blocked by pre-commit hook (exit code 1) with output:
Traceback (most recent call last):
File "/tmp/c.repo/hooks/pre-commit", line 35, in <module>
core.run_app(verify, sys.argv[1], sys.argv[2])
File "/usr/local/lib/svn-python/svn/core.py", line 281, in run_app
return func(application_pool, *args, **kw)
File "/tmp/c.repo/hooks/pre-commit", line 31, in verify
repos.svn_repos_replay2(txn_root, "", -1, True, e_ptr, e_baton, None,
pool)
File "/usr/local/lib/svn-python/libsvn/repos.py", line 311, in
svn_repos_replay2
return apply(_repos.svn_repos_replay2, args)
SystemError: NULL result without error in PyObject_Call
-------------------------------------------------------------------------
Hope that helps.
Regards,
Alexey.
Re: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Роман Донченко <DX...@yandex.ru>.
Alexey Neyman <st...@att.net> писал в своём письме Thu, 18 Mar 2010
21:51:53 +0300:
> On Wednesday 17 March 2010 04:59:34 pm Роман Донченко wrote:
>> Alexey Neyman <st...@att.net> писал в своём письме Wed, 17 Mar 2010
>>
>> 00:05:01 +0300:
>> > Hi all,
>> >
>> > The svn_repos_history2() function allows the history_func() to return
>> > a special error, SVN_ERR_CEASE_INVOCATION, to stop the search. This
>> > is not supported in Python bindings, though: attempt to return
>> > core.SVN_ERR_CEASE_INVOCATION from the history receiver results in an
>> > exception:
>> >
>> > def history_lookup(path, rev, pool):
>> > return core.SVN_ERR_CEASE_INVOCATION
>> >
>> > repos.svn_repos_history2(..., history_lookup, ...)
>> >
>> > svn.core.SubversionException: ('Python callback returned an invalid
>> > object', 20014)
>> >
>> > Indeed, svn_swig_py_repos_history_func() only expects a return of
>> > None. This patch allows the callback to return
>> > core.SVN_ERR_CEASE_INVOCATION. Currently, this is only used in
>> > svn_repos_history2() - apparently, it's the only function that
>> > supports SVN_ERR_CEASE_INVOCATION. However, there is a FIXME at least
>> > in copyfrom_info_receiver() in libsvn_client/log.c, stating that
>> > other callbacks may eventually be able to return
>> > SVN_ERR_CEASE_INVOCATION as well.
>>
>> Good idea, but I think that the callback should signal an error the
>> same way the Subversion functions do it - namely, by throwing
>> core.SubversionException. The wrapper would then translate the
>> exception's fields into an svn_error_t. It looks like tweaking
>> callback_exception_error is the most obvious way to do that. You don't
>> even have to special-case SVN_ERR_CEASE_INVOCATION, just copy whatever
>> code the exception had into the error.
>
> I think that may be beyond my current knowledge of Python's C API... I
> tried that:
>
> /* Return a Subversion error about a failed callback. */
> static svn_error_t *callback_exception_error(void)
> {
> PyObject *err = PyErr_Occurred();
> PyObject *svn_module, *exc_class;
> PyObject *message, *apr_err;
>
> if ((svn_module = PyImport_ImportModule((char *)"svn.core")) == NULL)
> goto finished;
> if ((exc_class = PyObject_GetAttrString(svn_module,
> (char *)"SubversionException")) == NULL)
> goto finished;
> if (PyErr_GivenExceptionMatches(exc_class, err))
> {
> message = PyObject_GetAttrString(err, (char *)"message");
> apr_err = PyObject_GetAttrString(err, (char *)"apr_err");
> if (message && apr_err && PyString_Check(message)
> && PyInt_Check(apr_err))
> {
> return svn_error_create(PyInt_AsLong(apr_err), NULL,
> PyString_AsString(message));
> }
> }
>
> finished:
> return svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
> "Python callback raised an exception");
> }
>
> First, it cannot find "apr_err" in the instance of
> core.Subversion_Exception; PyObject_GetAttrString(err, (char *)"apr_err")
> returns NULL.
>
> And, it produces the following error:
>
> SystemError: NULL result without error in PyObject_Call
>
> Can you help? What's wrong with that code?
Hmm. How are you testing this? I'd write a test script, but I'm short on
time, and you probably already have one. 8=]
>> It would also be great if you added a relevant test to the testsuite.
>
> I thought about it, that would be the next step once the API is settled
> and implemented.
Okay.
>
> Regards,
> Alexey.
Ditto,
Roman.
Re: [PATCH] Allow "cease invocation" return from Python callback for repos.svn_repos_history2()
Posted by Alexey Neyman <st...@att.net>.
On Wednesday 17 March 2010 04:59:34 pm Роман Донченко wrote:
> Alexey Neyman <st...@att.net> писал в своём письме Wed, 17 Mar 2010
>
> 00:05:01 +0300:
> > Hi all,
> >
> > The svn_repos_history2() function allows the history_func() to return
> > a special error, SVN_ERR_CEASE_INVOCATION, to stop the search. This
> > is not supported in Python bindings, though: attempt to return
> > core.SVN_ERR_CEASE_INVOCATION from the history receiver results in an
> > exception:
> >
> > def history_lookup(path, rev, pool):
> > return core.SVN_ERR_CEASE_INVOCATION
> >
> > repos.svn_repos_history2(..., history_lookup, ...)
> >
> > svn.core.SubversionException: ('Python callback returned an invalid
> > object', 20014)
> >
> > Indeed, svn_swig_py_repos_history_func() only expects a return of
> > None. This patch allows the callback to return
> > core.SVN_ERR_CEASE_INVOCATION. Currently, this is only used in
> > svn_repos_history2() - apparently, it's the only function that
> > supports SVN_ERR_CEASE_INVOCATION. However, there is a FIXME at least
> > in copyfrom_info_receiver() in libsvn_client/log.c, stating that
> > other callbacks may eventually be able to return
> > SVN_ERR_CEASE_INVOCATION as well.
>
> Good idea, but I think that the callback should signal an error the
> same way the Subversion functions do it - namely, by throwing
> core.SubversionException. The wrapper would then translate the
> exception's fields into an svn_error_t. It looks like tweaking
> callback_exception_error is the most obvious way to do that. You don't
> even have to special-case SVN_ERR_CEASE_INVOCATION, just copy whatever
> code the exception had into the error.
I think that may be beyond my current knowledge of Python's C API... I
tried that:
/* Return a Subversion error about a failed callback. */
static svn_error_t *callback_exception_error(void)
{
PyObject *err = PyErr_Occurred();
PyObject *svn_module, *exc_class;
PyObject *message, *apr_err;
if ((svn_module = PyImport_ImportModule((char *)"svn.core")) == NULL)
goto finished;
if ((exc_class = PyObject_GetAttrString(svn_module,
(char *)"SubversionException")) == NULL)
goto finished;
if (PyErr_GivenExceptionMatches(exc_class, err))
{
message = PyObject_GetAttrString(err, (char *)"message");
apr_err = PyObject_GetAttrString(err, (char *)"apr_err");
if (message && apr_err && PyString_Check(message)
&& PyInt_Check(apr_err))
{
return svn_error_create(PyInt_AsLong(apr_err), NULL,
PyString_AsString(message));
}
}
finished:
return svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
"Python callback raised an exception");
}
First, it cannot find "apr_err" in the instance of
core.Subversion_Exception; PyObject_GetAttrString(err, (char *)"apr_err")
returns NULL.
And, it produces the following error:
SystemError: NULL result without error in PyObject_Call
Can you help? What's wrong with that code?
> It would also be great if you added a relevant test to the testsuite.
I thought about it, that would be the next step once the API is settled
and implemented.
Regards,
Alexey.
Re: [PATCH] Allow "cease invocation" return from Python callback for
repos.svn_repos_history2()
Posted by Роман Донченко <DX...@yandex.ru>.
Alexey Neyman <st...@att.net> писал в своём письме Wed, 17 Mar 2010
00:05:01 +0300:
> Hi all,
>
> The svn_repos_history2() function allows the history_func() to return a
> special error, SVN_ERR_CEASE_INVOCATION, to stop the search. This is not
> supported in Python bindings, though: attempt to return
> core.SVN_ERR_CEASE_INVOCATION from the history receiver results in an
> exception:
>
> def history_lookup(path, rev, pool):
> return core.SVN_ERR_CEASE_INVOCATION
>
> repos.svn_repos_history2(..., history_lookup, ...)
>
> svn.core.SubversionException: ('Python callback returned an invalid
> object', 20014)
>
> Indeed, svn_swig_py_repos_history_func() only expects a return of None.
> This patch allows the callback to return core.SVN_ERR_CEASE_INVOCATION.
> Currently, this is only used in svn_repos_history2() - apparently, it's
> the only function that supports SVN_ERR_CEASE_INVOCATION. However, there
> is a FIXME at least in copyfrom_info_receiver() in libsvn_client/log.c,
> stating that other callbacks may eventually be able to return
> SVN_ERR_CEASE_INVOCATION as well.
Good idea, but I think that the callback should signal an error the same
way the Subversion functions do it - namely, by throwing
core.SubversionException. The wrapper would then translate the exception's
fields into an svn_error_t. It looks like tweaking
callback_exception_error is the most obvious way to do that. You don't
even have to special-case SVN_ERR_CEASE_INVOCATION, just copy whatever
code the exception had into the error.
It would also be great if you added a relevant test to the testsuite.
Thanks,
Roman.