Closed
Bug 957721
Opened 10 years ago
Closed 10 years ago
Change mach's shebang to use python2.7, not python
Categories
(Firefox Build System :: Mach Core, enhancement)
Firefox Build System
Mach Core
Tracking
(firefox-esr31 fixed)
RESOLVED
FIXED
mozilla32
Tracking | Status | |
---|---|---|
firefox-esr31 | --- | fixed |
People
(Reporter: gps, Assigned: glandium)
References
Details
Attachments
(2 files, 1 obsolete file)
856 bytes,
patch
|
glandium
:
review+
gaston
:
feedback+
|
Details | Diff | Splinter Review |
1014 bytes,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
mach's shebang is looking for 'python'. OpenBSD only has 'pythonX.Y' and 'python' can be ambiguous and pick up other python versions on the system. Let's switch the shebang to: #!/usr/bin/env python2.7
Reporter | ||
Comment 1•10 years ago
|
||
Assigning review to glandium because he usually has interesting opinions on low-level operating systems stuff like this :) The only alternative I see is we have build/mach_bootstrap.py silently exec python2.7 if found and the current Python is not 2.7. But that doesn't solve the problem where 'python' isn't on PATH.
Attachment #8357289 -
Flags: review?(mh+mozilla)
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment 2•10 years ago
|
||
Comment on attachment 8357289 [details] [diff] [review] Switch mach's shebang to look for python2.7 I, for one, would love this, since it'd allow me to use mach directly: dawn:~/src/mozilla-central/ $./mach env: python: Is a directory patched: dawn:~/src/mozilla-central/ $./mach usage: mach [global arguments] command [command arguments]
Attachment #8357289 -
Flags: feedback+
Reporter | ||
Comment 3•10 years ago
|
||
I should mention that long-term I envision we'll likely have some kind of "build environment" that modifies shell variables so things like mach and python "just work." Currently, ./mach kinda does that at run-time, which is extremely hacky in terms of python land.
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8357289 [details] [diff] [review] Switch mach's shebang to look for python2.7 Review of attachment 8357289 [details] [diff] [review]: ----------------------------------------------------------------- I'm okayish with this, but I have concerns that there are setups where python 2.7 is installed by python2.7 is not available. I think I've seen setups with python27 instead of python2.7, but maybe I'm mixing things up because python27 is a recurring name for python 2.7 packages (homebrew and redhat, at least). As a side note, aiui most linux distros nowadays settled on providing python 2.x as python and python 3.x as python3 for use in shebangs, where some value for python 2.x is better than a specific python2.x. OpenBSD should do that too. "Fortunately", mach only supports python 2.7 at this point, so as long as every python 2.7 comes with python2.7, we're fine, but that comes back to my concern in the first sentence.
Attachment #8357289 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #3) > I should mention that long-term I envision we'll likely have some kind of > "build environment" that modifies shell variables so things like mach and > python "just work." Currently, ./mach kinda does that at run-time, which is > extremely hacky in terms of python land. This tendency to use "activate" scripts is *very* irritating. Please keep it out of the build system.
Reporter | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/faafb9526e32 Let's see what happens. We can back this out if it causes too much badness. In my ideal world, we have the build system / mach reporting statistics back to Mozilla. We could add a probe that looks for {python, python2, python2.7, python27} and their respective versions and make a data driven decision that works for the plurality of users. We may inevitably just switch this back to "python" and have mach exec back into an appropriate Python interpreter if the current one isn't acceptable. Alternatively, people can cp mach into $PATH and change the shebang.
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/faafb9526e32
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment 8•10 years ago
|
||
Backed out for causing bug 960042.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla29 → ---
Comment 9•10 years ago
|
||
Merge of backout: https://hg.mozilla.org/mozilla-central/rev/cdfa438e9817
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Reporter | ||
Comment 11•10 years ago
|
||
Boo. We /could/ update MozillaBuild to ensure a python2.7.exe is installed. But I'm not sure it is worth it. Tempted to WONTFIX or LATER this bug.
Reporter | ||
Comment 12•10 years ago
|
||
Or change the summary to have mach or build/mach_bootstrap.py exec a found Python 2.7 binary if the current interpreter isn't 2.7.
Status: REOPENED → NEW
Comment 13•10 years ago
|
||
Don't know what was wrong before that led to bug 960042, but I just re-tested this now and it's working fine.
Keywords: checkin-needed
Updated•10 years ago
|
Target Milestone: mozilla29 → ---
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c72c5ea486c
Keywords: checkin-needed
Comment 15•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3c72c5ea486c
Status: NEW → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 16•10 years ago
|
||
[/c/src-gecko/inbound]$ mach dxr qimportbz /usr/bin/env: python2.7: No such file or directory [/c/src-gecko/inbound]$ python2.7 bash: python2.7: command not found I'm using a separately installed version of Python (from https://www.python.org/download/), since that in Mozilla-build is out of date: [/c/src-gecko/inbound]$ /c/mozilla-build/python/python --version Python 2.7.5 [/c/src-gecko/inbound]$ python --version Python 2.7.6 We should get this fixed in upstream Python first.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 17•10 years ago
|
||
I've filed http://bugs.python.org/issue21506 to get the official python.org msi releases to mklink (symlink) python2.7.exe to python.exe. However for now this is breaking workflows, so backed out: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/28ceedf9734d
Assignee | ||
Comment 18•10 years ago
|
||
> However for now this is breaking workflows, so backed out:
> remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/28ceedf9734d
Huh? Who is not using mozilla-build?
Comment 19•10 years ago
|
||
Myself for one; I use a newer msys and separately installed (and kept up to date) python, hg etc in parallel with mozilla-build as my primary environment. I accept that build failures on !mozilla-build are no doubt wontfix, however this affects usage of mach that fails outside building - ie: other than building I don't use a mozilla-build environment, since as Ted and others have said several times, it's not intended to be an up-to-date development environment. When performing repo manipulations and other non-building development, it's too much of a pain to switch to Mozilla-Build.
Comment 20•10 years ago
|
||
I imagine this will also break the cygwin folk
Comment 21•10 years ago
|
||
sigh sorry third reply... s/fails/falls/
Assignee | ||
Comment 22•10 years ago
|
||
How about this kind of black magic?
Attachment #8422879 -
Flags: review?(mshal)
Assignee | ||
Updated•10 years ago
|
Assignee: gps → mh+mozilla
Status: REOPENED → ASSIGNED
Comment 23•10 years ago
|
||
Comment on attachment 8422879 [details] [diff] [review] Transform mach into a shell script that reexecutes itself with python I generally prefer staying away from the dark arts, but I don't have a better solution :). However, we will need to pass along the arguments ($@ I guess?) to the python invocation, otherwise you only ever get mach's help message.
Attachment #8422879 -
Flags: review?(mshal) → review-
Assignee | ||
Comment 24•10 years ago
|
||
Doh. "$@" is the magic thing that expands to "$1" "$2" ...
Attachment #8423448 -
Flags: review?(mshal)
Assignee | ||
Updated•10 years ago
|
Attachment #8422879 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8423448 -
Flags: review?(mshal) → review+
Comment 25•10 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #19) > Myself for one; I use a newer msys and separately installed (and kept up to > date) python, hg etc in parallel with mozilla-build as my primary > environment. I submit that we should not support this sort of environment, honestly.
Comment 26•10 years ago
|
||
In any event, rather than jump through hoops to support this, why not just copy python.exe to python2.7.exe in your local install? That feels like an acceptable workaround to support your nonstandard setup.
Comment 27•10 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #25) > I submit that we should not support this sort of environment, honestly. I understand, and to be honest seeing the awfulness of the next best alternative in the attached patch, I'm inclined to just concede and suggest we just reland as it was before. It's just frustrating that the root cause here is that python27 is not actually valid on Windows (ie My environment isn't the custom one, the mozilla-build env is), and won't be until that upstream bug is fixed (and they look pretty reluctant in that upstream bug).
Comment 28•10 years ago
|
||
What about comment 12 as an alternative?
Comment 29•10 years ago
|
||
Honestly I'm not sure I see a compelling reason to fix this bug at all. It would certainly be nice to work in this configuration, but if it causes other issues then maybe it's not worthwhile.
Assignee | ||
Comment 30•10 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #28) > What about comment 12 as an alternative? The problem is that there are places where there is not a python executable at all, only python2/python2.7.
Assignee | ||
Comment 31•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/29a86e1eb5d2
Comment 32•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/29a86e1eb5d2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
status-firefox-esr31:
--- → fixed
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•