After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 732478 - [patch] add python3 support to gnome-shell
[patch] add python3 support to gnome-shell
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks: python3
 
 
Reported: 2014-06-30 13:24 UTC by Bohuslav "Slavek" Kabrda
Modified: 2014-11-07 09:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make gnome-shell python3 compatible (10.09 KB, patch)
2014-06-30 13:24 UTC, Bohuslav "Slavek" Kabrda
none Details | Review
Port gnome-shell to Python 3 only (9.72 KB, patch)
2014-08-29 12:38 UTC, Bohuslav "Slavek" Kabrda
needs-work Details | Review
Fix deficiencies of previous patch (10.26 KB, patch)
2014-08-29 13:44 UTC, Bohuslav "Slavek" Kabrda
reviewed Details | Review
'git format'ted version of the previous patch (10.76 KB, patch)
2014-08-29 14:24 UTC, Bohuslav "Slavek" Kabrda
committed Details | Review

Description Bohuslav "Slavek" Kabrda 2014-06-30 13:24:40 UTC
Created attachment 279600 [details] [review]
Make gnome-shell python3 compatible

Hi, I'm attaching a patch that adds python3 support to gnome-shell, while maintaining python >= 2.6 compatibility. I used the python-six library [1] to do this. It is a very small dependency that is used by most projects that need both python2 and python3 compatibility, so I hope that's ok for gnome-shell, too.
Thanks for considering!

[1] http://pythonhosted.org/six/
Comment 1 Bohuslav "Slavek" Kabrda 2014-08-29 08:37:05 UTC
ust found out that this should block bug 684666...
Comment 2 Florian Müllner 2014-08-29 11:50:29 UTC
(In reply to comment #0)
> Created an attachment (id=279600) [details] [review]
> Make gnome-shell python3 compatible
> 
> Hi, I'm attaching a patch that adds python3 support to gnome-shell, while
> maintaining python >= 2.6 compatibility.

Why? A quick look through https://wiki.gnome.org/Initiatives/GnomeGoals/Python3Porting didn't turn up any other modules that used that approach, so I don't see why we should not just unconditionally move to python3 at this point ...
Comment 3 Bohuslav "Slavek" Kabrda 2014-08-29 11:55:20 UTC
(In reply to comment #2)
> (In reply to comment #0)
> > Created an attachment (id=279600) [details] [review] [details] [review]
> > Make gnome-shell python3 compatible
> > 
> > Hi, I'm attaching a patch that adds python3 support to gnome-shell, while
> > maintaining python >= 2.6 compatibility.
> 
> Why? A quick look through
> https://wiki.gnome.org/Initiatives/GnomeGoals/Python3Porting didn't turn up any
> other modules that used that approach, so I don't see why we should not just
> unconditionally move to python3 at this point ...

The page says "For modules which have to support both python 2 and 3, add a --with-python configure option; look at pygobject for an example ", but I found no list of modules that should keep both 2 and 3 compat - so I assumed it's better to keep it.
I have no problem whatsoever with modifying the patch to be Python 3 only, if that's what you'd like to see :)
Comment 4 Florian Müllner 2014-08-29 12:10:27 UTC
(In reply to comment #3)
> The page says "For modules which have to support both python 2 and 3, add a
> --with-python configure option; look at pygobject for an example "

That makes sense for anything that installs python modules used by other modules, which does not apply to the two small scripts in gnome-shell - so yes, I'd prefer a "real" port to python3.
Comment 5 Bohuslav "Slavek" Kabrda 2014-08-29 12:21:00 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > The page says "For modules which have to support both python 2 and 3, add a
> > --with-python configure option; look at pygobject for an example "
> 
> That makes sense for anything that installs python modules used by other
> modules, which does not apply to the two small scripts in gnome-shell - so yes,
> I'd prefer a "real" port to python3.

It also makes sense for projects that want to be in base installs of distributions that don't want to move to Python 3 yet - on my "Python 3 as a Default in Fedora 22" adventure, I've met few of those. Anyway, I'll send a new python 3 only patch shortly. Thanks.
Comment 6 Florian Müllner 2014-08-29 12:25:50 UTC
(In reply to comment #5)
> It also makes sense for projects that want to be in base installs of
> distributions that don't want to move to Python 3 yet

That's a downstream issue - if they care, they can revert the change or patch out the two scripts completely (neither one is required to run gnome-shell)
Comment 7 Bohuslav "Slavek" Kabrda 2014-08-29 12:38:02 UTC
Created attachment 284813 [details] [review]
Port gnome-shell to Python 3 only

I did some simple testing, everything seems to work fine on Python 3.
Comment 8 Florian Müllner 2014-08-29 12:56:48 UTC
Review of attachment 284813 [details] [review]:

- configure still asks for a python version >= 2.5
 - check-for-missing.py is a helper script that is never
   installed, so porting doesn't seem too urgent; however
   if you do, than don't assume that /usr/bin/python is
   python3
Comment 9 Bohuslav "Slavek" Kabrda 2014-08-29 13:44:44 UTC
Created attachment 284816 [details] [review]
Fix deficiencies of previous patch

This fixes:
- hashbang usage in check-for-missing.py
- checking for Python 3 (only) in configure.ac

Thanks for the review.
Comment 10 Florian Müllner 2014-08-29 14:06:53 UTC
Review of attachment 284816 [details] [review]:

LGTM - can you attach a git-format'ted version? (Or I could do this before pushing if you want me to)
Comment 11 Bohuslav "Slavek" Kabrda 2014-08-29 14:24:59 UTC
Created attachment 284818 [details] [review]
'git format'ted version of the previous patch
Comment 12 Florian Müllner 2014-08-29 14:27:19 UTC
Review of attachment 284818 [details] [review]:

LGTM