On 2020/07/29 23:43, C. Michael Pilato wrote:
> Makes sense to me as a way to get access to all the things that can be
> returned in an errorful situation.

Okey, I implemented it. The attached patch add "conflict_p" and "new_rev"
attributes to SubversionException instance on svn_fs.commit_txn and
svn_repos.fs_commit_txn. Also, svn_repos.fs_commit_txn now returns
tuple of (None, int), for a consistency.

I've tested this method on other API with dummy attribute, but I didn't
test on those svn_fs.commit_txn and svn_repos.fs_commit_txn.

Could you please review and try it ?

Thanks, -- 
Yasuhito FUTATSUKI <futat...@yf.bsclub.org>
Index: subversion/bindings/swig/include/svn_types.swg
===================================================================
--- subversion/bindings/swig/include/svn_types.swg      (revision 1880203)
+++ subversion/bindings/swig/include/svn_types.swg      (working copy)
@@ -427,6 +427,10 @@
    Specify how svn_error_t returns are turned into exceptions.
 */
 #ifdef SWIGPYTHON
+%{
+#define svn_error_t_with_attrs svn_error_t
+%}
+
 %typemap(out) svn_error_t * {
     if ($1 != NULL) {
         if ($1->apr_err != SVN_ERR_SWIG_PY_EXCEPTION_SET)
@@ -438,6 +442,10 @@
     Py_INCREF(Py_None);
     $result = Py_None;
 }
+
+%typemap(out) svn_error_t_with_attrs * (svn_error_t * _global_err) {
+    _global_err = $1;
+}
 #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 1880203)
+++ 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,6 +498,18 @@
   Py_XDECREF(file_ob);
   Py_XDECREF(line_ob);
   Py_XDECREF(svn_module);
+}
+
+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 && exc_ob != NULL)
+    {
+      PyErr_SetObject(exc_class, exc_ob);
+    }
   Py_XDECREF(exc_class);
   Py_XDECREF(exc_ob);
 }
Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
===================================================================
--- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h        
(revision 1880203)
+++ 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 1880203)
+++ subversion/bindings/swig/svn_fs.i   (working copy)
@@ -104,12 +104,60 @@
    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, svn_revnum_t *new_rev)
+                 (PyObject *exc_class, PyObject *exc_ob,
+                  PyObject* conflict_ob, PyObject* rev_ob) {
+    if (_global_err != NULL)
+      {
+        if (_global_err->apr_err == SVN_ERR_SWIG_PY_EXCEPTION_SET)
+          {
+            svn_error_clear(_global_err);
+          }
+        else
+          {
+            svn_swig_py_build_svn_exception(&exc_class, &exc_ob, _global_err);
+            if (*$1 == NULL)
+              {
+                Py_INCREF(Py_None);
+                conflict_ob = Py_None;
+              }
+            else
+              {
+                conflict_ob = PyBytes_FromString((const char *)*$1);
+              }
+            PyObject_SetAttrString(exc_ob, "conflict_p", conflict_ob); 
+            rev_ob = PyInt_FromLong((long)*$2);
+            PyObject_SetAttrString(exc_ob, "new_rev", rev_ob); 
+            /* Raise the exception. */
+            PyErr_SetObject(exc_class, exc_ob);
+            Py_XDECREF(exc_class);
+            Py_XDECREF(exc_ob);
+            Py_DECREF(conflict_ob);
+            Py_DECREF(rev_ob);
+            SWIG_fail;
+          }
+        $result = NULL;
+      }
+    else
+      {
+        /* build the result tuple */
+        $result = Py_BuildValue(
+%#if PY_VERSION_HEX >= 0x03000000
+                                "yi",
+%#else
+                                "zi",
+%#endif
+                                *$1, (long)*$2);
+      }
 }
+
+svn_error_t_with_attrs *
+svn_fs_commit_txn(const char **conflict_p,
+                  svn_revnum_t *new_rev,
+                  svn_fs_txn_t *txn,
+                  apr_pool_t *pool);
+%ignore svn_fs_commit_txn;
+
 #endif
 
 /* Ruby fixups for functions not following the pool convention. */
Index: subversion/bindings/swig/svn_repos.i
===================================================================
--- subversion/bindings/swig/svn_repos.i        (revision 1880203)
+++ subversion/bindings/swig/svn_repos.i        (working copy)
@@ -109,6 +109,75 @@
 #endif
 
 /* -----------------------------------------------------------------------
+   Fix the return value for svn_repos_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.
+
+   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_repos_t *repos,
+                  svn_revnum_t *new_rev)
+                 (PyObject *exc_class, PyObject *exc_ob,
+                  PyObject* conflict_ob, PyObject* rev_ob) {
+    if (_global_err != NULL)
+      {
+        if (_global_err->apr_err == SVN_ERR_SWIG_PY_EXCEPTION_SET)
+          {
+            svn_error_clear(_global_err);
+          }
+        else
+          {
+            svn_swig_py_build_svn_exception(&exc_class, &exc_ob, _global_err);
+            if (*$1 == NULL)
+              {
+                Py_INCREF(Py_None);
+                conflict_ob = Py_None;
+              }
+            else
+              {
+                conflict_ob = PyBytes_FromString((const char *)*$1);
+              }
+            PyObject_SetAttrString(exc_ob, "conflict_p", conflict_ob); 
+            rev_ob = PyInt_FromLong((long)*$3);
+            PyObject_SetAttrString(exc_ob, "new_rev", rev_ob); 
+            /* Raise the exception. */
+            PyErr_SetObject(exc_class, exc_ob);
+            Py_XDECREF(exc_class);
+            Py_XDECREF(exc_ob);
+            Py_DECREF(conflict_ob);
+            Py_DECREF(rev_ob);
+            SWIG_fail;
+          }
+        $result = NULL;
+      }
+    else
+      {
+        /* build the result tuple */
+        $result = Py_BuildValue(
+%#if PY_VERSION_HEX >= 0x03000000
+                                "yi",
+%#else
+                                "zi",
+%#endif
+                                *$1, (long)*$3);
+      }
+}
+
+svn_error_t_with_attrs *
+svn_repos_fs_commit_txn(const char **conflict_p,
+                        svn_repos_t *repos,
+                        svn_revnum_t *new_rev,
+                        svn_fs_txn_t *txn,
+                        apr_pool_t *pool);
+%ignore svn_repos_fs_commit_txn;
+
+#endif
+/* -----------------------------------------------------------------------
    handle svn_repos_get_committed_info().
 */
 #ifdef SWIGRUBY

Reply via email to