Commit c58d0e6b authored by Mitchell Hentges's avatar Mitchell Hentges
Browse files

Bug 1730712: Command virtualenvs should include Mach's import scope. r=ahal 

Mach's import scope includes:
* Its `pth` entries
* Its pip packages, which is either:
    * The Mach virtualenv site-packages, if packages were "pip
      installed" over the internet.
    * The system environment's site-packages, if installing packages
      over the internet is disabled and Mach is grabbing packages from
      the system Python instead.

Command virtualenvs _already_ had this import scope when they were
dynamically activated from an existing Mach process. However, when
used directly (e.g. by `<venv>/bin/python <script>`), they would be
missing this import scope, which was a confusing inconsistency.

However, resolving this inconsistency adds a new risk: when Mach is
using the system Python, the system packages now populate the same
context as command virtualenv packages - and they hadn't been checked
for compatibility. So, this patch also
includes behaviour to verify system<=>command-venv compatibility at
activation-time.

A few notes about this system-package-verification:
* It happens at virtualenv activation-time instead of build-time because
  system packages may change after the virtualenv is built.
* It takes roughly 1.5s, which is significant, but it should mainly occur
  in CI, where it's acceptable. Devs using `MACH_USE_SYSTEM_PACKAGES`
  should unset the variable to avoid the time delay.
* The algorithm works by asserting top-level requirements (e.g.
  `psutil>=5.4.2,<=5.8.0`), then runs `pip check` over the combined set
  of packages that would be in-scope.
* Note that, in this patch, system packages are *not* asserted to be
  the same version as their vendored counterparts. This is because, until
  we parse `third_party/python/requirements.in`, we don't know which
  packages we depend on directly (and whose APIs we care about if they
  change). Since leaning on system packages is essentially only used in
  CI (which we have strong control on), this downside seemed acceptable.

Differential Revision: https://phabricator.services.mozilla.com/D126288
parent cea54423

configure.py

+26 −3
Original line number Diff line number Diff line
@@ -22,7 +22,8 @@ sys.path.insert(0, os.path.join(base_dir, "python", "mozbuild")) 
sys.path.insert(0, os.path.join(base_dir, "third_party", "python", "packaging"))

sys.path.insert(0, os.path.join(base_dir, "third_party", "python", "pyparsing"))

sys.path.insert(0, os.path.join(base_dir, "third_party", "python", "six"))

from mach.site import CommandSiteManager

from mach.site import CommandSiteManager, MachSiteManager, MozSiteMetadata

from mozboot.util import get_state_dir

from mozbuild.configure import (

    ConfigureSandbox,

    TRACE,
@@ -226,16 +227,38 @@ def config_status(config, execute=True): 




def _activate_build_virtualenv():

    """Ensure that the build virtualenv is activated



    configure.py may be executed through Mach, or via "./configure, make".

    In the first case, the build virtualenv should already be activated.

    In the second case, we're likely being executed with the system Python, and must

    prepare the virtualenv and activate it ourselves.

    """



    version = ".".join(str(i) for i in sys.version_info[0:3])

    print(f"Using Python {version} from {sys.executable}")



    active_site = MozSiteMetadata.from_runtime()

    if active_site and active_site.site_name == "build":

        # We're already running within the "build" virtualenv, no additional work is

        # needed.

        return



    # We're using the system python (or are nested within a non-build mach-managed

    # virtualenv), so we should activate the build virtualenv as expected by the rest of

    # configure.



    topobjdir = os.path.realpath(".")

    topsrcdir = os.path.realpath(os.path.dirname(__file__))

    state_dir = get_state_dir()



    build_site = CommandSiteManager(

    mach_site = MachSiteManager.from_environment(topsrcdir, state_dir)

    mach_site.activate()

    build_site = CommandSiteManager.from_environment(

        topsrcdir,

        os.path.join(topobjdir, "_virtualenvs"),

        state_dir,

        "build",

        os.path.join(topobjdir, "_virtualenvs"),

    )

    if not build_site.ensure():

        print("Created Python 3 virtualenv")

python/mach/mach/site.py

+467 −105

File changed.

Preview size limit exceeded, changes collapsed.

python/mach/mach/test/test_decorators.py

+23 −4
Original line number Diff line number Diff line
@@ -5,10 +5,13 @@ 
from __future__ import absolute_import, unicode_literals



import os

from unittest import mock



import pytest

from unittest.mock import Mock



from mach.requirements import MachEnvRequirements

from mach.site import CommandSiteManager, SitePackagesSource, MozSiteMetadata

from mozunit import main



import mach.registrar
@@ -91,6 +94,22 @@ def test_register_command_sets_up_class_at_runtime(registrar): 
        )

        inner_function("bar")



    def from_environment_patch(topsrcdir, state_dir, virtualenv_name, dir):

        return CommandSiteManager(

            "",

            "",

            virtualenv_name,

            virtualenv_name,

            MozSiteMetadata(

                0, "mach", SitePackagesSource.VENV, SitePackagesSource.VENV, "", ""

            ),

            SitePackagesSource.VENV,

            MachEnvRequirements(),

        )



    with mock.patch.object(

        CommandSiteManager, "from_environment", from_environment_patch

    ):

        registrar.dispatch("cmd_foo", context)

        inner_function.assert_called_with("foo")

        registrar.dispatch("cmd_bar", context)

python/mozbuild/mozbuild/base.py

+4 −2
Original line number Diff line number Diff line
@@ -290,12 +290,14 @@ class MozbuildObject(ProcessExecutionMixin): 
    @property

    def virtualenv_manager(self):

        from mach.site import CommandSiteManager

        from mozboot.util import get_state_dir



        if self._virtualenv_manager is None:

            self._virtualenv_manager = CommandSiteManager(

            self._virtualenv_manager = CommandSiteManager.from_environment(

                self.topsrcdir,

                os.path.join(self.topobjdir, "_virtualenvs"),

                get_state_dir(),

                self._virtualenv_name,

                os.path.join(self.topobjdir, "_virtualenvs"),

            )



        return self._virtualenv_manager

python/mozbuild/mozbuild/controller/building.py

+10 −9
Original line number Diff line number Diff line
@@ -23,13 +23,14 @@ from collections import ( 
)

from textwrap import TextWrapper



from mach.site import CommandSiteManager



try:

    import psutil

except Exception:

    psutil = None



from mach.mixin.logging import LoggingMixin

from mozboot.util import get_mach_virtualenv_binary

import mozfile

from mozsystemmonitor.resourcemonitor import SystemResourceMonitor

from mozterm.widgets import Footer
@@ -1508,15 +1509,15 @@ class BuildDriver(MozbuildObject): 
                if eq == "=":

                    append_env[k] = v



        if six.PY3:

            python = sys.executable

        else:

            # Try to get the mach virtualenv Python if we can.

            python = get_mach_virtualenv_binary()

            if not os.path.exists(python):

                python = "python3"

        build_site = CommandSiteManager.from_environment(

            self.topsrcdir,

            self.statedir,

            "build",

            os.path.join(self.topobjdir, "_virtualenvs"),

        )

        build_site.ensure()



        command = [python, os.path.join(self.topsrcdir, "configure.py")]

        command = [build_site.python_path, os.path.join(self.topsrcdir, "configure.py")]

        if options:

            command.extend(options)