New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 2 End of Life 2020 #169

Closed
FSMaxB opened this Issue Jan 8, 2018 · 33 comments

Comments

Projects
None yet
6 participants
@FSMaxB

FSMaxB commented Jan 8, 2018

Python 2 will stop receiving support in 2020. Are there any plans already to port syncplay to Python 3?

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jan 10, 2018

Just dropping my 2 cents here, nothing official. We are currently in the middle of another transition, changing the GUI binding to support (eventually) Qt 5. So, in my opinion, it would be better to wait for the official release of PySide2 before thinking to migrate to Python 3. Hopefully, this should happen long before 2020.

That being said, I played a little with 2to3 and syncplay today, and the most annoying compatibility issues that I found so far were caused by the str/Unicode/byte type changes introduced in Python 3. These issues will force to adjust all the player protocols introducing a lot of decode() and encode() in sent or received strings, according to the types that Twisted for Python 3 is now expecting.

This is what I found so far, but maybe someone else is aware of further, much more worrisome, potential problems.

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jan 11, 2018

I'm playing around with syncplay on python3.6 and so far I've been able to easily setup the server and have users connect to it. Chat, Hello seems to work, but when the client receives the timestamp of synchronizing, the server gives the client , which I will have to look into.

Edit: Also getting all the dependencies to work with python3.6 is a pain (PySide2 in particular)

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jan 11, 2018

Good question, @FSMaxB. Python 3 support is not something that I have looked into for many years and no recent decisions have been made as to moving Syncplay to Python 3 by any specific date (or whether we would want to support both in parallel by a specific date). Any support people can provide in terms of helping understand and overcome barriers and assessing whether or not there are reasons for urgency or delay would be appreciated.

From the above comments by @alby128 and @xNinjaKittyx it seems clear there are problems with PySide2 on Python 3. Is the situation of PySide for Python 3 any different to Python 2? In Python 2 my understanding is that PySide1 is fully supported (other than a few OS X quirks) and PySide2 is working if you build it yourself but is not yet stable and accessible enough to be depended upon (especially for Windows).

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jan 11, 2018

As I was working on it a bit more, I finally was able to find a way to get Pyside2 on Python 3.6 through conda packaging (miniconda or anaconda, I used anaconda) And it seems to work fine. It was hell trying to build it myself (as I don't have much experience in that area), and after 2 hours I found out that conda had a package for python 3.6

PySide2 natively (through pip) supports python 3.2, 3.3, 3.4, but it does not support any other versions without having to make your own build, but conda-forge distribution seems to have one that "just works" I was able to start running the client, but.....

There are a lot of issues involving the player APIs. I'm not 100% sure, but in MpcHcApi class, it doesn't seem to like the classes inside classes (it says that __Locks() is not an attribute of class MpcHcApi), but I have to look more into that.

Another issue I was thinking of was py2exe because it never upgraded to python3. But from the top answer from Google, it seems like cx_freeze might be something worth looking into.

From studying the code more, I definitely agree that @alby128 suggestion is considered to move to Qt5 before upgrading to python 3. One issue I ran into was doing a QtDateTime check. You can't use toPython when QtDateTime is Year 0. Nonecheck does not work. False check is working(?).

As for python2 -> python3 syntax changes, here are the ones I found in syncplay's code:

  • All <> must be converted to !=
  • All dict.has_key() operations should be converted to either dict.get(value, defaultvalue) or key in dict depending on the type of code.
  • urllib is split into 4 modules in python 3, so those need be change accordingly
  • ConfigParser had some changes. I completely commented out the ConfigParserSafeUnicode and replaced them with the default ConfigParser.... And I have no idea what the reprimands will be for that.
  • u"" isn't really needed anymore since all strings parsed the same regardless in python3 (accepts raw unicode I believe...) (But it can be left in python 3 to be backwards compatible with python 2.7)
  • Tabs/spaces must never be mixed (I found some mixed indents...Everything preferably should be spaces)
  • dict.iteritems() -> dict.items() and dict.itervalues() -> dict.values()

I literally just went around replacing things and doing minor edits. It starts (at least).

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jan 11, 2018

I was just writing that conda had binaries for PySide2 on Python 2 and 3 for all operative systems. Sorry for the time you wasted trying to compile it, I had to compile PySide2 for macOS myself and I know what you have been through. This will definitely be solved once PySide2 is released and official binaries will be available.

Anyway, it seems that you met the same errors in the player interfaces that I experienced during my tests, so we should probably focus on those when we will attempt this migration. With 2to3 and some minor changes I ran Syncplay and the player (I tried VLC and mpv), but communication between the two was impossible.

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jan 11, 2018

Tonight, I'll probably try to do some more debugging as I think it's partially a server issue.

When I was connecting a brand new python3.6 syncplayServer with a 1.5.0 python2.7 syncplayClient, the timestamp given to the client is always None, which is strange to me. I tried looking at the debug logs, but I didn't spend too much time looking at them. I'll probably do a comparison between a 2.7 server and a 3.6 server to find anomales

Another thing that would be nice to add is to add --debug to the server side just like how it is on the client side. Right now, I'm just adding print() statements. And it would be nice to be able to see debug messages on binaries instead of having to grab source.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jan 15, 2018

@xNinjaKittyx My understanding is that the the server doesn't have debug because (a) we didn't want to encourage invasion of privacy, and (b) for most purposes you can just put debug mode on the client side and get the same info.

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jan 15, 2018

That's understandable.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Apr 24, 2018

I started to do some work about this in my fork, given that the release of PySide2 is approaching (as Python 2 EOL). At this moment, the code contained in https://github.com/albertosottile/syncplay/tree/py3-qtpy-pyside2 works with VLC on macOS 10.12+ with Python 3.6. I tested it locally and using current online servers (running on 2.7) and everything works fine.

This is how I proceeded, you may use the following as guidelines. I used 2to3 to convert the codebase, then I proceeded with the following fixes:

  • commented the Python 2.7 checks in syncplayClient and syncplayServer
  • adapted protocols.py to send and receive bytes via twisted (instead of str)
  • adapted vlc.py to send and receive bytes from VLC streams

Unfortunately, the code ported in this way is not compatible with 2.7 anymore, but since its EOL is quickly approaching, I think that a net transition would probably be more advisable (and definitely easier, given the codebase).

Now I am working with mplayer.py to support mpv, too. I applied the same method that I used for vlc.py, but for some reason the player does not answer to the client, after loading a file, for the next ~40 seconds, then it answers for multiple times all at once, then it "freezes" again. Any idea why? (@Et0h or anyone else, suggestions are welcome)

Basically, an approximate roadmap for this transition could be (IHMO):

  1. wait for a stable release of PySide2
  2. distribute a new version of Syncplay with embedded Pyside2 and Qt 5.x
  3. start the porting of this codebase to Python 3.x (we have to be careful with dependencies)
  4. test all platform and players (Win, macOS, Linux -- VLC, mpv, MPC)
  5. test packing code for distribution (Win and macOS)
  6. distribute a new version of Syncplay with embedded Python 3.x libraries

Note: vlc.py currently uses asyncore which is included in 3.6 but deprecated so, sooner or later, we should port that to asyncio.

Note2: there are issues with 3.x and both py2exe and py2app, so item 5. will not be trivial.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Apr 25, 2018

I am using this issue to keep track of the current status of the fork.

Good news:

  • everything works with VLC and MPC-HC on Windows 10 with Python 3.5 (did not try 3.6 yet).
  • the packaging of .dmg with py2app now works. Indeed, a macOS executable copy of 1.5.3 with embedded Python 3.6 and PySide2 5.9.0a1 (Qt 5.9.5) can be downloaded from here: Syncplay_1.5.3_macOS_py36_pyside2.dmg. Alas, it works only with VLC.

Bad news:

  • mpv on macOS still does not work, even though the player seems to be able to communicate with the client and vice-versa. Hope to find a fix soon with the help of @Et0h and anyone else.
  • py2exe is mostly dead (see here). It is currently impossible to pack a .exe executable for Windows with the codebase in my fork. I am afraid we will have to abandon py2exe and switch to something else. People from PySide2 advised me to use PyInstaller, I will give a look at it in the near future.
@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Apr 25, 2018

As seen in the stackoverflow link and what I've mentioned before, cx_freeze could also be an alternative, but I have no experience in executable packaging.
cx_freeze is also cross platform, which could simplify workflow if it works.

Edit: Looks like PyInstaller is a lot better documented, maintained, and cross-platform. I think that's probably the better choice.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Apr 26, 2018

Just a quick update, I wrote a .spec file for PyInstaller and I was able to generate a .exe that works on Windows 10 x64 with VLC and MPC-HC. I still have to push this to the fork, though. This procedure could be viable to pack the portable release, but all the NSIS part of the current packaging script still has to be adapted to generate the setup release.

Side note: with PyInstaller it is impossible to put all the dependencies in a separate 'lib' folder, as we currently do with py2exe. See here for further information. Hence, the portable release looks a little "uglier" compared to what we have now. This should not be a problem for the setup release.

@Hummer12007

This comment has been minimized.

Contributor

Hummer12007 commented Apr 26, 2018

Re PySide2: it's Qt for Python since a few weeks ago, see http://blog.qt.io/blog/2018/04/13/qt-for-python-is-coming-to-a-computer-near-you/

It seems to me, that they're picking up the pace (5.11 will have it as a technological preview already), so stabilization is not too far away imo.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Apr 26, 2018

Yes, you are right. As far as I know, they are targeting June for the first release. From our side, we are ready to deploy packed .dmg and .exe versions of Syncplay with embedded PySide2 and Qt 5.9.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Apr 27, 2018

Great news, everyone. Thanks to @Et0h, now mpv works with Python 3.x on Windows and macOS. I think this completes the player roster for these two platforms, so we can now begin tests on Linux.

PyInstaller for packing the windows distributable works greatly with the --onedir option, not so good with the --onefile option that creates only a giant .exe file with all the libraries embedded. However, since we need to copy some files to other folders and since we want to provide an installer, I would not recommend the --onefile option for distribution. I still have to code and test the packaging of the server .exe.

So, here there is a recap of what has to be done yet:

  • code and test the packaging of syncplayServer with PyInstaller on Windows
  • create a separate NSIS script to build a Windows installer without using py2exe
  • test syncplayClient on Linux (with VLC and mpv)
  • test server packaging on Windows
  • test the server behavior on all platforms

I will focus on Linux in the next few days, though I can only test with a single distro. Any suggestion on which one I should use?

P.S. Of course, all the codebase will still require extensive and detailed tests from users of all the platforms (and most definitely a beta phase), given that we do not have an automatic test suite to confirm that all the features work.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Apr 27, 2018

Something I was discussing with @albertosottile the other day is that Syncplay's vlc.py currently uses asyncore and asynchat to communicate with VLC via syncplay.lua. According to https://docs.python.org/3/library/asyncore.html and https://docs.python.org/3/library/asynchat.html both asyncore and asynchat these have been deprecated since Python 3.6 in favour of asyncio.

For now that is not an issue as asyncore and asynchat are currently retained for the purposes of backwards compatibility, but in the future we may wish to either go with asyncio for Python 3.4+ or come up with custom threaded STDIN/STDOUT code which does not rely on any of the async* modules.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jul 17, 2018

PR #191 ports the codebase to Python 3.x. Everyone is encouraged to test the code in it on all the platforms. If needed, I can provide links for packed binaries for Windows and macOS.

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jul 17, 2018

@albertosottile Nice.
Does it currently support all players? I may try this later today on Windows 10 w/ MPC-BE/MPC-HC

Binaries would be helpful.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jul 17, 2018

It should have exactly the same features of 1.5.5, so try to stress it as much as possible.

@xNinjaKittyx You can download binaries for Windows from here:
https://bintray.com/alby128/Syncplay/download_file?file_path=Syncplay-1.5.6-Setup.exe
https://bintray.com/alby128/Syncplay/download_file?file_path=Syncplay_1.5.6_Portable.zip

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jul 18, 2018

Not sure if it's just my setup, but I get this error:

Traceback (most recent call last):
  File "syncplayClient.py", line 19, in <module>
  File "C:\projects\syncplay\syncplay\clientManager.py", line 7, in run
  File "C:\projects\syncplay\syncplay\ui\ConfigurationGetter.py", line 449, in getConfiguration
  File "C:\projects\syncplay\syncplay\ui\ConfigurationGetter.py", line 356, in _parseConfigFile
  File "C:\Python34\lib\configparser.py", line 735, in readfp
  File "C:\Python34\lib\configparser.py", line 690, in read_file
  File "C:\Python34\lib\configparser.py", line 1043, in _read
configparser.DuplicateSectionError: While reading from 'C:\\Users\\Dan\\AppData\\Roaming\\syncplay.ini' [line 71]: section 'general' already exists

It also doesn't even show syncplay opening, it just crashes immediately.

Am watching a few anime episodes with my friend. So far seems to work well with MPC-BE.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jul 18, 2018

Did you reinstall over an old Syncplay installation or try to run portable?

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jul 18, 2018

Reinstall on top of an old syncplay.

After installing 1.5.6 and trying to run it, I reinstalled 1.5.5 (It worked), then I reinstalled 1.5.6 again. Still didn't work, so I deleted the config file to make it work.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jul 18, 2018

Weird... I am not seeing this on macOS, perhaps @Et0h can provide some more data about Windows.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jul 18, 2018

The NSIS installer for Syncplay sometimes creates an additional entry for [general] if one already exists and that causes the DuplicateSectionError error in Syncplay.

My understanding this duplicate INI entry occurs when the WriteINIStr commands used at https://github.com/Syncplay/syncplay/blob/master/buildPy2exe.py#L503 are updating a Unicode INI. Maybe it is because it is looking for an ANSI string, I've not tested it.

So we should try and fix the bug with the NSIS installer which allows it to work correctly whether the INI file is ANSI or Unicode.

We also need a way to handle the issue for those who have already installed and have duplicate entries, either through a pre-parser or an except for DuplicateSectionError.

In the meantime, the workaround is to remove the duplicate [general] entry manually.

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jul 18, 2018

It's a little odd because the friend that I was testing this with also has a similar setup to mine (Windows 10) but he didn't have any problems, which is interesting.

I do still have a copy of that faulty .ini (from the recycle bin lol), so I'm gonna try later today to replicate it, then I could post its contents and see if it adds more insight.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jul 18, 2018

@xNinjaKittyx You can check and change the file encoding in Notepad++. I think if you have a Unicode syncplay.ini then causes problems next time you install on that system.

@xNinjaKittyx

This comment has been minimized.

Contributor

xNinjaKittyx commented Jul 19, 2018

I used the faulty .ini, and i didn't have to change the encoding (Notepad++ says UTF-8 BOM)

[general]
language = en
checkforupdatesautomatically = True
lastcheckedforupdates = 2018-02-10 05:43:18.080000

[client_settings]
name = NinjaKitty
room = SuchKek
playerpath = c:\program files (x86)\kcp\mpc-hc\mpc-hc.exe
perplayerarguments = {}
slowdownthreshold = 1.5
rewindthreshold = 4.0
fastforwardthreshold = 5.0
slowondesync = True
rewindondesync = True
fastforwardondesync = True
dontslowdownwithme = False
forceguiprompt = True
filenameprivacymode = SendRaw
filesizeprivacymode = SendRaw
unpauseaction = IfOthersReady
pauseonleave = False
readyatstart = False
autoplayminusers = -1.0
autoplayinitialstate = None
mediasearchdirectories = {}
sharedplaylistenabled = True
loopatendofplaylist = False
loopsinglefiles = False
onlyswitchtotrusteddomains = True
trusteddomains = [u'youtube.com', u'youtu.be', u'video.xx.fbcdn.net', u'video-lga3-1.xx.fbcdn.net']
publicservers = [u'syncplay.pl:8995', u'syncplay.pl:8996', u'syncplay.pl:8997', u'syncplay.pl:8998', u'syncplay.pl:8999']

[gui]
showosd = True
showosdwarnings = True
showslowdownosd = True
showdifferentroomosd = False
showsameroomosd = True
shownoncontrollerosd = False
showdurationnotification = True
chatinputenabled = True
chatinputfontunderline = False
chatinputfontfamily = sans-serif
chatinputrelativefontsize = 24.0
chatinputfontweight = 1.0
chatinputfontcolor = #FFFF00
chatinputposition = Top
chatdirectinput = False
chatoutputfontfamily = sans-serif
chatoutputrelativefontsize = 24.0
chatoutputfontweight = 1.0
chatoutputfontunderline = False
chatoutputmode = Chatroom
chatmaxlines = 7.0
chattopmargin = 25.0
chatleftmargin = 20.0
chatbottommargin = 30.0
chatmoveosd = True
chatosdmargin = 110.0
notificationtimeout = 3.0
alerttimeout = 5.0
chattimeout = 7.0
chatoutputenabled = True

[server_data]
host = 192.168.5.130
port = 8999
password = 

[general]
language=en
CheckForUpdatesAutomatically=True

Looks like there's a duplicate general that has no spaces?

Deleted the duplicate and it opened correctly.

For some reason on my new .ini file created from scratch, the general section is empty:

[general]
language = 
checkforupdatesautomatically = True
lastcheckedforupdates = 
@Et0h

This comment has been minimized.

Contributor

Et0h commented Jul 20, 2018

It sees to be the same bug as reported at https://sourceforge.net/p/nsis/bugs/787/ which is that if there is a UTF-8 file with a BOM chracter at the start then NSIS does not properly recognise the first group. This means that when [general] is the first group NSIS thinks it is (BOM)[general] and so assumes there is no group called [general] and creates a new one.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jul 20, 2018

https://docs.python.org/3/library/configparser.html notes that Python 3.2 moved from strict being False by default to being True by default. This change is why the problem has only become apparent now. If we set strict back to False then it will revert to the old behaviour where Syncplay will merge the duplicate entry with the later entries taking precedence, which I think is the behaviour we want.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jul 20, 2018

Okay, just committed albertosottile@76a6390 which resolves the issue from Syncplay's side.

@albertosottile

This comment has been minimized.

Member

albertosottile commented Jul 20, 2018

Closed by #191. Thanks to everyone who contributed to this. Long live Syncplay.

@Et0h

This comment has been minimized.

Contributor

Et0h commented Jul 22, 2018

@54

This comment has been minimized.

54 commented Jul 31, 2018

Someone might want to update the Linux section on the install guide, by the way. Sat here for a while unsure as to why the gui wouldn't work having not realized we transitioned to Python 3.x ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment