Merge lp:~savoirfairelinux-openerp/lp-community-utils/pep394 into lp:lp-community-utils

[Sandy Carter (http://www.savoirfairelinux.com)]

Adapted to crushbangs to PEP394 specifications.

The reason for this is added portability since not all distributions use python2 as the default python.
The biggest distro to use python3 as the target of the python symlink is Arch Linux.

`/usr/bin/env python` should only be used when code can be run in both python2 and python3

[Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903)]

review: Approve

[Ronald Portier (Therp) (rportier1962)]

I doubt wether this change is really a good idea.

When running python from a virtualenv, in many cases this relies on a virtualenv specific python command to be the first in the path. When using python2, suddenly the whole virtualenv will be ignored and the script will revert to using the system python.

Therefore the change will break most virtualenv installations, which I think will be much more common than Arch Linux installs.

Or do I overlook something?

review: Needs Information

[Ronald Portier (Therp) (rportier1962)]

Just to add to my previous comment:

I do think it is a good idea to use #!/usr/bin/env but it should be followed by python. For the - at this moment - rare occasions where python3 is the default, it will be necesarry to manipulate the path to use python2 instead.

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

@rportier1962
As you pointed out, /usr/bin/env will take the python from the virtualenv.
If you do a ls -l, you'll notice that python is a symlink to the python2 or python3 executable, depending on if you used virtualenv2 or virtualenv3.
My changes will, in fact, not break any virtualenv installations for that reason.
Now, if we use /usr/bin/env python instead of /usr/bin/env python2, we are potentially breaking the shebang of anyone not in a python2 virtualenv.
I reiterate that /usr/bin/env python2 should be used as is specified in http://www.python.org/dev/peps/pep-0394/.

[Ronald Portier (Therp) (rportier1962)]

Hello Sandy,

Ik rechecked the effect of the different commands on our virtualenv installation running on debian testing.

As can be seen /usr/bin/env python2 is using the system python, even when running from a virtualenv. I think you are using a much newer version of virtualenv, that is creating the symbolic links you are referring to. My point is that a lot of existing - even relatively recent and up to date - installations, do not have those symlinks.

rportier@vanaheim:~$ virtualenv --python=python2.7 test_usr_bin_env
Running virtualenv with interpreter /usr/bin/python2.7
New python executable in test_usr_bin_env/bin/python2.7
Also creating executable in test_usr_bin_env/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
rportier@vanaheim:~$ ls -l
total 24
-rw------- 1 rportier people 1643 Jan 3 15:59 <email address hidden>
-rw------- 1 rportier people 1070 Jan 3 15:59 <email address hidden>
-rw------- 1 rportier people 1751 Jan 3 15:57 <email address hidden>
-rw------- 1 rportier people 4448 Jan 3 16:00 <email address hidden>
drwxr-xr-x 6 rportier people 4096 Feb 7 10:46 test_usr_bin_env
rportier@vanaheim:~$ . test_usr_bin_env/bin/activate
(test_usr_bin_env)rportier@vanaheim:~$ python
Python 2.7.3 (default, Jan 2 2013, 13:56:14)
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/home/rportier/test_usr_bin_env'
>>> quit()
(test_usr_bin_env)rportier@vanaheim:~$ /usr/bin/env python
Python 2.7.3 (default, Jan 2 2013, 13:56:14)
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/home/rportier/test_usr_bin_env'
>>> quit()
(test_usr_bin_env)rportier@vanaheim:~$ /usr/bin/env python2
Python 2.7.3 (default, Jan 2 2013, 13:56:14)
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/usr'

[Virgil Dupras (hsoft)]

Interesting, I don't get the same results (I'm on Arch). It must be virtualenv's version. In any case, it looks like it indeed isn't safe to rely on /usr/bin/env python2.

vdupras@vdupras-thinkpad-x201:~/src/openerp$ . env/bin/activate
(env)vdupras@vdupras-thinkpad-x201:~/src/openerp$ python
Python 2.7.4 (default, Apr 19 2013, 18:28:01)
[GCC 4.7.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/home/vdupras/src/openerp/env'
>>>
(env)vdupras@vdupras-thinkpad-x201:~/src/openerp$ /usr/bin/env python
Python 2.7.4 (default, Apr 19 2013, 18:28:01)
[GCC 4.7.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/home/vdupras/src/openerp/env'
>>>
(env)vdupras@vdupras-thinkpad-x201:~/src/openerp$ /usr/bin/env python2
Python 2.7.4 (default, Apr 19 2013, 18:28:01)
[GCC 4.7.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/home/vdupras/src/openerp/env'
>>>
(env)vdupras@vdupras-thinkpad-x201:~/src/openerp$ deactivate
vdupras@vdupras-thinkpad-x201:~/src/openerp$ virtualenv2 --version
1.11.2
vdupras@vdupras-thinkpad-x201:~/src/openerp$

[Ronald Portier (Therp) (rportier1962)]

Just on the virtualenv version.

This is ours:

rportier@vanaheim:~$ virtualenv --version
1.7.1.2
rportier@vanaheim:~$ virtualenv2 --version
-su: virtualenv2: command not found

[Ronald Portier (Therp) (rportier1962)]

/usr/bin/env python2 on debian stable from a virtualenv picks up the system python, as noted before, but it will be working on debian testing (jessie):

openeyedev@laptop01:~/tmp/buildout_test$ . sandbox/bin/activate
(sandbox)openeyedev@laptop01:~/tmp/buildout_test$ which python
/home/openeyedev/tmp/buildout_test/sandbox/bin/python
(sandbox)openeyedev@laptop01:~/tmp/buildout_test$ virtualenv --version
1.11.2
(sandbox)openeyedev@laptop01:~/tmp/buildout_test$ virtualenv2 --version
bash: virtualenv2: opdracht niet gevonden
(sandbox)openeyedev@laptop01:~/tmp/buildout_test$ /usr/bin/env python2
Python 2.7.6 (default, Jan 11 2014, 17:06:02)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.prefix
'/home/openeyedev/tmp/buildout_test/sandbox'

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

Ronald, I cannot replicate the behaviour:

scarter ~ % virtualenv --version
1.11.2
scarter ~ % python -c "import sys; print(sys.prefix); print(sys.version)"
/usr
3.3.3 (default, Nov 26 2013, 13:33:18)
[GCC 4.8.2]
scarter ~ % python2 -c "import sys; print(sys.prefix); print(sys.version)"
/usr
2.7.6 (default, Nov 26 2013, 12:52:49)
[GCC 4.8.2]
scarter ~ % /usr/bin/env python -c "import sys; print(sys.prefix); print(sys.version)"
/usr
3.3.3 (default, Nov 26 2013, 13:33:18)
[GCC 4.8.2]
scarter ~ % /usr/bin/env python2 -c "import sys; print(sys.prefix); print(sys.version)"
/usr
2.7.6 (default, Nov 26 2013, 12:52:49)
[GCC 4.8.2]
scarter ~ % virtualenv --python=python2.7 test_usr_bin_env
Running virtualenv with interpreter /usr/bin/python2.7
New python executable in test_usr_bin_env/bin/python2.7
Also creating executable in test_usr_bin_env/bin/python
Installing setuptools, pip...done.
scarter ~ % . test_usr_bin_env/bin/activate
(test_usr_bin_env)scarter ~ % python -c "import sys; print(sys.prefix); print(sys.version)"
/home/scarter/test_usr_bin_env
2.7.6 (default, Nov 26 2013, 12:52:49)
[GCC 4.8.2]
(test_usr_bin_env)scarter ~ % python2 -c "import sys; print(sys.prefix); print(sys.version)"
/home/scarter/test_usr_bin_env
2.7.6 (default, Nov 26 2013, 12:52:49)
[GCC 4.8.2]
(test_usr_bin_env) scarter ~ % /usr/bin/env python -c "import sys; print(sys.prefix); print(sys.version)"
/home/scarter/test_usr_bin_env
2.7.6 (default, Nov 26 2013, 12:52:49)
[GCC 4.8.2]
(test_usr_bin_env)scarter ~ % /usr/bin/env python2 -c "import sys; print(sys.prefix); print(sys.version)"
/home/scarter/test_usr_bin_env
2.7.6 (default, Nov 26 2013, 12:52:49)
[GCC 4.8.2]
(test_usr_bin_env)scarter ~ % deactivate

Whether or not we use /usr/bin/env or not doesn't really matter to me. Most of the shebangs in this MP don't even use it. I am just following the official specifications of PEP394.
What matters to me is python2 over python.
A script should not expect the user to change the /usr/bin/python symlink.

[Virgil Dupras (hsoft)]

So, from what I understand, using a standard shebang line would prevent people still on debian stable and under a virtualenv from executing those scripts directly. They would now have to execute them as "python script.py".

That doesn't sound like a big deal to me, and even less a reason to keep using a non-standard shebang line.

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

They would be better off using `python2 script.py` ;)

[Virgil Dupras (hsoft)]

I doubt that debian stable has "python2" right now. I don't think OpenERP will even *exist* anymore the day debian stable symlinks "python" to python3.

[Ronald Portier (Therp) (rportier1962)]

As the unmodified scripts are breaking virtualenv anyway, and as oldstyle virtualenvs - like on debian stable - can use the scripts in the way Virgil suggested (python script.py), I think taking everything into consideration that the merge request is really OK.

review: Approve (code review and partial testing)

[Stefan Rijnhart (Opener) (stefan-opener)]

I confirm that virtualenv creates the local bin/python2 symlink in later versions. I suspect it's this commit[1], which would correspond to release 1.8.3. Current version in stable is 1.7.1. This is also the same version in the latest Ubuntu server LTS, so I would suggest we could stick to '/usr/bin/env python' for now instead of '/usr/bin/env python2'. What do you think?

[1] https://github.com/pypa/virtualenv/commit/03969cbcf00a016a29516ca02fb796845f33fbee

review: Needs Information

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

If we do use python in lieu of python2, I must urge us to make all scripts python2 and python3 compatible [1]:

> The more general python command should be installed whenever any version of Python is installed and should invoke the same version of Python as either python2 or python3.

[1] http://www.python.org/dev/peps/pep-0394/#recommendation

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

> [Scripts that are deliberately written to be source compatible with both Python 2.x and 3.x] may continue to use python on their shebang line without affecting their portability.

[Stefan Rijnhart (Opener) (stefan-opener)]

Yes, there are valid counterarguments I suppose. I'll abstain because I don't pretend to be able to decide between the pros and cons.

review: Abstain

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

I'm not favorable to the use of /usr/bin/python2 in an ecosystem where /usr/bin/python is used everywhere to mean python 2.6 (which is the current compatibility focus for the OpenERP ecosystem).

Users of distributions using python3.x as default /usr/bin/python have to work around this already. I'm in favor of of changing #!/usr/bin/python to #!/usr/bin/env python but not python2.

This is a personal opinion statement, not a technical judgement.

review: Disapprove (political opinion)

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

@alexandre-fayolle-c2c I urge you to reconsider.
We, users of distros using python3 don't usually have to do any workaround since using python2 is standard in most python project.
Even openerp using the buildout recipe using the anybox recipe will put the following python2 shebang:
#!/usr/bin/python2

The whole reason I did this MP is, being a python3 default user, these scripts surprised me when they did not work the same way any other python script I use, locally and from external sources thanks to the PEP394 convention.

[Stefan Rijnhart (Opener) (stefan-opener)]

> Even openerp using the buildout recipe using the anybox recipe will put the following python2 shebang:

FYI: the anybox recipy, by means of the buildout scripts mechanism will
put in the python interpreter with which the buildout is being build.

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

For those who want /usr/bin/env, but have do not necessarily agree with using python2, you can approve the following MP:
https://code.launchpad.net/~savoirfairelinux-openerp/lp-community-utils/pep394-env/+merge/210217

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

review: Abstain

[Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c)]

Hi,

Honestly no idea what's better here, so I go for abstain as well..

review: Abstain

[Leonardo Pistone (lepistone)]

If I understand correctly, we are trading a small smoothness advantage for old distributions vs. more up-to-date ones that received pep394.

I lean a bit for calling python2 explicitly. I prefer those with old distributions to go through some (easy) hoops to comply with future-proof pep's that the contrary.

In any case, these scripts are used by people that can figure out how to work around the problem in both cases, so I won't fight over this.

review: Approve

Unmerged revisions

24. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-02-03

[IMP] PEP 394 fix hashbangs

=== modified file 'clone_mp_to_community.py'
--- clone_mp_to_community.py	2013-09-22 14:36:16 +0000
+++ clone_mp_to_community.py	2014-02-03 16:51:43 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python2
 # -*- coding: utf-8 -*-
 #
 #
 
=== modified file 'merge_mp.py'
--- merge_mp.py	2013-09-22 14:36:16 +0000
+++ merge_mp.py	2014-02-03 16:51:43 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python2
 # -*- coding: utf-8 -*-
 #
 #
 
=== modified file 'openerp-nag'
--- openerp-nag	2013-09-22 14:36:16 +0000
+++ openerp-nag	2014-02-03 16:51:43 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 # Copyright 2012 Canonical Ltd.
 # Written by:
 #   Zygmunt Krynicki <[e-mail]>
 
=== modified file 'replay_missing.py'
--- replay_missing.py	2013-09-22 14:36:16 +0000
+++ replay_missing.py	2014-02-03 16:51:43 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python2
 # -*- coding: utf-8 -*-
 #
 #