You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Marshall White <cs...@yahoo.com> on 2003/02/01 04:14:42 UTC

SWIG/Python interface bug

I am going through looking into what I can do with the SWIG Python stuff.

I found one thing I can't do.

It seems like it doesn't like functions with arguments of "const char **"

I wrote a test script to verify this for myself.  It is at the bottom of this email (I named it
"py_broke.py").
For instance, my output is:
mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
commit date   = `_60141a08_p_char`
commit author = `_f0371a08_p_char`

The commit date and authors are human readable strings.
The problem is that the arguments are being returned as "char *" which means nothing to Python. 
They should instead be returned back as Python string objects.

The function I am calling in this script is just one of many that currently won't work correctly.
In fact one example of this bug exists in cvs2svn (that's where I got the idea):
    conflicts, new_rev = fs.commit_txn(txn)

    # set the time to the proper (past) time
    fs.change_rev_prop(t_fs, new_rev, 'svn:date', date, c_pool)

    ### how come conflicts is a newline?
    if conflicts != '\n':
      print '    CONFLICTS:', `conflicts`

Indeed conflicts should not be a newline.  In fact, most of the time it isn't.  It is one of those
"*_p_char" things.

1) Is this a known bug?
2) I have a fairly simple patch to fix this.  It causes "conflicts" in cvs2svn to be an empty
string (as it should be).  It also allows the attached test script to print readable dates and
author names.  I will post shortly.

Marshall


#!/usr/bin/env python
#
# SVN SWIG Python interface test: ...
#
# How well do we handle C functions with const char ** arguments
#

import os
import sys

from svn import fs, util, repos


def test_this(pool, ctx):
  t_repos = repos.svn_repos_open(ctx.repospath, pool)
  t_fs = repos.svn_repos_fs(t_repos)

  rev = fs.youngest_rev(t_fs, pool)
  txn = fs.begin_txn(t_fs, rev, pool)
  root = fs.txn_root(txn, pool)

  cr, cd, la = repos.svn_repos_get_committed_info(root, ctx.filepath, pool)

  print 'commit date   = `%s`' % cd
  print 'commit author = `%s`' % la

class _ctx:
  pass

def usage(ctx):
  print 'USAGE: %s svn-repos-path repos-file' % os.path.basename(sys.argv[0])
  print '  svn-repos-path - A path to a valid repository'
  print '  repos-file     - A path to a file in that repository'
  sys.exit(1)

def main():
  # prepare the operation context
  ctx = _ctx()

  if len(sys.argv) != 3:
    usage(ctx)

  ctx.repospath = sys.argv[1]
  ctx.filepath = sys.argv[2]

  util.run_app(test_this, ctx)

if __name__ == '__main__':
  main()




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: SWIG/Python interface bug

Posted by Marshall White <cs...@yahoo.com>.
Sander,

It is NOT still an issue.  The patch is NOT still valid.

Thanks,
Marshall


--- Sander Roobol <ph...@wanadoo.nl> wrote:
> On Sat, Feb 01, 2003 at 06:45:44PM -0800, Marshall White wrote:
> > --- Marshall White <cs...@yahoo.com> wrote:
> > > At the bottom of this email is the patch for the bug I wrote about in my last email.
> > > 
> > > Here is a before and after:
> > > BEFORE:
> > > mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
> > > commit date   = `_60141a08_p_char`
> > > commit author = `_f0371a08_p_char`
> > > 
> > > AFTER:
> > > mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
> > > commit date   = `2003-01-31T04:46:32.052521Z`
> > > commit author = `mwhite`
> > > 
> 
> Is this still an issue, and is your patch still valid?
> 
> Sander


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: SWIG/Python interface bug

Posted by Sander Roobol <ph...@wanadoo.nl>.
On Sat, Feb 01, 2003 at 06:45:44PM -0800, Marshall White wrote:
> --- Marshall White <cs...@yahoo.com> wrote:
> > At the bottom of this email is the patch for the bug I wrote about in my last email.
> > 
> > Here is a before and after:
> > BEFORE:
> > mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
> > commit date   = `_60141a08_p_char`
> > commit author = `_f0371a08_p_char`
> > 
> > AFTER:
> > mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
> > commit date   = `2003-01-31T04:46:32.052521Z`
> > commit author = `mwhite`
> > 

Is this still an issue, and is your patch still valid?

Sander

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH]: SWIG/Python interface bug

Posted by Marshall White <cs...@yahoo.com>.
--- Marshall White <cs...@yahoo.com> wrote:
> At the bottom of this email is the patch for the bug I wrote about in my last email.
> 
> Here is a before and after:
> BEFORE:
> mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
> commit date   = `_60141a08_p_char`
> commit author = `_f0371a08_p_char`
> 
> AFTER:
> mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
> commit date   = `2003-01-31T04:46:32.052521Z`
> commit author = `mwhite`
> 
> Comments/suggestions?
> 
> 
> Marshall
> 

I still think my patch is okay, but my original test script wasn't great.

For those of you who tried it (if anyone did), it leaves open transactions in your repository.
Thank goodness for "svnadmin."

Here is the updated test script.

Marshall


#!/usr/bin/env python
#
# SVN SWIG Python interface test:
#
# Tests the interface with functions that have const char ** arguments.
# The argument should be a return value of a Python string.
#

import os
import sys

from svn import fs, util, repos


def test_this(pool, ctx):
  t_repos = repos.svn_repos_open(ctx.repospath, pool)
  t_fs = repos.svn_repos_fs(t_repos)

  rev = fs.youngest_rev(t_fs, pool)
  txn = fs.begin_txn(t_fs, rev, pool)
  root = fs.txn_root(txn, pool)

  cr, cd, la = repos.svn_repos_get_committed_info(root, ctx.filepath, pool)
  txn_name = fs.txn_name(txn, pool)

  # Be sure to abort the transaction before exiting.
  fs.abort_txn(txn)

  print 'commit date   = `%s`' % cd
  print 'commit author = `%s`' % la
  print 'trxn name     = `%s`' % txn_name

class _ctx:
  pass

def usage(ctx):
  print 'USAGE: %s svn-repos-path repos-file' % os.path.basename(sys.argv[0])
  print '  svn-repos-path - A path to a valid repository'
  print '  repos-file     - A path to a file in that repository'
  sys.exit(1)

def main():
  # prepare the operation context
  ctx = _ctx()

  if len(sys.argv) != 3:
    usage(ctx)

  ctx.repospath = sys.argv[1]
  ctx.filepath = sys.argv[2]

  util.run_app(test_this, ctx)

if __name__ == '__main__':
  main()



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH]: SWIG/Python interface bug

Posted by Marshall White <cs...@yahoo.com>.
At the bottom of this email is the patch for the bug I wrote about in my last email.

Here is a before and after:
BEFORE:
mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
commit date   = `_60141a08_p_char`
commit author = `_f0371a08_p_char`

AFTER:
mwhite@big-dog:/storage/code/python# ./py_broke.py /home/svnrepos /trunk
commit date   = `2003-01-31T04:46:32.052521Z`
commit author = `mwhite`

Comments/suggestions?


Marshall



Log:
Fix SWIG Python interface so that C functions with arguments of type
"const char **" behave correctly.
* subversion/bindings/swig/svn_string.i
  (%typemap(python,argout,fragment="t_output_helper") const char **OUTPUT):
  Return an empty string instead of a Py_None binding.  If the Python code
  is expecting something, and the function generates nothing, a run-time
  error will result.
  Added "in,numinputs=0" and an initial value for "temp" to 
  (%typemap(python,in,numinputs=0) const char **OUTPUT 
  (const char *temp = (const char *)0))

* subversion/bindings/swig/svn_fs.i:
  Move "const char **" from "%apply SWIGTYPE **OUTPARAM" to
  "%apply const char **OUTPUT"
  This forces C functions with "const char **" args to use the right binding.

Fix return value check for function fixed above
* tools/cvs2svn/cvs2svn.py (Commit::commit):
  Fixed return value check from "fs.commit_xtn"


Index: subversion/bindings/swig/svn_string.i
===================================================================
--- subversion/bindings/swig/svn_string.i	(revision 4694)
+++ subversion/bindings/swig/svn_string.i	(working copy)
@@ -115,19 +115,23 @@
 */
 
 /* ### note that SWIG drops the const in the arg decl, so we must cast */
-%typemap(python) const char **OUTPUT (const char *temp) {
+%typemap(python,in,numinputs=0) const char **OUTPUT (const char *temp = (const char *)0) {
     $1 = (char **)&temp;
 }
 %typemap(python,argout,fragment="t_output_helper") const char **OUTPUT {
     PyObject *s;
     if (*$1 == NULL) {
-        Py_INCREF(Py_None);
-        s = Py_None;
+        /* We MUST return a string, even if it is empty.
+         * Returning Py_None will cause errors...
+         */
+        s = PyString_FromString("");
     }
     else {
         s = PyString_FromString(*$1);
-        if (s == NULL)
-            return NULL;
+    }
+    if (s == NULL) {
+        PyErr_SetString(PyExc_TypeError, "Could not create string");
+        return NULL;
     }
     $result = t_output_helper($result, s);
 }
Index: subversion/bindings/swig/svn_fs.i
===================================================================
--- subversion/bindings/swig/svn_fs.i	(revision 4694)
+++ subversion/bindings/swig/svn_fs.i	(working copy)
@@ -40,9 +40,9 @@
     svn_fs_txn_t **,
     void **,
     svn_fs_id_t **,
-    const char **,
     svn_stream_t **
 };
+%apply const char **OUTPUT { const char ** };
 
 /* ### need to deal with IN params which have "const" and OUT params which
    ### return non-const type. SWIG's type checking may see these as
Index: tools/cvs2svn/cvs2svn.py
===================================================================
--- tools/cvs2svn/cvs2svn.py	(revision 4694)
+++ tools/cvs2svn/cvs2svn.py	(working copy)
@@ -439,8 +439,8 @@
     # set the time to the proper (past) time
     fs.change_rev_prop(t_fs, new_rev, 'svn:date', date, c_pool)
 
-    ### how come conflicts is a newline?
-    if conflicts != '\n':
+    # If there was a conflict, print it.
+    if conflicts != '':
       print '    CONFLICTS:', `conflicts`
     print '    new revision:', new_rev
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org