servo: Merge #13151 - Make sure that mach gets the correct paths of executables in virtualenv (from Wafflespeanut:mach_cleanup); r=Ms2ger
authorRavi Shankar <wafflespeanut@gmail.com>
Thu, 01 Sep 2016 06:58:50 -0500
changeset 339631 056dd19aa8a525a034f5efa4e07b37833b2f6065
parent 339630 787d4eab10c399164ef754af22cfa44ad7bd3d94
child 339632 d4e17e62128b60532ef046afdc991d5e405b10fc
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMs2ger
servo: Merge #13151 - Make sure that mach gets the correct paths of executables in virtualenv (from Wafflespeanut:mach_cleanup); r=Ms2ger <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #13141 (github issue number if applicable). <!-- Either: --> - [x] These changes do not require tests because it's a cleanup related to `mach` <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: e699d9bfad6ce5e1b0cf31d5a6d06512e267f41a
servo/mach
servo/python/mach_bootstrap.py
--- a/servo/mach
+++ b/servo/mach
@@ -7,20 +7,19 @@
 # such that the script starts with the shell and is reexecuted with
 # the right python.
 '''which' python2.7 > /dev/null 2> /dev/null && exec python2.7 "$0" "$@" || exec python "$0" "$@"
 '''
 
 from __future__ import print_function, unicode_literals
 
 import os
-from os import path
 import sys
 
 def main(args):
-    topdir = path.abspath(path.dirname(sys.argv[0]))
+    topdir = os.path.abspath(os.path.dirname(sys.argv[0]))
     sys.path.insert(0, os.path.join(topdir, "python"))
     import mach_bootstrap
     mach = mach_bootstrap.bootstrap(topdir)
     sys.exit(mach.run(sys.argv[1:]))
 
 if __name__ == '__main__':
     main(sys.argv)
--- a/servo/python/mach_bootstrap.py
+++ b/servo/python/mach_bootstrap.py
@@ -1,19 +1,19 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import print_function, unicode_literals
 
 import os
 import platform
-import subprocess
 import sys
 from distutils.spawn import find_executable
+from subprocess import PIPE, Popen
 
 SEARCH_PATHS = [
     os.path.join("python", "tidy"),
     os.path.join("tests", "wpt"),
     os.path.join("tests", "wpt", "harness"),
 ]
 
 # Individual files providing mach commands.
@@ -21,17 +21,16 @@ MACH_MODULES = [
     os.path.join('python', 'servo', 'bootstrap_commands.py'),
     os.path.join('python', 'servo', 'build_commands.py'),
     os.path.join('python', 'servo', 'testing_commands.py'),
     os.path.join('python', 'servo', 'post_build_commands.py'),
     os.path.join('python', 'servo', 'package_commands.py'),
     os.path.join('python', 'servo', 'devenv_commands.py'),
 ]
 
-
 CATEGORIES = {
     'bootstrap': {
         'short': 'Bootstrap Commands',
         'long': 'Bootstrap the build system',
         'priority': 90,
     },
     'build': {
         'short': 'Build Commands',
@@ -71,100 +70,98 @@ CATEGORIES = {
     'disabled': {
         'short': 'Disabled',
         'long': 'The disabled commands are hidden by default. Use -v to display them. These commands are unavailable '
                 'for your current context, run "mach <command>" to see why.',
         'priority': 0,
     }
 }
 
+# Possible names of executables
+PYTHON_NAMES = ["python-2.7", "python2.7", "python2", "python"]
+VIRTUALENV_NAMES = ["virtualenv-2.7", "virtualenv2.7", "virtualenv2", "virtualenv"]
+PIP_NAMES = ["pip-2.7", "pip2.7", "pip2", "pip"]
 
-def _get_exec(*names):
+
+def _get_exec_path(names, is_valid_path=lambda _path: True):
     for name in names:
         path = find_executable(name)
-        if path is not None:
+        if path and is_valid_path(path):
             return path
     return None
 
 
 def _get_virtualenv_script_dir():
     # Virtualenv calls its scripts folder "bin" on linux/OSX/MSYS64 but "Scripts" on Windows
-    if os.name == "nt" and os.path.sep != "/":
+    if os.name == "nt" and os.sep != "/":
         return "Scripts"
     return "bin"
 
 
-# Possible names of executables, sorted from most to least specific
-PYTHON_NAMES = ["python-2.7", "python2.7", "python2", "python"]
-VIRTUALENV_NAMES = ["virtualenv-2.7", "virtualenv2.7", "virtualenv2", "virtualenv"]
-PIP_NAMES = ["pip-2.7", "pip2.7", "pip2", "pip"]
-
-
 def _activate_virtualenv(topdir):
     virtualenv_path = os.path.join(topdir, "python", "_virtualenv")
-    python = _get_exec(*PYTHON_NAMES)
-    if python is None:
-        sys.exit("Python is not installed. Please install it prior to running mach.")
+    check_exec_path = lambda path: path.startswith(virtualenv_path)
+    python = _get_exec_path(PYTHON_NAMES)   # If there was no python, mach wouldn't have run at all!
+    if not python:
+        sys.exit('Failed to find python executable for starting virtualenv.')
 
     script_dir = _get_virtualenv_script_dir()
     activate_path = os.path.join(virtualenv_path, script_dir, "activate_this.py")
     if not (os.path.exists(virtualenv_path) and os.path.exists(activate_path)):
-        virtualenv = _get_exec(*VIRTUALENV_NAMES)
-        if virtualenv is None:
+        virtualenv = _get_exec_path(VIRTUALENV_NAMES)
+        if not virtualenv:
             sys.exit("Python virtualenv is not installed. Please install it prior to running mach.")
 
-        process = subprocess.Popen(
-            [virtualenv, "-p", python, virtualenv_path],
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE)
+        process = Popen([virtualenv, "-p", python, virtualenv_path], stdout=PIPE, stderr=PIPE)
         process.wait()
         if process.returncode:
-            sys.exit("Python virtualenv failed to execute properly: {}"
-                     .format(process.communicate()[1]))
+            out, err = process.communicate()
+            print('Python virtualenv failed to execute properly:')
+            sys.exit('Output: %s\nError: %s' % (out, err))
 
     execfile(activate_path, dict(__file__=activate_path))
 
-    python = find_executable("python")
-    if python is None or not python.startswith(virtualenv_path):
-        sys.exit("Python virtualenv failed to activate.")
+    python = _get_exec_path(PYTHON_NAMES, is_valid_path=check_exec_path)
+    if not python:
+        sys.exit("Python executable in virtualenv failed to activate.")
 
     # TODO: Right now, we iteratively install all the requirements by invoking
     # `pip install` each time. If it were the case that there were conflicting
     # requirements, we wouldn't know about them. Once
     # https://github.com/pypa/pip/issues/988 is addressed, then we can just
     # chain each of the requirements files into the same `pip install` call
     # and it will check for conflicts.
     requirements_paths = [
         os.path.join("python", "requirements.txt"),
         os.path.join("tests", "wpt", "harness", "requirements.txt"),
         os.path.join("tests", "wpt", "harness", "requirements_firefox.txt"),
         os.path.join("tests", "wpt", "harness", "requirements_servo.txt"),
     ]
+
     for req_rel_path in requirements_paths:
         req_path = os.path.join(topdir, req_rel_path)
         marker_file = req_rel_path.replace(os.path.sep, '-')
         marker_path = os.path.join(virtualenv_path, marker_file)
+
         try:
             if os.path.getmtime(req_path) + 10 < os.path.getmtime(marker_path):
                 continue
         except OSError:
             pass
 
-        pip = _get_exec(*PIP_NAMES)
-        if pip is None:
-            sys.exit("Python pip is not installed. Please install it prior to running mach.")
+        pip = _get_exec_path(PIP_NAMES, is_valid_path=check_exec_path)
+        if not pip:
+            sys.exit("Python pip is either not installed or not found in virtualenv.")
 
-        process = subprocess.Popen(
-            [pip, "install", "-q", "-r", req_path],
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE)
+        process = Popen([pip, "install", "-q", "-r", req_path], stdout=PIPE, stderr=PIPE)
         process.wait()
         if process.returncode:
-            sys.exit("Pip failed to execute properly: {}"
-                     .format(process.communicate()[1]))
+            out, err = process.communicate()
+            print('Pip failed to execute properly:')
+            sys.exit('Output: %s\nError: %s' % (out, err))
 
         open(marker_path, 'w').close()
 
 
 def _ensure_case_insensitive_if_windows():
     # The folder is called 'python'. By deliberately checking for it with the wrong case, we determine if the file
     # system is case sensitive or not.
     if _is_windows() and not os.path.exists('Python'):
@@ -194,18 +191,17 @@ def bootstrap(topdir):
     # We don't support paths with spaces for now
     # https://github.com/servo/servo/issues/9442
     if ' ' in topdir:
         print('Cannot run mach in a path with spaces.')
         print('Current path:', topdir)
         sys.exit(1)
 
     # Ensure we are running Python 2.7+. We put this check here so we generate a
-    # user-friendly error message rather than a cryptic stack trace on module
-    # import.
+    # user-friendly error message rather than a cryptic stack trace on module import.
     if not (3, 0) > sys.version_info >= (2, 7):
         print('Python 2.7 or above (but not Python 3) is required to run mach.')
         print('You are running Python', platform.python_version())
         sys.exit(1)
 
     _activate_virtualenv(topdir)
 
     def populate_context(context, key=None):
@@ -216,15 +212,14 @@ def bootstrap(topdir):
         raise AttributeError(key)
 
     sys.path[0:0] = [os.path.join(topdir, path) for path in SEARCH_PATHS]
     import mach.main
     mach = mach.main.Mach(os.getcwd())
     mach.populate_context_handler = populate_context
 
     for category, meta in CATEGORIES.items():
-        mach.define_category(category, meta['short'], meta['long'],
-                             meta['priority'])
+        mach.define_category(category, meta['short'], meta['long'], meta['priority'])
 
     for path in MACH_MODULES:
         mach.load_commands_from_file(os.path.join(topdir, path))
 
     return mach