Bug 885233 - Use python2 when available in activation script

[mrshu]

The following pull requests fixes the bug 885233.

The general procedure is as follows:

If there is a python2 executable present in the system the activation script will use it and change every occurrence of python on lines like #!/usr/bin/env python.

This approach has been used for long for the Arch Linux's PKGBUILD which can be found in the AUR (https://aur.archlinux.org/packages/ad/addon-sdk/PKGBUILD)

Please note that running source bin/activate takes a bit longer to run for the first time (around 3 seconds on my machine) but then uses the python2 executable without any delay.

[warner]

Am I correct in understanding that this recursively edits every single file in the source tree (including JS, markdown, even .PNGs) and replaces "python" with "python2"? That sounds overkill.. wouldn't it be sufficient to only modify bin/cfx? I think that's the only entrypoint.

I'm a bit nervous about having bin/activate edit source code when it runs, but I suppose it's not too bad. One downside is that people using the SDK from a git checkout will observe some files being modified, so developers will need to pay attention and not commit the modified version.

It might be safer to have the script replace the first line of bin/cfx with a shbang that references the specific path at which "python2" was found, rather than using sed (and thus modifying all sorts of internal strings, including comments).

Also, I'd recommend having the script run python -V to test whether it's python2.x or 3.x, rather than assuming that the presence of python2 automatically means python is 3.x .. that would reduce the frequency of making this change on a system where python==python2!=python3. Maybe start with a loop that tests python and python2 (in that order), until it finds one where -V returns Python 2*, then if the name isn't python, modify bin/cfx.

[mrshu]

Sorry for this late reply.

Am I correct in understanding that this recursively edits every single file in the source tree (including JS, markdown, even .PNGs) and replaces "python" with "python2"? That sounds overkill.. wouldn't it be sufficient to only modify bin/cfx? I think that's the only entrypoint.

You are absolutely correct and I do agree. Will change the code to do so.

I'm a bit nervous about having bin/activate edit source code when it runs, but I suppose it's not too bad. One downside is that people using the SDK from a git checkout will observe some files being modified, so developers will need to pay attention and not commit the modified version.

Hmm, I understand where you nervousness comes from. Is there a better way of doing something like this? If not git update-index --assume-unchanged will have to be used on bin/cfx.

It might be safer to have the script replace the first line of bin/cfx with a shbang that references the specific path at which "python2" was found, rather than using sed (and thus modifying all sorts of internal strings, including comments).

Very good point, will do it that way.

Also, I'd recommend having the script run python -V to test whether it's python2.x or 3.x, rather than assuming that the presence of python2 automatically means python is 3.x .. that would reduce the frequency of making this change on a system where python==python2!=python3. Maybe start with a loop that tests python and python2 (in that order), until it finds one where -V returns Python 2*, then if the name isn't python, modify bin/cfx.

Well, I don't very much see what would this be good for, python2 should work just the same if python==python2. Since we agreed above that just one line will be changed it won't be expensive in terms of time/frequency. Also, this change will not happen if python2 is already used in bin/cfx. Did I miss something?

I can only think of one scenario where this thinking fails and that is when python2 -V returns Python 3.*.

[Mossop]

Changing bin/cfx as a part of the activation script isn't something we should do. At the least it will cause developers to accidentally submit those changes in patches

[erikvold]

Sorry we won't be releasing any new versions of cfx, jpm is the replacement https://www.npmjs.com/package/jpm