swig-py3 branch review for trunk merge

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

swig-py3 branch review for trunk merge

Troy Curtis Jr
I finally have the swig-py3 branch in a state that I believe is ready to be merged into trunk.  All the tests pass under python 2 and 3 now, and I successfully tested viewvc 1.1.26 against the python 2 bindings.  In order to have shared code for python 2 and 3, there were a fair number of substitutions of old ways of doing things, to ways that were more compatible with both python 2.7 and 3.  Where that wasn't possible, explicit feature or version detection was employed.  However, there was not really substantive changes, and should have no changes in the way the bindings actually work.

The two biggest changes from Python 2 perspective:
1. Switching SWIG out of "classic" mode.  The means that all the generated classes are now "new-style" classes, and use properties for most attribute access. 
2. Introduction of py3c compatibility library dependency.

#1 should really only be an issue if a client subclassed one of the swig classes.  This doesn't seem likely to me, as the swig classes already do not look very inheritance friendly, but I would definitely expect some differences to subclasses in the old-style to new-style class migration.

The biggest hole in my testing is Windows.  I am not a Windows developer, so hopefully someone that is could take a look.  I'm betting that there is at least something that needs to change in order to correctly locate and use the py3c library.

So if you get a chance, take a look and let me know if you agree that it is ready to merge.

Thanks,
Troy
Reply | Threaded
Open this post in threaded view
|

Re: swig-py3 branch review for trunk merge

Troy Curtis Jr


On Wed, Dec 27, 2017 at 11:47 PM Troy Curtis Jr <[hidden email]> wrote:
I finally have the swig-py3 branch in a state that I believe is ready to be merged into trunk.  All the tests pass under python 2 and 3 now, and I successfully tested viewvc 1.1.26 against the python 2 bindings.  In order to have shared code for python 2 and 3, there were a fair number of substitutions of old ways of doing things, to ways that were more compatible with both python 2.7 and 3.  Where that wasn't possible, explicit feature or version detection was employed.  However, there was not really substantive changes, and should have no changes in the way the bindings actually work.

The two biggest changes from Python 2 perspective:
1. Switching SWIG out of "classic" mode.  The means that all the generated classes are now "new-style" classes, and use properties for most attribute access. 
2. Introduction of py3c compatibility library dependency.

#1 should really only be an issue if a client subclassed one of the swig classes.  This doesn't seem likely to me, as the swig classes already do not look very inheritance friendly, but I would definitely expect some differences to subclasses in the old-style to new-style class migration.

The biggest hole in my testing is Windows.  I am not a Windows developer, so hopefully someone that is could take a look.  I'm betting that there is at least something that needs to change in order to correctly locate and use the py3c library.

So if you get a chance, take a look and let me know if you agree that it is ready to merge.

Thanks,
Troy

I'm now asking for swig-py3 merge to trunk review with fewer caveats.

Since this original request, I did revert back to classic classes for Python 2 at brane's suggestion for better compatibility with existing python usage.  I also incorporated several suggestions by danielsh and brane.  The biggest update is addition of Windows support with the new py3c dependency.  It was a very interesting learning experience playing at Windows development, and took a great save-my-sanity hint by Nathan to finally get it working.  I'm just glad it wasn't something I was measuring SLOC productivity on! 

I am able to build and test the python 2 bindings using both Visual Studio 2015 and 2008 (which was actually a bit challenging to find a download for...you'd think it was a decade old or something!).  There is currently a build failure with Python 3 (buried in some Py3 header) on Windows, but I don't expect it to be a tremendous change, and not a blocker for a merge since Python 3 on Windows isn't currently supported.  :)  (Though I am already starting to look at it now).

So if you get a chance, take a look, and if all agree, I'll merge to trunk.

Thanks,
Troy

Reply | Threaded
Open this post in threaded view
|

Re: swig-py3 branch review for trunk merge

Daniel Shahaf-2
Troy Curtis Jr wrote on Wed, 31 Jan 2018 04:30 +0000:
> I'm now asking for swig-py3 merge to trunk review with fewer caveats.
>
> [...] The biggest update is addition of Windows support with the new py3c
> dependency.

Thanks for looking into the windows build!

> So if you get a chance, take a look, and if all agree, I'll merge to trunk.

I'd like to review the updated branch, but I'm afraid I can't promise
when I'd have time.

Cheers,

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: swig-py3 branch review for trunk merge

Troy Curtis Jr


On Wed, Jan 31, 2018 at 7:16 AM Daniel Shahaf <[hidden email]> wrote:
Troy Curtis Jr wrote on Wed, 31 Jan 2018 04:30 +0000:
> I'm now asking for swig-py3 merge to trunk review with fewer caveats.
>
> [...] The biggest update is addition of Windows support with the new py3c
> dependency.

Thanks for looking into the windows build!

> So if you get a chance, take a look, and if all agree, I'll merge to trunk.

I'd like to review the updated branch, but I'm afraid I can't promise
when I'd have time.


Understood, thanks for all time you've already spent on it!  I still have a few little cleanups that I can poke at, but wanted to point out my opinion on its readiness to be merged.

Troy