Bug 1473727 - Avoid recreating virtual environment every time by using a unique environment for each Python version; r?dustin draft
authorDave Hunt <dhunt@mozilla.com>
Mon, 09 Jul 2018 14:57:38 +0100
changeset 815604 e8a9c71824c2309e6037519da1ae52b7b01c214e
parent 815592 3d20b0701781731e0f9b08e1cd40ac842f385e03
push id115569
push userbmo:dave.hunt@gmail.com
push dateMon, 09 Jul 2018 13:58:07 +0000
reviewersdustin
bugs1473727
milestone63.0a1
Bug 1473727 - Avoid recreating virtual environment every time by using a unique environment for each Python version; r?dustin This patch uses the PIPENV_PYTHON environment variable to append a suffix to the created virtual environment path according to the version specified. It also uses the PIPENV_DEFAULT_PYTHON_VERSION environment variable to avoid recreating the virtual environment every time. With these changes we are able to switch back and forth between Python versions without the expense of recreating environments, however there is a risk of these environments becoming stale. In this scenario it may be necessary to clobber the virtual environment root within the obj dir. MozReview-Commit-ID: C4vuwNh04CP
python/mach_commands.py
python/mozbuild/mozbuild/base.py
python/mozbuild/mozbuild/virtualenv.py
--- a/python/mach_commands.py
+++ b/python/mach_commands.py
@@ -63,16 +63,17 @@ class MachCommands(MachCommandBase):
 
     @Command('python-test', category='testing',
              description='Run Python unit tests with an appropriate test runner.')
     @CommandArgument('-v', '--verbose',
                      default=False,
                      action='store_true',
                      help='Verbose output.')
     @CommandArgument('--python',
+                     default='2.7',
                      help='Version of Python for Pipenv to use. When given a '
                           'Python version, Pipenv will automatically scan your '
                           'system for a Python that matches that given version.')
     @CommandArgument('-j', '--jobs',
                      default=1,
                      type=int,
                      help='Number of concurrent jobs to run. Default is 1.')
     @CommandArgument('--subsuite',
@@ -94,18 +95,17 @@ class MachCommands(MachCommandBase):
     def run_python_tests(self,
                          tests=None,
                          test_objects=None,
                          subsuite=None,
                          verbose=False,
                          jobs=1,
                          python=None,
                          **kwargs):
-        python = python or self.virtualenv_manager.python_path
-        self.activate_pipenv(pipfile=None, args=['--python', python], populate=True)
+        self.activate_pipenv(pipfile=None, populate=True, python=python)
 
         if test_objects is None:
             from moztest.resolve import TestResolver
             resolver = self._spawn(TestResolver)
             # If we were given test paths, try to find tests matching them.
             test_objects = resolver.resolve_tests(paths=tests, flavor='python')
         else:
             # We've received test_objects from |mach test|. We need to ignore
--- a/python/mozbuild/mozbuild/base.py
+++ b/python/mozbuild/mozbuild/base.py
@@ -752,21 +752,21 @@ class MozbuildObject(ProcessExecutionMix
         self._activate_virtualenv()
         pipenv = os.path.join(self.virtualenv_manager.bin_path, 'pipenv')
         if not os.path.exists(pipenv):
             for package in ['certifi', 'pipenv', 'six', 'virtualenv', 'virtualenv-clone']:
                 path = os.path.normpath(os.path.join(self.topsrcdir, 'third_party/python', package))
                 self.virtualenv_manager.install_pip_package(path, vendored=True)
         return pipenv
 
-    def activate_pipenv(self, pipfile=None, args=None, populate=False):
+    def activate_pipenv(self, pipfile=None, populate=False, python=None):
         if pipfile is not None and not os.path.exists(pipfile):
             raise Exception('Pipfile not found: %s.' % pipfile)
         self.ensure_pipenv()
-        self.virtualenv_manager.activate_pipenv(pipfile, args, populate)
+        self.virtualenv_manager.activate_pipenv(pipfile, populate, python)
 
 
 class MachCommandBase(MozbuildObject):
     """Base class for mach command providers that wish to be MozbuildObjects.
 
     This provides a level of indirection so MozbuildObject can be refactored
     without having to change everything that inherits from it.
     """
--- a/python/mozbuild/mozbuild/virtualenv.py
+++ b/python/mozbuild/mozbuild/virtualenv.py
@@ -545,48 +545,65 @@ class VirtualenvManager(object):
         # This will confuse pip and cause the package to attempt to install
         # against the executing interpreter. By creating a new process, we
         # force the virtualenv's interpreter to be used and all is well.
         # It /might/ be possible to cheat and set sys.executable to
         # self.python_path. However, this seems more risk than it's worth.
         pip = os.path.join(self.bin_path, 'pip')
         subprocess.check_call([pip] + args, stderr=subprocess.STDOUT, cwd=self.topsrcdir)
 
-    def activate_pipenv(self, pipfile=None, args=None, populate=False):
+    def activate_pipenv(self, pipfile=None, populate=False, python=None):
         """Activate a virtual environment managed by pipenv
 
         If ``pipfile`` is not ``None`` then the Pipfile located at the path
         provided will be used to create the virtual environment. If
         ``populate`` is ``True`` then the virtual environment will be
-        populated from the manifest file. The optional ``args`` list will be
-        passed to the pipenv commands.
+        populated from the manifest file. The optional ``python`` argument
+        indicates the version of Python for pipenv to use.
         """
         pipenv = os.path.join(self.bin_path, 'pipenv')
         env = os.environ.copy()
         env.update({
             b'PIPENV_IGNORE_VIRTUALENVS': b'1',
             b'WORKON_HOME': str(os.path.normpath(os.path.join(self.topobjdir, '_virtualenvs'))),
         })
 
-        args = args or []
+        if python is not None:
+            env[b'PIPENV_DEFAULT_PYTHON_VERSION'] = python
+            env[b'PIPENV_PYTHON'] = python
+
+        def ensure_venv():
+            """Create virtual environment if needed and return path"""
+            venv = get_venv()
+            if venv is not None:
+                return venv
+            if python is not None:
+                subprocess.check_call(
+                    [pipenv, '--python', python],
+                    stderr=subprocess.STDOUT,
+                    env=env)
+            return get_venv()
+
+        def get_venv():
+            """Return path to virtual environment or None"""
+            try:
+                return subprocess.check_output(
+                    [pipenv, '--venv'],
+                    stderr=subprocess.STDOUT,
+                    env=env).rstrip()
+            except subprocess.CalledProcessError:
+                # virtual environment does not exist
+                return None
 
         if pipfile is not None:
             # Install from Pipfile
             env[b'PIPENV_PIPFILE'] = str(pipfile)
-            args.append('install')
+            subprocess.check_call([pipenv, 'install'], stderr=subprocess.STDOUT, env=env)
 
-        subprocess.check_call(
-            [pipenv] + args,
-            stderr=subprocess.STDOUT,
-            env=env)
-
-        self.virtualenv_root = subprocess.check_output(
-            [pipenv, '--venv'],
-            stderr=subprocess.STDOUT,
-            env=env).rstrip()
+        self.virtualenv_root = ensure_venv()
 
         if populate:
             # Populate from the manifest
             subprocess.check_call([
                 pipenv, 'run', 'python', os.path.join(here, 'virtualenv.py'), 'populate',
                 self.topsrcdir, self.topobjdir, self.virtualenv_root, self.manifest_path],
                 stderr=subprocess.STDOUT, env=env)