885233 - Use python2 when available in activation script

Status: RESOLVED WONTFIX

(Add-on SDK Graveyard :: General, defect, P3)

Description

[marek]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130514164240

Steps to reproduce:

Just ran bin/activate on and Arch Linux machine. The activation script will nicely say that you are using Python3 and it's not ready for that.




Actual results:

When I then ran bin/cfx it failed with an error because of the difference between python2 and python3


Expected results:

I believe that when running bin/activate it should detect that `which python2` actually exists and change every occurence of python to python2.

The Arch Linux AUR PKGBUILD solves this with a oneliner

  grep -Rl python . | xargs sed -ri 's/([^!]|^)python(\s|$)/\1python2\2/g'

which I think should be also added to the activation script along with the detection code described above. Since it's also written in bash it should not be a big technical issue.

If you decide this issue is worth fixing I'd like to try to fix it.

Thanks

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

We'd like to have you try to fix it. :)

Comment 2

[marek]

I have submitted the pull request here.

https://github.com/mozilla/addon-sdk/pull/1088

In there I tried (and to my understanding) also fixed it.

Please let me know if it's good enough.

Comment 3

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Hey Brian, would you have some time to review this patch ? I don't think we do really have python people on a team to reason about this changes.

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Cc-ing Mossop, since he should have a better idea who can review this.

Comment 5

[Brian Warner [:warner :bwarner]]

Comments added to the pull request.. the idea is ok, but the patch is a bit overkill IMO, there's probably a gentler way to do it.

Comment 6

[Dave Townsend [:mossop]]

Attachment: pull request

Sorry that this slipped by me, I'll get this reviewed.

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 8334845 [details] [review]
pull request

I don't have a problem with how it selects which python in activate but modifying bin/cfx as part of that isn't acceptable.

Comment 8

[Joseph Booker]

I submitted a PR on github at https://github.com/mozilla/addon-sdk/pull/1602 for this issue, but perhaps it was ignored since I didn't bring it up here.

This works as long as all the platforms supported are new enough to have a python2 binary. Are there any systems in the last ~ three years that don't have it?

Comment 9

[Joseph Booker]

Attachment: PR1604.patch

Adding a patch for this bug. I can rebase on master if needed, although I can't run testcfx on master as is. See https://github.com/mozilla/addon-sdk/pull/1602

Comment 10

[Erik Vold [:erikvold] (please needinfo? me)]

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