-
-
Notifications
You must be signed in to change notification settings - Fork 637
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
Upgrade to pex 1.4.5. #6267
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}") | ||
|
@@ -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==||") | ||
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} \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}")" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How are these used here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
@classmethod | ||
def register_options(cls, register): | ||
super(Conan, cls).register_options(register) | ||
|
@@ -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 |
---|---|---|
|
@@ -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:]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
|
@@ -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: | ||
|
@@ -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) | ||
|
There was a problem hiding this comment.
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:
which should net the same effect w/o the manual parsing.
There was a problem hiding this comment.
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.