swig-py: Allow SubversionException to add attributes In SWIG Python bindings, when C API a function causes an error the Python wrapper function couldn't any return value related to the error because it raises an SubversionException. With this patch, we introduce new set of typemaps to help adding attributes related to the error to the exception instance object. [in subversion/bindings/swig/] * include/svn_types.swg (typemap(out) svn_error_t * SVN_ERR_WITH_ATTRS, typemap(ret) svn_error_t * SVN_ERR_WITH_ATTRS): New typemap. Those wrapper functions use attributes on exception object feature apply this, and should use argout typemaps only that aware they may be executed even the C API return an error status. With those typemaps, the argout typemaps can use the status code of the subversion error 'arp_err' and the exception instance object 'exc_ob'. * python/libsvn_swig_py/swigutil_py.h, python/libsvn_swig_py/swigutil_py.c (svn_swig_py_build_svn_exception): New function. (svn_swig_py_svn_exception): Use svn_swig_py_build_svn_exception to get the exception class and to make the instance of it from subversion error. * svn_fs.i (typemap(argout) (const char **conflict_p, svn_revnum_t *new_rev)): Removed. This reverts the behavor of the svn.fs.commit_txn() return value, that it returns 2-tuple which of the first emenent is always None if exception is not occured. (typemap(argout) (const char **conflict_p): New custom typemap to add an attibute to exception instance object. This is applied to svn_fs_merge(), svn_fs_commit_txn() and svn_repos_fs_commit_txn(). (typemap(argout) (svn_revnum_t *new_rev): New custom typemap to add an attibute to exception instance object. This is applied to svn_fs_commit_txn() and svn_repos_fs_commit_txn(). (): Apply the typemap svn_error_t * SVN_ERR_WITH_ATTRS to svn_fs_merge() and svn_fs_commit_txn(). * svn_repos.i (): Apply the typemap svn_error_t * SVN_ERR_WITH_ATTRS to svn_repos_fs_merge(). Found by: cmpilato Index: subversion/bindings/swig/include/svn_types.swg =================================================================== --- subversion/bindings/swig/include/svn_types.swg (revision 1880475) +++ subversion/bindings/swig/include/svn_types.swg (working copy) @@ -438,6 +438,54 @@ Py_INCREF(Py_None); $result = Py_None; } + +%typemap(out) svn_error_t * SVN_ERR_WITH_ATTRS (apr_status_t apr_err, + PyObject *exc_class, + PyObject *exc_ob) { + apr_err = 0; + exc_class = exc_ob = NULL; + if ($1 != NULL) + { + apr_err = $1->apr_err; + if (apr_err != SVN_ERR_SWIG_PY_EXCEPTION_SET) + { + svn_swig_py_build_svn_exception(&exc_class, &exc_ob, $1); + if (exc_ob == NULL) + { + /* we cannot add attrs to exception instance */ + if (exc_class != NULL) + { + /* Raise an exception without instance ... */ + PyErr_SetNone(exc_class); + Py_DECREF(exc_class); + } + SWIG_fail; + } + /* pending to raise the exception */ + } + else + { + svn_error_clear($1); + SWIG_fail; + } + } + else + { + Py_INCREF(Py_None); + $result = Py_None; + } +} + +%typemap(ret) svn_error_t * SVN_ERR_WITH_ATTRS { + if (exc_ob != NULL) + { + PyErr_SetObject(exc_class, exc_ob); + Py_DECREF(exc_class); + Py_DECREF(exc_ob); + Py_XDECREF($result); + SWIG_fail; + } +} #endif #ifdef SWIGPERL Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c =================================================================== --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (revision 1880475) +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (working copy) @@ -411,10 +411,11 @@ /*** Custom SubversionException stuffs. ***/ -void svn_swig_py_svn_exception(svn_error_t *error_chain) +void svn_swig_py_build_svn_exception( + PyObject **exc_class, PyObject **exc_ob,svn_error_t *error_chain) { PyObject *args_list, *args, *apr_err_ob, *message_ob, *file_ob, *line_ob; - PyObject *svn_module, *exc_class, *exc_ob; + PyObject *svn_module; svn_error_t *err; if (error_chain == NULL) @@ -422,7 +423,7 @@ /* Start with no references. */ args_list = args = apr_err_ob = message_ob = file_ob = line_ob = NULL; - svn_module = exc_class = exc_ob = NULL; + svn_module = *exc_class = *exc_ob = NULL; if ((args_list = PyList_New(0)) == NULL) goto finished; @@ -481,16 +482,13 @@ /* Create the exception object chain. */ if ((svn_module = PyImport_ImportModule((char *)"svn.core")) == NULL) goto finished; - if ((exc_class = PyObject_GetAttrString(svn_module, - (char *)"SubversionException")) == NULL) - goto finished; - if ((exc_ob = PyObject_CallMethod(exc_class, (char *)"_new_from_err_list", - (char *)"O", args_list)) == NULL) - goto finished; + if ((*exc_class = PyObject_GetAttrString(svn_module, + (char *)"SubversionException")) != NULL) + { + *exc_ob = PyObject_CallMethod(*exc_class, (char *)"_new_from_err_list", + (char *)"O", args_list); + } - /* Raise the exception. */ - PyErr_SetObject(exc_class, exc_ob); - finished: /* Release any references. */ Py_XDECREF(args_list); @@ -500,11 +498,30 @@ Py_XDECREF(file_ob); Py_XDECREF(line_ob); Py_XDECREF(svn_module); - Py_XDECREF(exc_class); - Py_XDECREF(exc_ob); } +void svn_swig_py_svn_exception(svn_error_t *error_chain) +{ + PyObject *exc_class, *exc_ob; + svn_swig_py_build_svn_exception(&exc_class, &exc_ob, error_chain); + /* Raise the exception. */ + if (exc_class != NULL) + { + if (exc_ob != NULL) + { + PyErr_SetObject(exc_class, exc_ob); + Py_DECREF(exc_ob); + } + else + { + PyErr_SetNone(exc_class); + } + Py_DECREF(exc_class); + } +} + + /*** Helper/Conversion Routines ***/ Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h =================================================================== --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h (revision 1880475) +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h (working copy) @@ -101,6 +101,11 @@ /*** Functions to expose a custom SubversionException ***/ +/* get a subversion exception class object and its instance built from + error_chain but not raise immediately. cunsume the error. */ +void svn_swig_py_build_svn_exception( + PyObject **exc_class, PyObject **exc_ob, svn_error_t *error_chain); + /* raise a subversion exception, created from a normal subversion error. consume the error. */ void svn_swig_py_svn_exception(svn_error_t *err); Index: subversion/bindings/swig/svn_fs.i =================================================================== --- subversion/bindings/swig/svn_fs.i (revision 1880475) +++ subversion/bindings/swig/svn_fs.i (working copy) @@ -93,23 +93,68 @@ %apply apr_hash_t *MERGEINFO { apr_hash_t *mergeinhash }; /* ----------------------------------------------------------------------- - Fix the return value for svn_fs_commit_txn(). If the conflict result is - NULL, then %append_output() is passed Py_None, but that goofs up - because that is *also* the marker for "I haven't started assembling a - multi-valued return yet" which means the second return value (new_rev) - will not cause a 2-tuple to be manufactured. + Tweak a SubversionException instance when it is raised in + svn_fs_merge(), svn_fs_commit_txn() and svn_repos_fs_commit_txn(). + Those APIs return conflicts (and revision number on svn_fs_commit_txn + and svn_repos_fs_commit_txn) related to the conflict error when it + is occured. As Python wrapper functions report errors by raising + exceptions and don't return values, we use attributes of the exception + to pass these values to the caller. +*/ - The answer is to explicitly create a 2-tuple return value. - - FIXME: Do the Perl and Ruby bindings need to do something similar? -*/ #ifdef SWIGPYTHON -%typemap(argout) (const char **conflict_p, svn_revnum_t *new_rev) { - /* this is always Py_None */ - Py_DECREF($result); - /* build the result tuple */ - $result = Py_BuildValue("zi", *$1, (long)*$2); +%typemap(argout) (const char **conflict_p) (PyObject* conflict_ob) { + if (*$1 == NULL) + { + Py_INCREF(Py_None); + conflict_ob = Py_None; + } + else + { + /* Note: We can check if apr_err == SVN_ERR_FS_CONFLICT or not + before access to *$1 */ + conflict_ob = PyBytes_FromString((const char *)*$1); + if (conflict_ob == NULL) + { + Py_XDECREF(exc_ob); + Py_XDECREF($result); + SWIG_fail; + } + } + if (exc_ob != NULL) + { + PyObject_SetAttrString(exc_ob, "$1_name", conflict_ob); + Py_DECREF(conflict_ob); + } + else + { + %append_output(conflict_ob); + } } + +%typemap(argout) svn_revnum_t *new_rev (PyObject *rev_ob) { + rev_ob = PyInt_FromLong((long)*$1); + if (rev_ob == NULL) + { + Py_XDECREF(exc_ob); + Py_XDECREF($result); + SWIG_fail; + } + if (exc_ob != NULL) + { + PyObject_SetAttrString(exc_ob, "$1_name", rev_ob); + Py_DECREF(rev_ob); + } + else + { + %append_output(rev_ob); + } +} + +%apply svn_error_t *SVN_ERR_WITH_ATTRS { + svn_error_t * svn_fs_commit_txn, + svn_error_t * svn_fs_merge +}; #endif /* Ruby fixups for functions not following the pool convention. */ Index: subversion/bindings/swig/svn_repos.i =================================================================== --- subversion/bindings/swig/svn_repos.i (revision 1880475) +++ subversion/bindings/swig/svn_repos.i (working copy) @@ -109,6 +109,16 @@ #endif /* ----------------------------------------------------------------------- + Tweak a SubversionException instance (See svn_fs.i for detail). +*/ + +#ifdef SWIGPYTHON +%apply svn_error_t *SVN_ERR_WITH_ATTRS { + svn_error_t * svn_repos_fs_commit_txn +}; +#endif + +/* ----------------------------------------------------------------------- handle svn_repos_get_committed_info(). */ #ifdef SWIGRUBY