Skip to content

Commit

Permalink
Bug 1656611 - Remove objdir support from virtualenv_packages.txt
Browse files Browse the repository at this point in the history
…handling r=mhentges,froydnj

I noticed that the `objdir:build` entry in `build/virtualenv_packages.txt` entry was apparently unused. This originates from bug 841713, seven years ago, when the `objdir` handling was introduced. Today, this doesn't appear to be serving a purpose. There is no Python library in my `$objdir/build` directory; nor can I see anything in `build/moz.build` or any related files suggesting one could ever appear. I can only assume this feature has outlived its usefulness, so delete it and the relevant in-tree support.

This necessitates slightly changing the signature and implementation of the `activate_pipenv()` method; also update all callers.

Differential Revision: https://phabricator.services.mozilla.com/D85635
  • Loading branch information
Ricky Stewart committed Aug 7, 2020
1 parent 57ed679 commit 3ddb065
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 44 deletions.
2 changes: 1 addition & 1 deletion build/moz.configure/init.configure
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def virtualenv_python3(env_python, build_env, mozconfig, help):
virtualenvs_root = os.path.join(topobjdir, '_virtualenvs')
with LineIO(lambda l: log.info(l), 'replace') as out:
manager = VirtualenvManager(
topsrcdir, topobjdir,
topsrcdir,
os.path.join(virtualenvs_root, 'init_py3'), out,
os.path.join(topsrcdir, 'build', 'virtualenv_packages.txt'))

Expand Down
1 change: 0 additions & 1 deletion build/virtualenv_packages.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ mozilla.pth:third_party/python/json-e
mozilla.pth:third_party/python/yamllint
mozilla.pth:third_party/python/zipp
mozilla.pth:build
objdir:build
mozilla.pth:config
mozilla.pth:config/mozunit
mozilla.pth:dom/bindings
Expand Down
4 changes: 2 additions & 2 deletions python/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ def _activate_test_virtualenvs(self, python):
python = python or default_manager.python_path
py3_root = default_manager.virtualenv_root + '_py3'

self.activate_pipenv(pipfile=None, populate=True, python=python)
self.activate_pipenv(os.path.dirname(default_manager.virtualenv_root),
pipfile=None, populate=True, python=python)

# The current process might be running under Python 2 and the Python 3
# virtualenv will not be set up by mach bootstrap. To avoid problems in tests
Expand All @@ -263,7 +264,6 @@ def _activate_test_virtualenvs(self, python):

py3_manager = VirtualenvManager(
default_manager.topsrcdir,
default_manager.topobjdir,
py3_root,
default_manager.log_handle,
default_manager.manifest_path,
Expand Down
7 changes: 4 additions & 3 deletions python/mozbuild/mozbuild/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ def virtualenv_manager(self):
name += "_py3"
self._virtualenv_manager = VirtualenvManager(
self.topsrcdir,
self.topobjdir,
os.path.join(self.topobjdir, '_virtualenvs', name),
sys.stdout,
os.path.join(self.topsrcdir, 'build', 'virtualenv_packages.txt')
Expand Down Expand Up @@ -853,11 +852,13 @@ def ensure_pipenv(self):
self.virtualenv_manager.install_pip_package(path, vendored=True)
return pipenv

def activate_pipenv(self, pipfile=None, populate=False, python=None):
def activate_pipenv(self, workon_home, 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, populate, python)
self.virtualenv_manager.activate_pipenv(workon_home, pipfile, populate,
python)

def _ensure_zstd(self):
try:
Expand Down
1 change: 0 additions & 1 deletion python/mozbuild/mozbuild/sphinx.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ def setup(app):
# properly. We leverage the in-tree virtualenv for this.
topsrcdir = manager.topsrcdir
ve = VirtualenvManager(topsrcdir,
os.path.join(topsrcdir, 'dummy-objdir'),
os.path.join(app.outdir, '_venv'),
sys.stderr,
os.path.join(topsrcdir, 'build', 'virtualenv_packages.txt'))
Expand Down
47 changes: 13 additions & 34 deletions python/mozbuild/mozbuild/virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@
class VirtualenvManager(object):
"""Contains logic for managing virtualenvs for building the tree."""

def __init__(self, topsrcdir, topobjdir, virtualenv_path, log_handle,
manifest_path):
def __init__(self, topsrcdir, virtualenv_path, log_handle, manifest_path):
"""Create a new manager.
Each manager is associated with a source directory, a path where you
Expand All @@ -48,7 +47,6 @@ def __init__(self, topsrcdir, topobjdir, virtualenv_path, log_handle,
assert os.path.isabs(
manifest_path), "manifest_path must be an absolute path: %s" % (manifest_path)
self.topsrcdir = topsrcdir
self.topobjdir = topobjdir
self.virtualenv_root = virtualenv_path

# Record the Python executable that was used to create the Virtualenv
Expand Down Expand Up @@ -160,7 +158,6 @@ def up_to_date(self, python):
for submanifest in submanifests:
submanifest = os.path.join(self.topsrcdir, submanifest)
submanager = VirtualenvManager(self.topsrcdir,
self.topobjdir,
self.virtualenv_root,
self.log_handle,
submanifest)
Expand Down Expand Up @@ -290,10 +287,6 @@ def populate(self):
will be read and processed as if its contents were concatenated
into the manifest being read.
objdir -- Denotes a relative path in the object directory to add to the
search path. e.g. "objdir:build" will add $topobjdir/build to the
search path.
windows -- This denotes that the action should only be taken when run
on Windows.
Expand Down Expand Up @@ -350,7 +343,6 @@ def handle_package(package):
src = os.path.join(self.topsrcdir, package[1])
assert os.path.isfile(src), "'%s' does not exist" % src
submanager = VirtualenvManager(self.topsrcdir,
self.topobjdir,
self.virtualenv_root,
self.log_handle,
src)
Expand All @@ -368,12 +360,7 @@ def handle_package(package):
# relative path allows the srcdir/objdir combination
# to be moved around (as long as the paths relative to
# each other remain the same).
try:
f.write("%s\n" % os.path.relpath(path, python_lib))
except ValueError:
# When objdir is on a separate drive, relpath throws
f.write("%s\n" % os.path.join(python_lib, path))

f.write("%s\n" % os.path.relpath(path, python_lib))
return True

if package[0] == 'optional':
Expand All @@ -399,15 +386,6 @@ def handle_package(package):
handle_package(package[1:])
return True

if package[0] == 'objdir':
assert len(package) == 2
path = os.path.join(self.topobjdir, package[1])

with open(os.path.join(python_lib, 'objdir.pth'), 'a') as f:
f.write('%s\n' % path)

return True

raise Exception('Unknown action: %s' % package[0])

# We always target the OS X deployment target that Python itself was
Expand Down Expand Up @@ -510,7 +488,7 @@ def build(self, python):
# See https://bugzilla.mozilla.org/show_bug.cgi?id=1635481
os.environ.pop('__PYVENV_LAUNCHER__', None)
args = [self.python_path, thismodule, 'populate', self.topsrcdir,
self.topobjdir, self.virtualenv_root, self.manifest_path]
self.virtualenv_root, self.manifest_path]

result = self._log_process_output(args, cwd=self.topsrcdir)

Expand Down Expand Up @@ -620,7 +598,8 @@ def _run_pip(self, args):
subprocess.check_call([pip] + args, stderr=subprocess.STDOUT, cwd=self.topsrcdir,
universal_newlines=PY3)

def activate_pipenv(self, pipfile=None, populate=False, python=None):
def activate_pipenv(self, workon_home, 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
Expand All @@ -633,7 +612,7 @@ def activate_pipenv(self, pipfile=None, populate=False, python=None):
env = ensure_subprocess_env(os.environ.copy())
env.update(ensure_subprocess_env({
'PIPENV_IGNORE_VIRTUALENVS': '1',
'WORKON_HOME': str(os.path.normpath(os.path.join(self.topobjdir, '_virtualenvs')))
'WORKON_HOME': str(os.path.normpath(workon_home)),
}))
# On mac, running pipenv with LC_CTYPE set to "UTF-8" (which happens
# when wrapping with run-task on automation) fails.
Expand Down Expand Up @@ -690,7 +669,7 @@ def get_venv():
# 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],
self.topsrcdir, self.virtualenv_root, self.manifest_path],
stderr=subprocess.STDOUT, env=env)

self.activate()
Expand Down Expand Up @@ -773,23 +752,23 @@ def ensure_text(s):


if __name__ == '__main__':
if len(sys.argv) < 5:
if len(sys.argv) < 4:
print(
'Usage: populate_virtualenv.py /path/to/topsrcdir '
'/path/to/topobjdir /path/to/virtualenv /path/to/virtualenv_manifest')
'Usage: virtualenv.py /path/to/topsrcdir '
'/path/to/virtualenv /path/to/virtualenv_manifest')
sys.exit(1)

verify_python_version(sys.stdout)

topsrcdir, topobjdir, virtualenv_path, manifest_path = sys.argv[1:5]
topsrcdir, virtualenv_path, manifest_path = sys.argv[1:4]
populate = False

# This should only be called internally.
if sys.argv[1] == 'populate':
populate = True
topsrcdir, topobjdir, virtualenv_path, manifest_path = sys.argv[2:]
topsrcdir, virtualenv_path, manifest_path = sys.argv[2:]

manager = VirtualenvManager(topsrcdir, topobjdir, virtualenv_path,
manager = VirtualenvManager(topsrcdir, virtualenv_path,
sys.stdout, manifest_path)

if populate:
Expand Down
4 changes: 3 additions & 1 deletion python/safety/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ def python_safety(self, python=None, **kwargs):
self.logger = commandline.setup_logging(
"python-safety", {"raw": sys.stdout})

self.activate_pipenv(pipfile=os.path.join(here, 'Pipfile'), python=python, populate=True)
self.activate_pipenv(
os.path.dirname(self.virtualenv_manager.virtualenv_root),
pipfile=os.path.join(here, 'Pipfile'), python=python, populate=True)

pattern = '**/*requirements*.txt'
path = mozpath.normsep(os.path.dirname(os.path.dirname(here)))
Expand Down
4 changes: 3 additions & 1 deletion tools/moztreedocs/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ def build_docs(self, path=None, fmt='html', outdir=None, auto_open=True,
if self.check_jsdoc():
return die(JSDOC_NOT_FOUND)

self.activate_pipenv(os.path.join(here, 'Pipfile'))
self.activate_pipenv(
os.path.dirname(self.virtualenv_manager.virtualenv_root),
pipfile=os.path.join(here, 'Pipfile'))

import webbrowser
from livereload import Server
Expand Down

0 comments on commit 3ddb065

Please sign in to comment.