Mach should use the python that called it when calling other things

RESOLVED FIXED in mozilla27

Status

RESOLVED FIXED
5 years ago
6 months ago

People

(Reporter: mossop, Assigned: gps)

Tracking

Trunk
mozilla27

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
0:00.33 C:/mozilla-build/msys/bin/sh.exe -c c:/mozilla/source/trunk/build/pymake/make.py
    -j4 -s install-tests
     0:01.42 /usr/bin/sh: c:/Users/Dave: No such file or directory
     0:01.42 c:\mozilla\build\nightly\Makefile:68:0: command 'MOZBUILD_BACKEND_CHECKED= c:/Users/Dave Townsend/python/Scripts/python.exe c:/mozilla/source/trunk/build/pymake/pymake/../make.py -C js/src backend.RecursiveMakeBackend.built' failed, return code 127
     0:01.42 <install-tests>: Found error

Here mach has decided to use the python in the PATH to run pymake and fails because it can't handle spaces in the path. The python I used to run mach here was the one from mozilla-build and doesn't have spaces in the path. It should just use that one.
(Assignee)

Comment 1

5 years ago
Yeah, this is a real bug. base.py's run_make() will invoke make.py as a command. It's shebang is "#!/usr/bin/env python", which is not ideal.
Component: mach → Build Config
(Assignee)

Comment 2

5 years ago
Created attachment 810174 [details] [diff] [review]
Invoke pymake with virtualenv or calling python, not $PATH python

Will need to test this before landing.
Attachment #810174 - Flags: review?(mh+mozilla)
(Assignee)

Updated

5 years ago
Assignee: nobody → gps
(Assignee)

Comment 3

5 years ago
Comment on attachment 810174 [details] [diff] [review]
Invoke pymake with virtualenv or calling python, not $PATH python

Please report how this works.
Attachment #810174 - Flags: feedback?(dtownsend+bugmail)
Comment on attachment 810174 [details] [diff] [review]
Invoke pymake with virtualenv or calling python, not $PATH python

Review of attachment 810174 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/base.py
@@ +464,5 @@
> +            ve_python = self.virtualenv_manager.python_path
> +            if os.path.exists(ve_python):
> +                return [ve_python, make_py]
> +            else:
> +                return [sys.executable, make_py]

That doesn't look like a great thing to do. Because if that runs before configure (in case of a clobber build), it's going to use system python for the entire build. Further builds will use the virtualenv one. The discrepancy is going to hurt.
Attachment #810174 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 5

5 years ago
So, the only way we can reliably use the virtualenv python is if the virtualenv python is guaranteed to exist. And that means moving virtualenv population out of configure and into client.mk or mach. That's something I've been wanting to do, but that's not something I'd like to block this bug.

Will you accept a patch that always uses sys.executable?
(In reply to Gregory Szorc [:gps] from comment #5)
> Will you accept a patch that always uses sys.executable?

yes
(Assignee)

Comment 7

5 years ago
Created attachment 810291 [details] [diff] [review]
Invoke pymake with virtualenv or calling python, not $PATH python

I removed the virtualenv from the equation.
Attachment #810291 - Flags: review?(mh+mozilla)
(Assignee)

Updated

5 years ago
Attachment #810174 - Attachment is obsolete: true
Attachment #810174 - Flags: feedback?(dtownsend+bugmail)
Comment on attachment 810291 [details] [diff] [review]
Invoke pymake with virtualenv or calling python, not $PATH python

Review of attachment 810291 [details] [diff] [review]:
-----------------------------------------------------------------

Don't forget to adjust the commit message.
Attachment #810291 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5897df7c56e2
Status: NEW → ASSIGNED
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/5897df7c56e2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27

Updated

6 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.