Re: [Patch] Support building with SWIG 4 on Python 3.x

On 2019/11/15 16:37, Branko Čibej wrote:
> On 15.11.2019 08:10, Yasuhito FUTATSUKI wrote:
>> On 2019/11/15 14:04, Nathan Hartman wrote:
>>> On Thu, Nov 14, 2019 at 8:18 AM Jun Omae <jun66j5_at_gmail.com> wrote:
>>>
>>>> (Posting to dev list, again...)
>>>>
>>>> On Thu, Nov 14, 2019 at 1:47 AM Julian Foad <julianfoad_at_apache.org>
>>>> wrote:
>>>>> Perhaps someone could commit this if there are no other concerns with
>>>>> it, and adjust INSTALL at the same time?
>>>>
>>>> Updated proposed patch, including a change of
>>>> subversion/bindings/swig/INSTALL.
>>>>
>>>
>>> Hi all,
>>>
>>> Jun, thank you for this patch.
>>>
>>> I would like to get this committed quickly if there are no
>>> objections, but
>>> this is not my area of expertise.
>>>
>>> Brane, do I understand correctly that you are satisfied with the
>>> results of
>>> your review/test?
>>
>> I also tested make check-swig-py on FreeBSD, with Python 2.7/3.7 and
>> SWIG 2.0.0/2.0.12/3.0.9/3.0.10/3.0.12/4.0.1 combination. Of course,
>> combination of Python 2.7 + SWIG 4.0.1 and combination of
>> Python 3.7 + SWIG < 3.0.10 are blocked by .check_swig_py target :)
>> Other combinations are passed the test.
>>
>> I considered if we can move classic style versus new style class
>> conditional
>> from run time to SWIG code generation time only, but it is not just
>> problem in this patch only, but also in current code.
>>
>>> In particular, the main question I have is with regards to the -modern
>>> option being added for SWIG 3.x .. <4 with Py3. If I understand
>>> correctly,
>>> this changes the way SWIG will generate accessors (properties vs
>>> getters/setters). Would this break any downstream code? (And if so,
>>> is that
>>> acceptable?)
>>
>> There is no Python 3 application depending on it, because we have not yet
>> released SWIG Python bindings that supports Python 3.
>>
>>> Also I don't fully understand the last part of the patch: is it creating
>>> replacements for the aforementioned getters/setters?
>>
>> No, they are only helpers to create them to absorb difference how to
>> access
>> attributes between combination of Python/SWIG versions. I think this is
>> a private part how we implement proxy objects for C data structures, and
>> not for expose to be used from applications.
>
> +1.
>
> While the modern/classic distinction is potentially visible from Python
> 2 code (especially when using metaclasses with swig-py bindings), I
> consider that very much a corner case. With the upcoming EOL of Python
> 2, this difference is quickly becoming irrelevant.
Moreover, swig-py bindings for Python 2 always use classic style for it,
if we can decide not to support SWIG 4+ for Python 2. In this case,
the distinction is needed only for Python 2/3 dual support in
applications, but it is equivalent to Python 2/3 distinction in such
case.

> We should add a note
> about that to the 1.14 release notes, but otherwise, I'm all for
> committing this patch.

I think it is also need some description on somewhere about other
difference between py2/py3 bindings, mapping from/to char * type in
C structures (and somethings else if they exist), but it is not a
reason to block to commit the patch.

Cheers,

-- 
Yasuhito FUTATSUKI <futatuki_at_yf.bsdclub.org>/<futatuki_at_poem.co.jp>