Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to pex 1.4.5. #6267

Merged
merged 3 commits into from
Jul 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion 3rdparty/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ mock==2.0.0
packaging==16.8
parameterized==0.6.1
pathspec==0.5.0
pex==1.4.3
pex==1.4.5
psutil==4.3.0
pycodestyle==2.4.0
pyflakes==2.0.0
Expand Down
33 changes: 21 additions & 12 deletions build-support/bin/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,6 @@ function build_pex() {
build_3rdparty_packages "${PANTS_UNSTABLE_VERSION}"
fi

activate_tmp_venv && trap deactivate RETURN && pip install "pex==1.4.3" || die "Failed to install pex."

local requirements=()
for pkg_name in $PANTS_PEX_PACKAGES; do
requirements=("${requirements[@]}" "${pkg_name}==${PANTS_UNSTABLE_VERSION}")
Expand All @@ -596,16 +594,27 @@ function build_pex() {
platform_flags=("${platform_flags[@]}" "--platform=${platform}")
done

pex \
-o "${dest}" \
--entry-point="pants.bin.pants_loader:main" \
--no-build \
--no-pypi \
--disable-cache \
"${platform_flags[@]}" \
-f "${DEPLOY_PANTS_WHEEL_DIR}/${PANTS_UNSTABLE_VERSION}" \
-f "${DEPLOY_3RDPARTY_WHEEL_DIR}/${PANTS_UNSTABLE_VERSION}" \
"${requirements[@]}"
(
PEX_VERSION=$(grep "pex==" "${ROOT}/3rdparty/python/requirements.txt" | sed -e "s|pex==||")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one potential alternative to this would be:

pex --constraints=3rdparty/python/requirements.txt pex

which should net the same effect w/o the manual parsing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bootstrap issue here. This grabs the pants pex dep to form the url to fetch the corresponding released pex pex, so we don't have pex yet.

PEX_PEX=pex27

cd $(mktemp -d -t build_pex.XXXXX)
trap "rm -rf $(pwd -P)" EXIT

curl -sSL https://github.com/pantsbuild/pex/releases/download/v${PEX_VERSION}/${PEX_PEX} -O
chmod +x ./${PEX_PEX}

./${PEX_PEX} \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be nice to libify this bootstrap-then-run routine for general use in common.sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, although this is the only shell script to use pex. I'll re-consider on #6214 which I plucked this from since it's languishing.

-o "${dest}" \
-c pants \
--no-build \
--no-pypi \
--disable-cache \
"${platform_flags[@]}" \
-f "${DEPLOY_PANTS_WHEEL_DIR}/${PANTS_UNSTABLE_VERSION}" \
-f "${DEPLOY_3RDPARTY_WHEEL_DIR}/${PANTS_UNSTABLE_VERSION}" \
"${requirements[@]}"
)

if [[ "${PANTS_PEX_RELEASE}" == "stable" ]]; then
mkdir -p "$(dirname "${stable_dest}")"
Expand Down
17 changes: 9 additions & 8 deletions src/python/pants/backend/native/subsystems/conan.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
from pex.pex_info import PexInfo

from pants.backend.python.python_requirement import PythonRequirement
from pants.backend.python.subsystems.python_repos import PythonRepos
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.tasks.pex_build_util import dump_requirements
from pants.backend.python.tasks.wrapped_pex import WrappedPEX
from pants.base.build_environment import get_pants_cachedir
from pants.subsystem.subsystem import Subsystem
from pants.util.dirutil import safe_concurrent_creation
Expand Down Expand Up @@ -49,6 +50,10 @@ class Conan(Subsystem):
def implementation_version(cls):
return super(Conan, cls).implementation_version() + [('Conan', 0)]

@classmethod
def subsystem_dependencies(cls):
return super(Conan, cls).subsystem_dependencies() + (PythonRepos, PythonSetup)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are these used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pants.backend.python.tasks.pex_build_util.dump_requirements sneakily grabs the global instances without requesting them, so you were getting lucky here. See #6202 (comment) for some discussion of thi problem. Nora is working on a fix.


@classmethod
def register_options(cls, register):
super(Conan, cls).register_options(register)
Expand All @@ -57,22 +62,18 @@ def register_options(cls, register):

class ConanBinary(datatype(['pex'])):
"""A `conan` PEX binary."""
pass

def bootstrap_conan(self):
pex_info = PexInfo.default()
pex_info.entry_point = 'conans.conan'
conan_bootstrap_dir = os.path.join(get_pants_cachedir(), 'conan_support')
conan_pex_path = os.path.join(conan_bootstrap_dir, 'conan_binary')
interpreter = PythonInterpreter.get()
if os.path.exists(conan_pex_path):
conan_binary = WrappedPEX(PEX(conan_pex_path, interpreter))
return self.ConanBinary(pex=conan_binary)
else:
if not os.path.exists(conan_pex_path):
with safe_concurrent_creation(conan_pex_path) as safe_path:
builder = PEXBuilder(safe_path, interpreter, pex_info=pex_info)
reqs = [PythonRequirement(req) for req in self.get_options().conan_requirements]
dump_requirements(builder, interpreter, reqs, logger)
builder.freeze()
conan_binary = WrappedPEX(PEX(conan_pex_path, interpreter))
return self.ConanBinary(pex=conan_binary)
conan_binary = PEX(conan_pex_path, interpreter)
return self.ConanBinary(pex=conan_binary)
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ def ensure_conan_remote_configuration(self, conan_binary):
# and replace it with Pants-controlled remotes.
remove_conan_center_remote_cmdline = self._remove_conan_center_remote_cmdline(conan_binary)
try:
# Slice the command line because subprocess errors when the first element in the
# list of command strings is the setting of an environment variable.
stdout = subprocess.check_output(remove_conan_center_remote_cmdline.split()[1:])
Copy link
Contributor

@CMLivingston CMLivingston Jul 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixed only if pex path is not specified, correct? Based on the changes to wrapped_pex below, it seems this would still be needed if PEX_PATH were set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code no longer uses WrappedPEX.

stdout = subprocess.check_output(remove_conan_center_remote_cmdline)
self.context.log.debug(stdout)
except subprocess.CalledProcessError as e:
if not "'conan-center' not found in remotes" in e.output:
Expand All @@ -197,7 +195,7 @@ def ensure_conan_remote_configuration(self, conan_binary):
index_num,
remote_url)
try:
stdout = subprocess.check_output(add_pants_conan_remote_cmdline.split()[1:])
stdout = subprocess.check_output(add_pants_conan_remote_cmdline)
self.context.log.debug(stdout)
except subprocess.CalledProcessError as e:
if not "already exists in remotes" in e.output:
Expand Down Expand Up @@ -252,7 +250,7 @@ def _fetch_packages(self, vt, vts_results_dir):

# Invoke conan to pull package from remote.
try:
stdout = subprocess.check_output(cmdline.split()[1:])
stdout = subprocess.check_output(cmdline)
except subprocess.CalledProcessError as e:
raise self.NativeExternalLibraryFetchError(
"Error invoking conan for fetch task: {}\n".format(e.output)
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/interpreter_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pex.resolver import resolve
from pex.variables import Variables

from pants.backend.python.pex_util import create_bare_interpreter, expand_and_maybe_adjust_platform
from pants.backend.python.pex_util import expand_and_maybe_adjust_platform
from pants.backend.python.targets.python_target import PythonTarget
from pants.base.exceptions import TaskError
from pants.process.lock import OwnerPrintingInterProcessFileLock
Expand Down Expand Up @@ -105,7 +105,7 @@ def _interpreter_from_path(self, path, filters):
executable = os.readlink(os.path.join(path, 'python'))
except OSError:
return None
interpreter = create_bare_interpreter(executable)
interpreter = PythonInterpreter.from_binary(executable, include_site_extras=False)
if self._matches(interpreter, filters):
return self._resolve(interpreter)
return None
Expand Down
14 changes: 0 additions & 14 deletions src/python/pants/backend/python/pex_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,6 @@
logger = logging.getLogger(__name__)


def create_bare_interpreter(binary_path):
"""Creates an interpreter for python binary at the given path.

The interpreter is bare in that it has no extras associated with it.

:returns: A bare python interpreter with no extras.
:rtype: :class:`pex.interpreter.PythonInterpreter`
"""
# TODO(John Sirois): Replace with a more direct PythonInterpreter construction API call when
# https://github.com/pantsbuild/pex/issues/510 is fixed.
interpreter_with_extras = PythonInterpreter.from_binary(binary_path)
return PythonInterpreter(binary_path, interpreter_with_extras.identity, extras=None)


def _interpreter_str(interp):
ident = interp.identity
return ('PythonInterpreter({binary!r}, {identity!r} with extended info: '
Expand Down
3 changes: 1 addition & 2 deletions src/python/pants/backend/python/tasks/select_interpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from pex.interpreter import PythonInterpreter

from pants.backend.python.interpreter_cache import PythonInterpreterCache
from pants.backend.python.pex_util import create_bare_interpreter
from pants.backend.python.subsystems.python_repos import PythonRepos
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
Expand Down Expand Up @@ -102,7 +101,7 @@ def _get_interpreter(interpreter_path_file):
with open(interpreter_path_file, 'r') as infile:
lines = infile.readlines()
binary = lines[0].strip()
interpreter = create_bare_interpreter(binary)
interpreter = PythonInterpreter.from_binary(binary, include_site_extras=False)
for line in lines[1:]:
dist_name, dist_version, location = line.strip().split('\t')
interpreter = interpreter.with_extra(dist_name, dist_version, location)
Expand Down
22 changes: 5 additions & 17 deletions src/python/pants/backend/python/tasks/wrapped_pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ class WrappedPEX(object):
"""

_PEX_PATH_ENV_VAR_NAME = 'PEX_PATH'
_PEX_PYTHON_PATH_ENV_VAR_NAME = 'PEX_PYTHON_PATH'

def __init__(self, pex, extra_pex_paths=None):
"""
Expand All @@ -38,28 +37,17 @@ def path(self):

def cmdline(self, args=()):
cmdline = ' '.join(self._pex.cmdline(args))

def render_env_var(key, value):
return '{key}={value}'.format(key=key, value=value)

env_vars = [(self._PEX_PYTHON_PATH_ENV_VAR_NAME, self._pex._interpreter.binary)]

pex_path = self._pex_path()
if pex_path:
env_vars.append((self._PEX_PATH_ENV_VAR_NAME, pex_path))

return '{execution_control_env_vars} {cmdline}'.format(
execution_control_env_vars=' '.join(render_env_var(k, v) for k, v in env_vars),
cmdline=cmdline
)
return '{env_var_name}={pex_path} {cmdline}'.format(env_var_name=self._PEX_PATH_ENV_VAR_NAME,
pex_path=pex_path,
cmdline=cmdline)
else:
return cmdline

def run(self, *args, **kwargs):
env = copy(kwargs.pop('env', {}))

# Hack around bug in PEX where custom interpreters are not forwarded to PEXEnvironments.
# TODO(John Sirois): Remove when https://github.com/pantsbuild/pex/issues/522 is fixed.
env[self._PEX_PYTHON_PATH_ENV_VAR_NAME] = self._pex._interpreter.binary

pex_path = self._pex_path()
if pex_path:
env[self._PEX_PATH_ENV_VAR_NAME] = pex_path
Expand Down