Re: Python bindings API confusion

On 2020/12/28 19:10, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Sat, 26 Dec 2020 11:02 +00:00:
>> On 2020/12/26 3:38, Nathan Hartman wrote:
>>
>>> But the solution you suggest above, adding a new_rev attribute to the
>>> exception and leaving the return value as it was, sounds like a good way to
>>> avoid the incompatibility and also avoid the API bump.
>>
>> I had written such a patch on typemaps at first because it was straight
>> forward. Although I removed it after r1880967 is commited, I can write
>> such a code again and I'll do if it is needed.
>>
>> Alternately, here is an easier way to do it. The patch below is a code
>> that replace wrapper function entity on top of the svn.fs module
>> (not tested, and more appropriate comment is needed in the code).

<snip>

>> Of course, we can use svn_fs_commit_txn2 instead of _svn_fs_commit_txn
>> in above patch, then it will satisfy the Daniel's proposal.
>
> I agree with Nathan that it'd be confusing for a Python
> svn_fs_commit_txn2() to exist when a C function of the same name does
> not, even if the C API skips a revv number in consideration of swig-py.
> So, if we'd like to introduce a simplified API, I'd vote to name it
> something else.

Then I'll commit patch amended only for restore Python 2 support,
without renaming to svn_fs_commit_txn2, if Mike agrees because he
already wrote some code using svn.fs.commit_txn().
(attached swig-py-fs_commit_txn-revert-return-value-type-patch.txt)

I also wrote tests for svn.fs.commit_txn, both for return value and
attributes in SubversionException, by porting tests for C API into Python.
(attached patch swig-py-fs_commit_txn-test-patch.txt; tested on Python
2.7.18 and 3.9.0 on FreeBSD 12)

During writing the test above I also found that on swig-py for Python 3
wrapper functions using svn_string_t * or svn_stringbuf_t * for in
input arguments don't accept str objects for their values. This was
my overlook, so I'll fix it. However, this is another topic.

Cheers,

-- 
Yasuhito FUTATSUKI <futatuki_at_yf.bsclub.org>