Re: [PATCH] Allow "cease invocation" return from Python callback for repos.svn_repos_history2()

[Alexey Neyman]

On Wednesday 17 March 2010 04:59:34 pm Роман Донченко wrote:
> Alexey Neyman <sti...@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.