Skip to content

Commit

Permalink
Eric's feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
jsirois committed Nov 14, 2019
1 parent caaf93d commit 5421323
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 44 deletions.
23 changes: 12 additions & 11 deletions pex/commands/bdist_pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pex.bin.pex import configure_clp
from pex.common import die
from pex.compatibility import ConfigParser, StringIO, string, to_unicode
from pex.executor import Executor
from pex.interpreter import PythonInterpreter


# Suppress checkstyle violations due to distutils command requirements.
Expand Down Expand Up @@ -96,26 +96,27 @@ def run(self):
target = os.path.join(self.bdist_dir, name + '-' + version + '.pex')
pex_specs.append((name if name in console_scripts else None, target))

# In order for code to run to here, pex is on the sys.path - make sure to propagate the
# sys.path so the subprocess can find us.
env = os.environ.copy()
env['PYTHONPATH'] = os.pathsep.join(sys.path)

args = [sys.executable, '-s', '-m', 'pex.bin.pex', package_dir] + reqs + self.pex_args
args = ['-m', 'pex', package_dir] + reqs + self.pex_args
if self.get_log_level() < log.INFO and options.verbosity == 0:
args.append('-v')

for script_name, target in pex_specs:
cmd = args + ['--output-file', target]
pex_cmd = args + ['--output-file', target]
if script_name:
log.info('Writing %s to %s' % (script_name, target))
cmd += ['--script', script_name]
pex_cmd += ['--script', script_name]
else:
# The package has no namesake entry point, so build an environment pex.
log.info('Writing environment pex into %s' % target)

log.debug('Building pex via: {}'.format(' '.join(cmd)))
process = Executor.open_process(cmd, stderr=subprocess.PIPE, env=env)
cmd, process = PythonInterpreter.get().open_process(
args=pex_cmd,
stderr=subprocess.PIPE,

# In order for code to run to here, pex is on the sys.path - make sure to propagate the
# sys.path so the subprocess can find us.
pythonpath=sys.path
)
_, stderr = process.communicate()
result = process.returncode
if result != 0:
Expand Down
2 changes: 1 addition & 1 deletion pex/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def compile(self, root, relpaths):
fp.flush()

try:
out, _ = Executor.execute([self._interpreter.binary, '-sE', fp.name])
_, out, _ = self._interpreter.execute(args=[fp.name])
except Executor.NonZeroExit as e:
raise self.CompilationFailure(
'encountered %r during bytecode compilation.\nstderr was:\n%s\n' % (e, e.stderr)
Expand Down
55 changes: 49 additions & 6 deletions pex/interpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,42 @@ def all(cls, paths=None):
def _from_binary_internal(cls):
return cls(sys.executable, PythonIdentity.get())

@classmethod
def _create_isolated_cmd(cls, binary, args=None, pythonpath=None, env=None):
cmd = [binary]

# Don't add the user site directory to `sys.path`.
#
# Additionally, it would be nice to pass `-S` to disable adding site-packages but unfortunately
# some python distributions include portions of the standard library there.
cmd.append('-s')

env = cls.sanitized_environment(env=env)
if pythonpath:
env['PYTHONPATH'] = os.pathsep.join(pythonpath)
else:
# Turn off reading of PYTHON* environment variables.
cmd.append('-E')

if args:
cmd.extend(args)

rendered_command = ' '.join(cmd)
if pythonpath:
rendered_command = 'PYTHONPATH={} {}'.format(pythonpath, rendered_command)
TRACER.log('Executing: {}'.format(rendered_command))

return cmd, env

@classmethod
def _execute(cls, binary, args=None, pythonpath=None, env=None, stdin_payload=None, **kwargs):
cmd, env = cls._create_isolated_cmd(binary, args=args, pythonpath=pythonpath, env=env)
stdout, stderr = Executor.execute(cmd, stdin_payload=stdin_payload, env=env, **kwargs)
return cmd, stdout, stderr

@classmethod
def _from_binary_external(cls, binary):
environ = cls.sanitized_environment()
stdout, _ = Executor.execute([binary, '-sE'],
env=environ,
stdin_payload=_generate_identity_source())
_, stdout, _ = cls._execute(binary, stdin_payload=_generate_identity_source())
identity = stdout.strip()
if not identity:
raise cls.IdentificationError('Could not establish identity of %s' % binary)
Expand Down Expand Up @@ -400,10 +430,10 @@ def version_filter(version):
yield interp

@classmethod
def sanitized_environment(cls):
def sanitized_environment(cls, env=None):
# N.B. This is merely a hack because sysconfig.py on the default OS X
# installation of 2.7 breaks.
env_copy = os.environ.copy()
env_copy = (env or os.environ).copy()
env_copy.pop('MACOSX_DEPLOYMENT_TARGET', None)
return env_copy

Expand Down Expand Up @@ -438,6 +468,19 @@ def version(self):
def version_string(self):
return str(self._identity)

def execute(self, args=None, stdin_payload=None, pythonpath=None, env=None, **kwargs):
return self._execute(self.binary,
args=args,
stdin_payload=stdin_payload,
pythonpath=pythonpath,
env=env,
**kwargs)

def open_process(self, args=None, pythonpath=None, env=None, **kwargs):
cmd, env = self._create_isolated_cmd(self.binary, args=args, pythonpath=pythonpath, env=env)
process = Executor.open_process(cmd, env=env, **kwargs)
return cmd, process

def __hash__(self):
return hash((self._binary, self._identity))

Expand Down
28 changes: 11 additions & 17 deletions pex/pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@
from __future__ import absolute_import, print_function

import os
import sys
from collections import deque

from pex import third_party
from pex.executor import Executor
from pex.interpreter import PythonInterpreter
from pex.platforms import Platform
from pex.tracer import TRACER
from pex.variables import ENV


Expand All @@ -21,14 +19,11 @@ class PipError(Exception):

def execute_pip_isolated(args, cache=None, interpreter=None):
env = os.environ.copy()
env.update({
'PYTHONPATH': os.pathsep.join(third_party.expose(['pip', 'setuptools', 'wheel'])),
'__PEX_UNVENDORED__': '1'
})
python = interpreter.binary if interpreter is not None else sys.executable
python_exe_args = [python, '-s']
env['__PEX_UNVENDORED__'] = '1'

pip_args = python_exe_args + ['-m', 'pip', '--disable-pip-version-check', '--isolated']
pythonpath = third_party.expose(['pip', 'setuptools', 'wheel'])

pip_args = ['-m', 'pip', '--disable-pip-version-check', '--isolated']

# The max pip verbosity is -vvv and for pex it's -vvvvvvvvv; so we scale down by a factor of 3.
verbosity = ENV.PEX_VERBOSE // 3
Expand All @@ -43,17 +38,16 @@ def execute_pip_isolated(args, cache=None, interpreter=None):
pip_args.append('--no-cache-dir')

pip_cmd = pip_args + args
TRACER.log('Executing: {}'.format(' '.join(pip_cmd)))
process = Executor.open_process(pip_cmd, env=env)
if 0 != process.wait():
raise PipError('Executing {} failed with {}'.format(' '.join(pip_cmd), process.returncode))

interpreter = interpreter or PythonInterpreter.get()
cmd, process = interpreter.open_process(args=pip_cmd, pythonpath=pythonpath, env=env)
if process.wait() != 0:
raise PipError('Executing {} failed with {}'.format(' '.join(cmd), process.returncode))


def _calculate_package_index_options(indexes=None, find_links=None):
# N.B.: We interpret None to mean accept pip index defaults, [] to mean turn off all index use.
if indexes is None:
pass
else:
if indexes is not None:
if len(indexes) == 0:
yield '--no-index'
else:
Expand Down
37 changes: 29 additions & 8 deletions pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
import os
import shutil
import subprocess
import sys
from collections import defaultdict, namedtuple
from textwrap import dedent

from pex import third_party
from pex.common import safe_mkdir, safe_mkdtemp
from pex.executor import Executor
from pex.interpreter import PythonInterpreter
from pex.orderedset import OrderedSet
from pex.pip import PipError, build_wheels, download_distributions, install_wheel
from pex.requirements import local_project_from_requirement, local_projects_from_requirement_file
Expand Down Expand Up @@ -57,13 +56,15 @@ def _calculate_dependency_markers(distributions, interpreter=None):
""".format(search_path=search_path))

env = os.environ.copy()
env.update({
'PYTHONPATH': os.pathsep.join(third_party.expose(['setuptools'])),
'__PEX_UNVENDORED__': '1'
})
python = interpreter.binary if interpreter is not None else sys.executable
env['__PEX_UNVENDORED__'] = '1'

process = Executor.open_process([python, '-s', '-c', program], stdout=subprocess.PIPE, env=env)
pythonpath = third_party.expose(['setuptools'])

interpreter = interpreter or PythonInterpreter.get()
_, process = interpreter.open_process(args=['-c', program],
stdout=subprocess.PIPE,
pythonpath=pythonpath,
env=env)
stdout, _ = process.communicate()
if process.returncode != 0:
raise Untranslateable('Could not determine dependency environment markers for {}'
Expand Down Expand Up @@ -130,6 +131,26 @@ def resolve(requirements=None,
a particular requirement.
"""

# This function has three stages: 1) resolve, 2) build, and 3) chroot.
#
# You'd think we might be able to just pip install all the requirements, but pexes can be
# multi-platform / multi-interpreter, in which case only a subset of distributions resolved into
# the PEX should be activated for the runtime interpreter. Sometimes there are platform specific
# wheels and sometimes python version specific dists (backports being the common case). As such,
# we need to be able to add each resolved distribution to the `sys.path` individually
# (`PEXEnvironment` handles this selective activation at runtime). Since pip install only accepts
# a single location to install all resolved dists, that won't work.
#
# This means we need to seperately resolve all distributions, then install each in their own
# chroot. To do this we use `pip download` for the resolve and download of all needed
# distributions and then `pip install` to install each distribution in its own chroot.
#
# As a complicating factor, the runtime activation scheme relies on PEP 425 tags; i.e.: wheel
# names. Some requirements are only available or applicable in source form - either via sdist, VCS
# URL or local projects. As such we need to insert a `pip wheel` step to generate wheels for all
# requirements resolved in source form via `pip download` / inspection of requirements to
# discover those that are local directories (local setup.py or pyproject.toml python projects).

if not requirements and not requirement_files:
# Nothing to resolve.
return []
Expand Down
9 changes: 8 additions & 1 deletion pex/vendor/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ def _root():
class VendorSpec(collections.namedtuple('VendorSpec', ['key', 'version', 'rewrite'])):
"""Represents a vendored distribution.
:field str key: The distribution requirement key; e.g.: for a requirement of
requests[security]==2.22.0 the key is 'requests'.
:field str version: The distribution requirement version; e.g.: for a requirement of
requests[security]==2.22.0 the version is '2.22.0'.

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Nov 14, 2019

Contributor

What is [security] in this example, if it isn’t key? Is there any way to indicate that? If not, certainly let’s reserve that for a followup PR so that we can land this.

:field bool rewrite: Whether to re-write the distribution's imports for use with the
`pex.third_party` importer.
NB: Vendored distributions should comply with the host distribution platform constraints. In the
case of pex, which is a py2.py3 platform agnostic wheel, vendored libraries should be as well.
"""
Expand Down Expand Up @@ -67,7 +74,7 @@ def create_packages(self):
:class:`pex.third_party.VendorImporter`.
"""
if not self.rewrite:
# The extra package structure is only required for vendored code used via import rewrties.
# The extra package structure is only required for vendored code used via import rewrites.
return

for index, _ in enumerate(self._subpath_components):
Expand Down

0 comments on commit 5421323

Please sign in to comment.