Skip to content

Commit 9c77153

Browse files
author
Mitchell Hentges
committed
Bug 1723031: Allow flexible dependency-specification in the Mach venv r=ahal
There's some trade-offs in play here: the major issue is that we can't pin `psutil`'s because it's pre-installed on some CI workers with a different version (5.4.2). Additionally, we can't "just" bump that version, because CI workers jump between different revisions to do work, so a specific pinned version won't work when we try to update such a package. One option is that we could avoid validating package versions in CI, but then that will cause surprises (heck, I didn't know we were still using `psutil==5.4.2` instead of `5.8.0` until now). By doing validation, we make it more explicit and avoid accidentally depending on behaviour of too new of such a package. However, in most cases, we manage the installation environment and can pin dependencies. So, I've made the top-level Mach virtualenv the _only_ one that is able to use requirement specifiers other than "==". Differential Revision: https://phabricator.services.mozilla.com/D122889
1 parent d76d2cf commit 9c77153

File tree

7 files changed

+82
-42
lines changed

7 files changed

+82
-42
lines changed

build/mach_initialize.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,16 @@ class MetaPathFinder(object):
169169

170170
def _activate_python_environment(topsrcdir):
171171
# We need the "mach" module to access the logic to parse virtualenv
172-
# requirements.
173-
sys.path.insert(0, os.path.join(topsrcdir, "python", "mach"))
172+
# requirements. Since that depends on "packaging" (and, transitively,
173+
# "pyparsing"), we add those to the path too.
174+
sys.path[0:0] = [
175+
os.path.join(topsrcdir, module)
176+
for module in (
177+
os.path.join("python", "mach"),
178+
os.path.join("third_party", "python", "packaging"),
179+
os.path.join("third_party", "python", "pyparsing"),
180+
)
181+
]
174182

175183
from mach.requirements import MachEnvRequirements
176184

@@ -182,6 +190,7 @@ def _activate_python_environment(topsrcdir):
182190
requirements = MachEnvRequirements.from_requirements_definition(
183191
topsrcdir,
184192
is_thunderbird,
193+
True,
185194
os.path.join(topsrcdir, "build", "mach_virtualenv_packages.txt"),
186195
)
187196
sys.path[0:0] = [

build/mach_virtualenv_packages.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,7 @@ packages.txt:build/common_virtualenv_packages.txt
33
# and it has to be built from source.
44
pypi-optional:glean-sdk==40.0.0:telemetry will not be collected
55
# Mach gracefully handles the case where `psutil` is unavailable.
6-
pypi-optional:psutil==5.8.0:telemetry will be missing some data
6+
# We aren't (yet) able to pin packages in automation, so we have to
7+
# support down to the oldest locally-installed version (5.4.2).
8+
pypi-optional:psutil>=5.4.2,<=5.8.0:telemetry will be missing some data
79
pypi:zstandard==0.15.2

configure.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
sys.path.insert(0, os.path.join(base_dir, "python", "mach"))
2525
sys.path.insert(0, os.path.join(base_dir, "python", "mozboot"))
2626
sys.path.insert(0, os.path.join(base_dir, "python", "mozbuild"))
27+
sys.path.insert(0, os.path.join(base_dir, "third_party", "python", "packaging"))
28+
sys.path.insert(0, os.path.join(base_dir, "third_party", "python", "pyparsing"))
2729
sys.path.insert(0, os.path.join(base_dir, "third_party", "python", "six"))
2830
from mozbuild.configure import (
2931
ConfigureSandbox,

python/mach/mach/requirements.py

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import os
66
from pathlib import Path
77

8+
from packaging.requirements import Requirement
9+
810

911
THUNDERBIRD_PYPI_ERROR = """
1012
Thunderbird requirements definitions cannot include PyPI packages.
@@ -17,18 +19,14 @@ def __init__(self, path):
1719

1820

1921
class PypiSpecifier:
20-
def __init__(self, package_name, version, full_specifier):
21-
self.package_name = package_name
22-
self.version = version
23-
self.full_specifier = full_specifier
22+
def __init__(self, requirement):
23+
self.requirement = requirement
2424

2525

26-
class PypiOptionalSpecifier:
27-
def __init__(self, repercussion, package_name, version, full_specifier):
26+
class PypiOptionalSpecifier(PypiSpecifier):
27+
def __init__(self, repercussion, requirement):
28+
super().__init__(requirement)
2829
self.repercussion = repercussion
29-
self.package_name = package_name
30-
self.version = version
31-
self.full_specifier = full_specifier
3230

3331

3432
class MachEnvRequirements:
@@ -65,17 +63,29 @@ def __init__(self):
6563

6664
@classmethod
6765
def from_requirements_definition(
68-
cls, topsrcdir, is_thunderbird, requirements_definition
66+
cls,
67+
topsrcdir,
68+
is_thunderbird,
69+
is_mach_or_build_virtualenv,
70+
requirements_definition,
6971
):
7072
requirements = cls()
7173
_parse_mach_env_requirements(
72-
requirements, requirements_definition, topsrcdir, is_thunderbird
74+
requirements,
75+
requirements_definition,
76+
topsrcdir,
77+
is_thunderbird,
78+
is_mach_or_build_virtualenv,
7379
)
7480
return requirements
7581

7682

7783
def _parse_mach_env_requirements(
78-
requirements_output, root_requirements_path, topsrcdir, is_thunderbird
84+
requirements_output,
85+
root_requirements_path,
86+
topsrcdir,
87+
is_thunderbird,
88+
is_mach_or_build_virtualenv,
7989
):
8090
topsrcdir = Path(topsrcdir)
8191

@@ -119,9 +129,10 @@ def _parse_requirements_line(
119129
if is_thunderbird_packages_txt:
120130
raise Exception(THUNDERBIRD_PYPI_ERROR)
121131

122-
package_name, version = _parse_package_specifier(params)
123132
requirements_output.pypi_requirements.append(
124-
PypiSpecifier(package_name, version, params)
133+
PypiSpecifier(
134+
_parse_package_specifier(params, is_mach_or_build_virtualenv)
135+
)
125136
)
126137
elif action == "pypi-optional":
127138
if is_thunderbird_packages_txt:
@@ -133,10 +144,14 @@ def _parse_requirements_line(
133144
'description in the format "package:fallback explanation", '
134145
'found "{}"'.format(params)
135146
)
136-
package, repercussion = params.split(":")
137-
package_name, version = _parse_package_specifier(package)
147+
raw_requirement, repercussion = params.split(":")
138148
requirements_output.pypi_optional_requirements.append(
139-
PypiOptionalSpecifier(repercussion, package_name, version, package)
149+
PypiOptionalSpecifier(
150+
repercussion,
151+
_parse_package_specifier(
152+
raw_requirement, is_mach_or_build_virtualenv
153+
),
154+
)
140155
)
141156
elif action == "thunderbird-packages.txt":
142157
if is_thunderbird:
@@ -164,10 +179,14 @@ def _parse_requirements_definition_file(
164179
_parse_requirements_definition_file(root_requirements_path, False)
165180

166181

167-
def _parse_package_specifier(specifier):
168-
if len(specifier.split("==")) != 2:
182+
def _parse_package_specifier(raw_requirement, is_mach_or_build_virtualenv):
183+
requirement = Requirement(raw_requirement)
184+
185+
if not is_mach_or_build_virtualenv and [
186+
s for s in requirement.specifier if s.operator != "=="
187+
]:
169188
raise Exception(
170-
"Expected pypi package version to be pinned in the "
171-
'format "package==version", found "{}"'.format(specifier)
189+
'All virtualenvs except for "mach" and "build" must pin pypi package '
190+
f'versions in the format "package==version", found "{raw_requirement}"'
172191
)
173-
return specifier.split("==")
192+
return requirement

python/mach/mach/test/test_virtualenv_compatibility.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,18 @@ def _resolve_command_virtualenv_names():
2626
return virtualenv_names
2727

2828

29-
def _requirement_definition_to_pip_format(virtualenv_name, cache, is_mach_env):
29+
def _requirement_definition_to_pip_format(virtualenv_name, cache, is_mach_or_build_env):
3030
"""Convert from parsed requirements object to pip-consumable format"""
3131
path = Path(topsrcdir) / "build" / f"{virtualenv_name}_virtualenv_packages.txt"
3232
requirements = MachEnvRequirements.from_requirements_definition(
33-
topsrcdir, False, path
33+
topsrcdir, False, is_mach_or_build_env, path
3434
)
3535

3636
lines = []
3737
for pypi in (
3838
requirements.pypi_requirements + requirements.pypi_optional_requirements
3939
):
40-
lines.append(pypi.full_specifier)
40+
lines.append(str(pypi.requirement))
4141

4242
for vendored in requirements.vendored_requirements:
4343
lines.append(cache.package_for_vendor_dir(Path(vendored.path)))
@@ -111,7 +111,9 @@ def test_virtualenvs_compatible(tmpdir):
111111

112112
for name in command_virtualenv_names:
113113
print(f'Checking compatibility of "{name}" virtualenv')
114-
command_requirements = _requirement_definition_to_pip_format(name, cache, False)
114+
command_requirements = _requirement_definition_to_pip_format(
115+
name, cache, name == "build"
116+
)
115117
with open(work_dir / "requirements.txt", "w") as requirements_txt:
116118
requirements_txt.write(mach_requirements)
117119
requirements_txt.write("\n")

python/mach_commands.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ def python(
7373
requirements = MachEnvRequirements.from_requirements_definition(
7474
command_context.topsrcdir,
7575
False,
76+
True,
7677
os.path.join(
7778
command_context.topsrcdir, "build", "mach_virtualenv_packages.txt"
7879
),

python/mozbuild/mozbuild/virtualenv.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -233,18 +233,17 @@ def up_to_date(self):
233233
installed_packages = {
234234
package["name"]: package["version"] for package in installed_packages
235235
}
236-
for requirement in env_requirements.pypi_requirements:
237-
if (
238-
installed_packages.get(requirement.package_name, None)
239-
!= requirement.version
236+
for pkg in env_requirements.pypi_requirements:
237+
if not pkg.requirement.specifier.contains(
238+
installed_packages.get(pkg.requirement.name, None)
240239
):
241240
return False
242241

243-
for requirement in env_requirements.pypi_optional_requirements:
244-
installed_version = installed_packages.get(
245-
requirement.package_name, None
246-
)
247-
if installed_version and installed_version != requirement.version:
242+
for pkg in env_requirements.pypi_optional_requirements:
243+
installed_version = installed_packages.get(pkg.requirement.name, None)
244+
if installed_version and not pkg.requirement.specifier.contains(
245+
installed_packages.get(pkg.requirement.name, None)
246+
):
248247
return False
249248

250249
return True
@@ -326,7 +325,10 @@ def _requirements(self):
326325
os.listdir(thunderbird_dir)
327326
)
328327
return MachEnvRequirements.from_requirements_definition(
329-
self.topsrcdir, is_thunderbird, self._manifest_path
328+
self.topsrcdir,
329+
is_thunderbird,
330+
self._virtualenv_name in ("mach", "build"),
331+
self._manifest_path,
330332
)
331333

332334
def populate(self):
@@ -370,14 +372,14 @@ def populate(self):
370372
f.write("{}\n".format(os.path.relpath(path, python_lib)))
371373

372374
for pypi_requirement in env_requirements.pypi_requirements:
373-
self.install_pip_package(pypi_requirement.full_specifier)
375+
self.install_pip_package(str(pypi_requirement.requirement))
374376

375377
for requirement in env_requirements.pypi_optional_requirements:
376378
try:
377-
self.install_pip_package(requirement.full_specifier)
379+
self.install_pip_package(str(requirement.requirement))
378380
except subprocess.CalledProcessError:
379381
print(
380-
f"Could not install {requirement.package_name}, so "
382+
f"Could not install {requirement.requirement.name}, so "
381383
f"{requirement.repercussion}. Continuing."
382384
)
383385

@@ -603,6 +605,9 @@ def verify_python_version(log_handle):
603605

604606
# We want to be able to import the "mach.requirements" module.
605607
sys.path.append(os.path.join(opts.topsrcdir, "python", "mach"))
608+
# Virtualenv logic needs access to the vendored "packaging" library.
609+
sys.path.append(os.path.join(opts.topsrcdir, "third_party", "python", "pyparsing"))
610+
sys.path.append(os.path.join(opts.topsrcdir, "third_party", "python", "packaging"))
606611

607612
manager = VirtualenvManager(
608613
opts.topsrcdir,

0 commit comments

Comments
 (0)