Skip to content

Commit

Permalink
Bug 1739177: Add requirements.pths_as_absolute() r=ahal
Browse files Browse the repository at this point in the history
There were a bunch of locations where we were doing path shenanigans
with `requirements.pth/.vendored` items.

There was a bit of complexity because we were specifically making each
`pthfile` line be a relative path to support moving the Firefox
topsrcdir without causing issues.

However, now that we're more intelligent about checking if `pthfile`
lines are up-to-date (and since moving your topsrcdir will still require
re-generating the Mach virtualenv), this behaviour became less useful.

So, generalize `MachEnvRequirements` -> "sys.path lines" logic and
reuse it everywhere.

Differential Revision: https://phabricator.services.mozilla.com/D129693
  • Loading branch information
Mitchell Hentges committed Nov 4, 2021
1 parent 0511cbc commit 67be954
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 40 deletions.
12 changes: 2 additions & 10 deletions build/mach_initialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,7 @@ def _activate_python_environment(topsrcdir, state_dir):
# (optional) dependencies are not installed.
_scrub_system_site_packages()

sys.path[0:0] = [
os.path.join(topsrcdir, pth.path)
for pth in requirements.pth_requirements
+ requirements.vendored_requirements
]
sys.path[0:0] = requirements.pths_as_absolute(topsrcdir)
elif is_mach_virtualenv:
# We're running in the Mach virtualenv - check that it's up-to-date.
# Note that the "pip package check" exists to ensure that a virtualenv isn't
Expand All @@ -284,11 +280,7 @@ def _activate_python_environment(topsrcdir, state_dir):
# Remove global site packages from sys.path to improve isolation accordingly.
_scrub_system_site_packages()

sys.path[0:0] = [
os.path.join(topsrcdir, pth.path)
for pth in requirements.pth_requirements
+ requirements.vendored_requirements
]
sys.path[0:0] = requirements.pths_as_absolute(topsrcdir)


def initialize(topsrcdir):
Expand Down
8 changes: 8 additions & 0 deletions python/mach/mach/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ def __init__(self):
self.pypi_optional_requirements = []
self.vendored_requirements = []

def pths_as_absolute(self, topsrcdir):
return sorted(
[
os.path.normcase(os.path.join(topsrcdir, pth.path))
for pth in (self.pth_requirements + self.vendored_requirements)
]
)

def validate_environment_packages(self, pip_command):
result = EnvironmentPackageValidationResult()
if not self.pypi_requirements and not self.pypi_optional_requirements:
Expand Down
31 changes: 4 additions & 27 deletions python/mach/mach/virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,26 +204,11 @@ def up_to_date(self, skip_pip_package_check=False):
with open(
os.path.join(self._site_packages_dir(), PTH_FILENAME)
) as file:
pth_lines = file.read().strip().split("\n")
current_paths = file.read().strip().split("\n")
except FileNotFoundError:
return False

current_paths = [
os.path.normcase(
os.path.abspath(os.path.join(self._site_packages_dir(), path))
)
for path in pth_lines
]

required_paths = [
os.path.normcase(
os.path.abspath(os.path.join(self.topsrcdir, pth.path))
)
for pth in env_requirements.pth_requirements
+ env_requirements.vendored_requirements
]

if current_paths != required_paths:
if current_paths != env_requirements.pths_as_absolute(self.topsrcdir):
return False

if not skip_pip_package_check:
Expand Down Expand Up @@ -310,17 +295,9 @@ def build(self):
self.create()
env_requirements = self.requirements()
site_packages_dir = self._site_packages_dir()
pthfile_lines = self.requirements().pths_as_absolute(self.topsrcdir)
with open(os.path.join(site_packages_dir, PTH_FILENAME), "a") as f:
for pth_requirement in (
env_requirements.pth_requirements
+ env_requirements.vendored_requirements
):
path = os.path.join(self.topsrcdir, pth_requirement.path)
# This path is relative to the .pth file. Using a
# relative path allows the srcdir/objdir combination
# to be moved around (as long as the paths relative to
# each other remain the same).
f.write("{}\n".format(os.path.relpath(path, site_packages_dir)))
f.write("\n".join(pthfile_lines))

old_env_variables = {}
try:
Expand Down
4 changes: 1 addition & 3 deletions python/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ def python(
)

append_env["PYTHONPATH"] = os.pathsep.join(
os.path.join(command_context.topsrcdir, pth.path)
for pth in requirements.pth_requirements
+ requirements.vendored_requirements
requirements.pths_as_absolute(command_context.topsrcdir)
)
else:
command_context.virtualenv_manager.ensure()
Expand Down

0 comments on commit 67be954

Please sign in to comment.