git-cola/git-cola

Modernization: Python 3 and Qt5

hsoft opened this issue · 17 comments

I was wondering if there were any Python3 / Qt5 modernization plans for git-cola. If there isn't, I would like to volunteer for that task. There are many paths we can take for that and I wanted to get a discussion started.

My thought is that since git-cola is targeted at developers, there isn't many chances that they run on a system that doesn't have Python 3, so we wouldn't need to keep the code python2-compatible.

Qt5, however, is another story. There hasn't been an Ubuntu LTS release with it yet, so having it as a dependency now might be a bit too soon.

The plan I have in mind is a 3 step modernization process, each of which resulting in a feature-release (1.10, 1.11, 1.12):

  • 1.10: Python 2 modernization (future imports and all) and using PyQt's new api. Drop Python 2.5.
  • 1.11: Python 3
  • 1.12: Qt5

Of course, that's only a proposal I make to get the discussion started. What do you think?

I agree that Qt5 is another story. Similarly, it could be argued that we could port to PySide instead of PyQt5, but I don't really have a strong feeling on which way would be best for the future, We'll see where the community goes and we should probably follow suit if there's a clear, obvious winner.

I think Python2 modernization (having a 2+3 compatible codebase) would be a great first step. I've done it on other (not quite as large) code bases with good success, and I imagine that it would probably work out pretty well.

I've already consciously tried to make the internals as amenable to the future as possible -- any data (bytes) that come from the outside goes through a decode step and everything that leaves the internals goes through encode so that the outside sees only bytes. In other words, our internals are already a nice strict unicode sandwich. This should hopefully make supporting 3.x much easier.

I think it would make sense to drop Python2.5 support if we gain 3.x support in the process. I'm imagining that we could probably get pretty far with a 2.6-3.x compatible codebase. Once we're there then we can consider whether it's worth jumping whole hog onto 3.x, but I'd probably be more inclined to keep compatibility if possible since there hopefully won't be much downside.

Thanks for bringing this up; yeah I'm a procrastinator, and python3.x isn't really on our radar at $dayjob yet, so it hasn't been a high priority for me, but it'll need to happen eventually. 👍

Well then, if you don't mind, I'll get started on python2 modernization + PyQt's new api (the latter probably being the trickiest part).

Oops, I misclicked.

Awesome, thanks. Yeah, the new-style signals+slots is pretty nice. It's probably possible to have pyqt+pyside compatible code using a shim, which would be an interesting experiment, but that's probably a separate topic. Looking forward to it. 👍

I ended up going straight for Python 3 support and left the "pyqt new api" part for later. I tested the app to the best of my capabilities and I think I've covered a lot of grounds, but I might have missed a few bugs.

Also, the interactive rebasing feature doesn't work for me, even on v1.9.4, so that's a part that I couldn't test at all.

I've started porting to qtpy which seems to be a good way for cola to not care about which library is used, and makes things more flexible for packagers, but I don't see qtpy packaged in fedora (although it is packaged in debian) so I hope it's not a burden on fedora (and other) maintainers to have to package a new dependency.

@comzeradd @kkofler @gcsideal heads up about the upcoming new python dependency. Up until now we've been completely dependency-free.

@hsoft once my port is finished we can close this issue. I suspect this may help #550 so testing from @fny @sitnikovme using PyQt5 and Qt5 would be appreciated once the port is complete, which should be soon.

To be honest, IMHO, you should just use PyQt5 directly. qtpy is just a pointless wrapper layer that is going to have to run through Fedora review, and in the end will not bring us any added functionality. Qt 4 is obsolescent and some distros are already talking about dropping it (it will remain in Fedora for the foresseable future though, as a deprecated compatibility library). I don't see why a packager would want to make git-cola choose PyQt4 over PyQt5. So in the end, qtpy just makes things more painful for us packagers, as it is yet another dependency to package.

Since we are ending the support for Qt 4.8...

http://blog.qt.io/blog/2016/03/16/qt-5-6-released/

Thanks for the thoughts, @kkofler.

For my purposes, I actually need the wrapper because I have existing users on old platforms that I am not yet prepared to abandon. We will also have a mix of PyQt4 and PyQt5 in production at my $dayjob, for at least another year, so being able to operate in both is a huge advantage for me.

That said, I am sympathetic to packagers and have vendored the QtPy dependency, and added a switch to opt-out of the vendoring, so that packagers are not required to introduce any additional packages. Things will continue to work for existing users as-is.

As for why packagers might choose one over another -- while doing this port I ran into known Qt bugs that cause segfaults on exit and not using the wrapper would have made it impossible for me to A/B test the libraries. As it stands, PyQt4 is the most stable choice and PyQt5 (via Qt5) has bugs that prevent us from recommending it as a stable platform at the moment. I'm optimistic that this will change in the future, though.

I will also note that using QtPy made porting the code much easier because I was able to incrementally port the project one file at a time. Had I not used QtPy then I would have immediately ran into a problem because module A uses PyQt4 while module B uses PyQt5.. so I wouldn't call it completely pointless ;-)

A huge thanks to @ccordoba12 @Nodd @astrofrog and @goanpeca for creating QtPy.

The code is written and is now available in the qtpy branch from my fork https://github.com/davvid/git-cola/tree/qtpy

I'll be testing this for the rest of the week and will likely merge it in by the end of the week or early next week.

OS X users, if you have PyQt5 available (via homebrew?) please test it and let me know if it resolves the "large icons" issue.

@davvid, hey, this comes as a pleasant surprise to me! git-cola is my favorite git commit tool on Linux, so I'm glad to know it's going to use one of the projects I contribute to ;-)

I recommend you to vendor qtpy 1.1.0 (once we release it) because it has important improvements and bug fixes over 1.0.2.


qtpy is just a pointless wrapper that is going to have to run through Fedora review, and in the end will not bring us any added functionality

Two things here:

  1. We designed qtpy to be used by Spyder, and now it's a hard dependency of its next major version (3.0, not released yet). So I'm sure it's going to be packaged by all major distros eventually, given Spyder's popularity ;-)
  2. It's a not so pointless package because we also have to support PyQt4 and 5, and eventually we hope to add support for PySide2 too.

@davvid, thanks for the kind words, we are very happy to create tools that are useful by other projects :-).

I will also note that using QtPy made porting the code much easier because I was able to incrementally port the project one file at a time. Had I not used QtPy then I would have immediately ran into a problem because module A uses PyQt4 while module B uses PyQt5

This is precisely why we decided to make this package as this road is common for other projects (not only spyder now :-) ). Even though qt4 is deprecated it will take a while for distros and projects to completely migrate.

@davvid, just a quick note to let you know that we released qtpy 1.1.0 some hours ago.

fny commented

@davvid I pulled the qtpy repo and ran ./bin/git-cola, but nothing has changed. Git cola may still be using PyQt4. Is there a way for me to check which Qt version is being used or force use of PyQt5?

The qtpy docs mention that you can set QT_API to pyqt5 and it will force use of pyqt5. LIkewise pyqt4

fny commented

@goanpeca It looks like qtpy can't find my PyQt5 installation on OS X.

Even after calling QT_API=pyqt5 ./bin/git-cola, print(qtpy.QtCore.__version__) kept returning v4.8.7, so I uninstalled PyQt4 and tried again to see what would happen, and it doesn't look like qtpy can find the PyQt5 installation. I now receive this error: qtpy.PythonQtError: No Qt bindings could be found.

I've installed PyQt5 using brew install pyqt5, and I can access the library at /usr/local/Cellar/pyqt5/5.6.

Any guidance on how to get this to link seamlessly?

Nodd commented

qtpy is just trying import pyqt5, so, you should check that it can be imported in python, and check your PYTHONPATH

Googling around, I think you may need to follow the instructions given for sip's caveats:

==> Caveats
The sip-dir for Python is /usr/local/share/sip.
If you need Python to find the installed site-packages:
  mkdir -p ~/Library/Python/2.7/lib/python/site-packages
  echo '/usr/local/lib/python2.7/site-packages' > ~/Library/Python/2.7/lib/python/site-packages/homebrew.pth

Via: http://pastebin.com/Xj1vSJKE

If you do that then python should then be able to find your modules. You can test to see if it's working correctly by doing:

python -c 'import sip'
python -c 'import PyQt5'