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

Ideas for migrating to Qt5 #152

Closed
albertosottile opened this Issue Oct 18, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@albertosottile
Member

albertosottile commented Oct 18, 2017

This issue serves as a discussion and as a reference about migrating the Syncplay codebase to Qt5.
The release of version 1.5.0 should happen soon, finally supporting macOS with a packed .app and no need of manual installation. Once that is settled, we should have some time to consider this migration.

In my opinion, there are three alternatives at this moment:

  • defer this migration and stick with PySide1.x
  • directly port all the UI code to PySide2
  • port the UI code to Qt.py.

All these strategies have pro and cons, I will try to list them hereafter, being as objective as possible. In the following, I did not include PyQt5 because its open-source license (GPL) is not compatible with the current license of Syncplay.

Defer the migration

  • Pro: postponing is always easy, no changes required.
  • Cons: Qt4 is not supported on any operative system anymore. We already have several patches specifically designed to correct quirks of Qt4 on macOS 10.12- (see PR #145 and #148). Plus, I have not tested Syncplay on the latest version of macOS yet. Any major OS update could break Syncplay entirely. This is not hypothetical, as it happened a year ago with macOS 10.12, see issue #116.

Port the UI code to PySide2

  • Pro: porting everything to PySide2 is not that difficult, due to its syntax. In fact, a working port is already available on alby128/pyside2, though it has not been updated to the latest commits yet. Doing this will keep the complexity of the codebase of Syncplay almost unchanged.
  • Cons: while PySide2 works with the current codebase of Syncplay, it is not ready for production deployment. PySide2 is still under active development and its team has not even released a tagged version so far. in this situation, finding binaries of PySide2 is quite hard, and sometimes they are not even consistent between each other, because they have been compiled with quite different sources.

Port the UI code to Qt.py

Note: Qt.py is a shim that allows to code the UI using a common syntax and supports all the major Qt bindings (PySide1, PySide2, PyQt4, and PyQt5).

  • Pro: Qt.py allows to support at the same time PySide and PySide2, while keeping exactly the same codebase. In this way, the UI works with any of these two bindings, so if one day Qt4 abruptly cease to work, migration to Qt5 would be instantaneous. Qt.py uses the syntax of PySide2, so migrating the code is quite easy. Indeed, a working port is available here, updated to the latest commit of master. This code has already been tested, coupled with PySide or PySide2, on Windows 7+, macOS 10.11+, and Ubuntu 16.04.
  • Cons: some minor code tweaks are still required to allow contemporary compatibility with PySide1 and PySide2 (e.g this one). These issues increase a little the codebase complexity of Syncplay.

Everyone is encouraged to comment this issue with their opinion and ideas about this migration.

@albertosottile

This comment has been minimized.

Show comment
Hide comment
@albertosottile

albertosottile Oct 18, 2017

Member

Now my personal comment on this: I would recommend to move to Qt.py right after the release of v1.5.0-final. The codebase is ready at present in my fork and requires only testing. This porting will allow to continue the coding of Syncplay without worrying about Qt anymore. The day PySide2 becomes sufficiently ready or PySide1 breaks, changing the binding will be equivalent to flip a switch.

Member

albertosottile commented Oct 18, 2017

Now my personal comment on this: I would recommend to move to Qt.py right after the release of v1.5.0-final. The codebase is ready at present in my fork and requires only testing. This porting will allow to continue the coding of Syncplay without worrying about Qt anymore. The day PySide2 becomes sufficiently ready or PySide1 breaks, changing the binding will be equivalent to flip a switch.

@soredake

This comment has been minimized.

Show comment
Hide comment
@soredake

soredake Oct 18, 2017

I vote for Qt.py (with hopes for a possible pyqt5 (I'm even to pay for pyqt5 support)).

so if one day Qt4 abruptly cease to work

In my case qt4 and all packages that depend on it (including syncplay) will be removed in near future.

soredake commented Oct 18, 2017

I vote for Qt.py (with hopes for a possible pyqt5 (I'm even to pay for pyqt5 support)).

so if one day Qt4 abruptly cease to work

In my case qt4 and all packages that depend on it (including syncplay) will be removed in near future.

@Uriziel

This comment has been minimized.

Show comment
Hide comment
@Uriziel

Uriziel Oct 31, 2017

Contributor

PySide2 seems fine, I believe others would leave us with licensing issues.

Contributor

Uriziel commented Oct 31, 2017

PySide2 seems fine, I believe others would leave us with licensing issues.

@albertosottile

This comment has been minimized.

Show comment
Hide comment
@albertosottile

albertosottile Oct 31, 2017

Member

Qt.py is licensed under the MIT license, so it is even more permissive than PySide and PySide2.

If we want to go directly to PySide2, we need to find binaries for it. Currently, the PySide2 team does not provide binaries for any platform. Anaconda is providing some binaries (based on two months old code) here, but these are not usable in macOS bundles due to packaging issues with py2app and conda. In my pyside2 and qtpy-pyside2 branches, I am currently using a custom PySide2 wheel that I built a month ago on my 10.11.6 system. Once again, the absence of tagged versions means that these two Windows and macOS versions of PySide2 will (almost certainly) be different from each other.

Member

albertosottile commented Oct 31, 2017

Qt.py is licensed under the MIT license, so it is even more permissive than PySide and PySide2.

If we want to go directly to PySide2, we need to find binaries for it. Currently, the PySide2 team does not provide binaries for any platform. Anaconda is providing some binaries (based on two months old code) here, but these are not usable in macOS bundles due to packaging issues with py2app and conda. In my pyside2 and qtpy-pyside2 branches, I am currently using a custom PySide2 wheel that I built a month ago on my 10.11.6 system. Once again, the absence of tagged versions means that these two Windows and macOS versions of PySide2 will (almost certainly) be different from each other.

@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Nov 2, 2017

Contributor

@alby128: I've spoken with Uriziel and he's happy with the proposed solutions and your summaries of them, so I think we're just waiting for 1.5.0 to be released and for @daniel-123 to confirm he has no objections and then we can go ahead with your recommendation of Qt.py which also has the support of @soredake and myself.

Contributor

Et0h commented Nov 2, 2017

@alby128: I've spoken with Uriziel and he's happy with the proposed solutions and your summaries of them, so I think we're just waiting for 1.5.0 to be released and for @daniel-123 to confirm he has no objections and then we can go ahead with your recommendation of Qt.py which also has the support of @soredake and myself.

Et0h added a commit that referenced this issue Dec 9, 2017

Merge pull request #162 from alby128/master
Ported GUI code to Qt.py to add Pyside2 support (#147 & #152)
@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Dec 10, 2017

Contributor

Closing as the decision has been made and it has been merged. If there are any new issues please make a new issue, but you can refer back to this thread if needed for context.

Contributor

Et0h commented Dec 10, 2017

Closing as the decision has been made and it has been merged. If there are any new issues please make a new issue, but you can refer back to this thread if needed for context.

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