On Fri, Feb 28, 2003 at 06:29:14AM -0800, Marshall White wrote:
> Greg,
>
> Your patch for svn_string.i was almost exactly what I posted a month ago. :-)
Unfortunate :-( But recall from the HACKING document:
If you don't get a response for a while, and don't see the patch
applied, it may just mean that people are really busy. Go ahead and
repost, and don't hesitate to point out that you're still waiting for
a response. One way to think of it is that patch management is highly
parallizable, and we need you to shoulder your share of the management
as well as the coding. Every patch needs someone to shepherd it
through the process, and the person best qualified to do that is the
original submitter.
>...
> However, doesn't that just always reduce it down to the OTHER type of problem?
> I see you hacked around one specific case in svn_fs.i, but what about any others?
The others will also need to be fixed. I'm not sure how many functions have
multiple-value returns. I don't think a general solution is possible, but
certainly wouldn't mind finding one :-)
> As of Jan 31, the affected functions that had "const char **" args were:
> svn_repos_get_committed_info
> svn_repos_fs_commit_txn
> svn_fs_commit_txn
> svn_fs_txn_name
> svn_fs_copied_from
> svn_fs_merge
> svn_opt_format_option
> svn_io_open_unique_file
> svn_io_detect_mimetype
>
> That list might be slightly different now.
I fixed the svn_fs functions. The repos functions would need to be fixed,
and SWIG shouldn't bind svn_opt_* or svn_io_open_unique_file(). Those don't
need bindings on the assumption that the native language has better
equivalents. The svn_io_detect_mimetype should probably be bound so that an
app can potentially use the same logic as SVN itself.
Just checked. The svn_repos functions are now working cuz svn_repos.i
imports svn_fs.i, which includes the typemap for 'const char **'. The
svn_io_detect_mimetype is already working.
> Isn't this really a general case problem anyway?
> As in, SWIG doesn't correctly handle functions that return NULL valued pointers as their first
> return values?
Well, it isn't specifically tied to NULL pointers, but any value where the
bindings treat a value as Py_None. The t_output_helper() function has a hard
time distinguishing between the fallback Py_None as the return value, and a
"real" return value of Py_None.
> Ultimately this sounds like something that should be fixed in SWIG, not hacked around in
> Subversion (even though that may be necessary in the short term.)
If we can figure out the Right Solution, then heck yah. I'd be more than
happy to see SWIG fixed for this. I just don't have that solution right now.
It would certainly require some thinking about how to revamp the computation
of the result.
> So if Subversion is going to hack around it, wouldn't it be better to have a general solution that
> works in every case rather than hack it one arg list at a time?
Of course.
> I would argue that this is
> exactly what Dale (and myself to some extent) seemed to be getting to.
Your solutions turned single-value-return functions into returning singleton
tuples. That wasn't right. If there is a way to avoid that, then great. I'm
totally fine with that.
> This same problem can and and I am sure will affect types other than "const char **".
>
> I guess I am looking for your feelings on the overall status of this. Do you consider this a case
> closed, or something that still needs attention? Should any more attention be given here, or
> should someone start a SWIG crusade?
I got a problem fixed using a solution that is quite fine in my mind.
However, it doesn't address the larger problem at hand. It *would* be good
to do that, but I haven't seen a good solution for that (yet).
So no... not closed.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Mar 1 01:37:41 2003