Skip to content

Commit

Permalink
Sanitize PEX_ROOT handling.
Browse files Browse the repository at this point in the history
In the past, portions of the pex cache could be controlled individually
but this was no longer the case in practice nor was it desirable. Unify
all cache handling under the PEX_ROOT and deprecate `--cache-dir` to
codify the new single pex cache as the PEX_ROOT.

Also add best-effort support for always finding a a viable PEX_ROOT even
when the configured value is not writeable. In conjunction with
`--runtime-pex-root` this provides for both a Pex CLI and created PEXes
that should always work wrt having a viable PEX_ROOT but still display a
warning if a suboptimal alternate had to be used.

Fixes pex-tool#746
Fixes pex-tool#816
Fixes pex-tool#926
  • Loading branch information
jsirois committed Mar 24, 2020
1 parent 1c6ae63 commit f2440b7
Show file tree
Hide file tree
Showing 11 changed files with 256 additions and 60 deletions.
59 changes: 37 additions & 22 deletions pex/bin/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from optparse import OptionGroup, OptionParser, OptionValueError
from textwrap import TextWrapper

from pex import pex_warnings
from pex.common import die, safe_delete, safe_mkdtemp
from pex.interpreter import PythonInterpreter
from pex.interpreter_constraints import (
Expand Down Expand Up @@ -170,17 +171,18 @@ def configure_clp_pex_resolution(parser):

group.add_option(
'--disable-cache',
action='callback',
dest='cache_dir',
callback=process_disable_cache,
dest='disable_cache',
default=False,
action='store_true',
help='Disable caching in the pex tool entirely.')

group.add_option(
'--cache-dir',
dest='cache_dir',
default='{pex_root}',
help='The local cache directory to use for speeding up requirement '
'lookups. [Default: ~/.pex]')
default=None,
help='DEPRECATED: Use --pex-root instead. '
'The local cache directory to use for speeding up requirement '
'lookups. [Default: {}]'.format(ENV.PEX_ROOT))

group.add_option(
'--wheel', '--no-wheel', '--no-use-wheel',
Expand Down Expand Up @@ -502,8 +504,9 @@ def configure_clp():
parser.add_option(
'--pex-root',
dest='pex_root',
default=ENV.PEX_ROOT,
help='Specify the pex root used in this invocation of pex. [Default: %default]')
default=None,
help='Specify the pex root used in this invocation of pex. '
'[Default: {}]'.format(ENV.PEX_ROOT))

parser.add_option(
'--help-variables',
Expand All @@ -523,7 +526,7 @@ def _safe_link(src, dst):
os.symlink(src, dst)


def build_pex(reqs, options):
def build_pex(reqs, options, cache=None):
interpreters = None # Default to the current interpreter.

# NB: options.python and interpreter constraints cannot be used together.
Expand Down Expand Up @@ -607,7 +610,7 @@ def walk_and_do(fn, src_dir):
platforms=options.platforms,
indexes=indexes,
find_links=options.find_links,
cache=options.cache_dir,
cache=cache,
build=options.build,
use_wheel=options.use_wheel,
compile=options.compile,
Expand Down Expand Up @@ -637,11 +640,6 @@ def walk_and_do(fn, src_dir):
return pex_builder


def make_relative_to_root(path):
"""Update options so that defaults are user relative to specified pex_root."""
return os.path.normpath(path.format(pex_root=ENV.PEX_ROOT))


def transform_legacy_arg(arg):
# inherit-path used to be a boolean arg (so either was absent, or --inherit-path)
# Now it takes a string argument, so --inherit-path is invalid.
Expand Down Expand Up @@ -670,18 +668,35 @@ def main(args=None):
args, cmdline = args, []

options, reqs = parser.parse_args(args=args)
if options.python and options.interpreter_constraint:
die('The "--python" and "--interpreter-constraint" options cannot be used together.')

with ENV.patch(PEX_VERBOSE=str(options.verbosity),
PEX_ROOT=options.pex_root) as patched_env:
if options.cache_dir:
pex_warnings.warn('The --cache-dir option is deprecated, use --pex-root instead.')
if options.pex_root and options.cache_dir != options.pex_root:
die('Both --cache-dir and --pex-root were passed with conflicting values. '
'Just set --pex-root.')

if options.disable_cache:
def warn_ignore_pex_root(set_via):
pex_warnings.warn('The pex root has been set via {via} but --disable-cache is also set. '
'Ignoring {via} and disabling caches.'.format(via=set_via))

# Don't alter cache if it is disabled.
if options.cache_dir:
options.cache_dir = make_relative_to_root(options.cache_dir)
warn_ignore_pex_root('--cache-dir')
elif options.pex_root:
warn_ignore_pex_root('--pex-root')
elif os.environ.get('PEX_ROOT'):
warn_ignore_pex_root('PEX_ROOT')

pex_root = safe_mkdtemp()
else:
pex_root = options.cache_dir or options.pex_root or ENV.PEX_ROOT

if options.python and options.interpreter_constraint:
die('The "--python" and "--interpreter-constraint" options cannot be used together.')

with ENV.patch(PEX_VERBOSE=str(options.verbosity), PEX_ROOT=pex_root) as patched_env:
with TRACER.timed('Building pex'):
pex_builder = build_pex(reqs, options)
pex_builder = build_pex(reqs, options, cache=ENV.PEX_ROOT)

pex_builder.freeze(bytecode_compile=options.compile)
pex = PEX(pex_builder.path(),
Expand Down
21 changes: 21 additions & 0 deletions pex/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,27 @@ def chmod_plus_w(path):
os.chmod(path, path_mode)


def can_write_dir(path):
"""Determines if the directory at path can be written to by the current process.
If the directory doesn't exist, determines if it can be created and thus written to.
N.B.: This is a best-effort check only that uses permission heuristics and does not actually test
that the directory can be written to with and writes.
:param str path: The directory path to test.
:return: `True` if the given path is a directory that can be written to by the current process.
:rtype boo:
"""
while not os.access(path, os.F_OK):
parent_path = os.path.dirname(path)
if not parent_path or (parent_path == path):
# We've recursed up to the root without success, which shouldn't happen,
return False
path = parent_path
return os.path.isdir(path) and os.access(path, os.R_OK | os.W_OK | os.X_OK)


def touch(file, times=None):
"""Equivalent of unix `touch path`.
Expand Down
2 changes: 1 addition & 1 deletion pex/pex_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def maybe_reexec_pex(compatibility_constraints=None):
def _bootstrap(entry_point):
from .pex_info import PexInfo
pex_info = PexInfo.from_pex(entry_point)
pex_warnings.configure_warnings(pex_info)
pex_warnings.configure_warnings(pex_info, ENV)
return pex_info


Expand Down
11 changes: 9 additions & 2 deletions pex/pex_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os

from pex import pex_warnings
from pex.common import open_zip
from pex.common import can_write_dir, open_zip, safe_mkdtemp
from pex.compatibility import PY2
from pex.compatibility import string as compatibility_string
from pex.orderedset import OrderedSet
Expand Down Expand Up @@ -295,7 +295,14 @@ def always_write_cache(self, value):

@property
def pex_root(self):
return os.path.expanduser(self._pex_info.get('pex_root', os.path.join('~', '.pex')))
pex_root = os.path.expanduser(self._pex_info.get('pex_root', os.path.join('~', '.pex')))
if not can_write_dir(pex_root):
tmp_root = safe_mkdtemp()
pex_warnings.warn('PEX_ROOT is configured as {pex_root} but that path is un-writeable, '
'falling back to a temporary PEX_ROOT of {tmp_root} which will hurt '
'performance.'.format(pex_root=pex_root, tmp_root=tmp_root))
pex_root = self._pex_info['pex_root'] = tmp_root
return pex_root

@pex_root.setter
def pex_root(self, value):
Expand Down
5 changes: 1 addition & 4 deletions pex/pex_warnings.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,12 @@

import warnings

from pex.variables import ENV


class PEXWarning(Warning):
"""Indicates a warning from PEX about suspect buildtime or runtime configuration."""


def configure_warnings(pex_info, env=None):
env = env or ENV
def configure_warnings(pex_info, env):
if env.PEX_VERBOSE > 0:
emit_warnings = True
elif env.PEX_EMIT_WARNINGS is not None:
Expand Down
2 changes: 1 addition & 1 deletion pex/pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def _spawn_pip_isolated(self, args, cache=None, interpreter=None):
pip_args.append('--no-cache-dir')

command = pip_args + args
with ENV.strip().patch(PEX_ROOT=ENV.PEX_ROOT, PEX_VERBOSE=str(pex_verbosity)) as env:
with ENV.strip().patch(PEX_ROOT=cache or ENV.PEX_ROOT, PEX_VERBOSE=str(pex_verbosity)) as env:
# Guard against API calls from environment with ambient PYTHONPATH preventing pip PEX
# bootstrapping. See: https://github.com/pantsbuild/pex/issues/892
pythonpath = env.pop('PYTHONPATH', None)
Expand Down
18 changes: 16 additions & 2 deletions pex/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
import sys
from contextlib import contextmanager

from pex.common import die
from pex import pex_warnings
from pex.common import can_write_dir, die, safe_mkdtemp

__all__ = ('ENV', 'Variables')

Expand Down Expand Up @@ -269,7 +270,20 @@ def PEX_ROOT(self):
The directory location for PEX to cache any dependencies and code. PEX must write
not-zip-safe eggs and all wheels to disk in order to activate them. Default: ~/.pex
"""
return self._get_path('PEX_ROOT', default=os.path.expanduser('~/.pex'))
pex_root = self._get_path('PEX_ROOT', default=os.path.expanduser('~/.pex'))

if pex_root is None:
# PEX_ROOT is no set and we're running in stripped_defaults mode.
return None

if not can_write_dir(pex_root):
tmp_root = safe_mkdtemp()
pex_warnings.warn('PEX_ROOT is configured as {pex_root} but that path is un-writeable, '
'falling back to a temporary PEX_ROOT of {tmp_root} which will hurt '
'performance.'.format(pex_root=pex_root, tmp_root=tmp_root))
pex_root = self._environ['PEX_ROOT'] = tmp_root

return pex_root

@property
def PEX_PATH(self):
Expand Down
25 changes: 25 additions & 0 deletions tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
Chroot,
PermPreservingZipFile,
atomic_directory,
can_write_dir,
chmod_plus_x,
temporary_dir,
touch
Expand Down Expand Up @@ -178,3 +179,27 @@ def test_chroot_perms_link_cross_device():
mock_link.side_effect = OSError(expected_errno, os.strerror(expected_errno))

assert_chroot_perms(Chroot.link)


def test_can_write_dir_writeable_perms():
with temporary_dir() as writeable:
assert can_write_dir(writeable)

path = os.path.join(writeable, 'does/not/exist/yet')
assert can_write_dir(path)
touch(path)
assert not can_write_dir(path), 'Should not be able to write to a file.'


def test_can_write_dir_unwriteable_perms():
with temporary_dir() as writeable:
no_perms_path = os.path.join(writeable, 'no_perms')
os.mkdir(no_perms_path, 0o444)
assert not can_write_dir(no_perms_path)

path_that_does_not_exist_yet = os.path.join(no_perms_path, 'does/not/exist/yet')
assert not can_write_dir(path_that_does_not_exist_yet)

os.chmod(no_perms_path, 0o744)
assert can_write_dir(no_perms_path)
assert can_write_dir(path_that_does_not_exist_yet)
80 changes: 78 additions & 2 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

import pytest

from pex.common import safe_copy, safe_mkdir, safe_open, safe_sleep, temporary_dir
from pex.common import safe_copy, safe_mkdir, safe_open, safe_rmtree, safe_sleep, temporary_dir
from pex.compatibility import WINDOWS, nested, to_bytes
from pex.pex_info import PexInfo
from pex.pip import get_pip
Expand All @@ -39,7 +39,7 @@
temporary_content
)
from pex.third_party import pkg_resources
from pex.util import named_temporary_file
from pex.util import DistributionHelper, named_temporary_file


def make_env(**kwargs):
Expand Down Expand Up @@ -1727,3 +1727,79 @@ def test_pex_run_strip_env():
assert pex_env == json.loads(subprocess.check_output([unstripped_pex_file], env=env)), (
'Expected the entrypoint environment to be left un-stripped.'
)


def iter_distributions(pex_root, project_name=None):
found = set()
for root, dirs, _ in os.walk(pex_root):
for d in dirs:
if project_name and not d.startswith(project_name):
continue
if not d.endswith('.whl'):
continue
wheel_path = os.path.realpath(os.path.join(root, d))
if wheel_path in found:
continue
dist = DistributionHelper.distribution_from_path(wheel_path)
if dist.project_name == project_name:
found.add(wheel_path)
yield dist


def test_pex_cache_dir_and_pex_root():
python = ensure_python_interpreter(PY36)
with temporary_dir() as td:
cache_dir = os.path.join(td, 'cache_dir')
pex_root = os.path.join(td, 'pex_root')

# When the options both have the same value it should be accepted.
pex_file = os.path.join(td, 'pex_file')
run_pex_command(
python=python,
args=[
'--cache-dir', cache_dir,
'--pex-root', cache_dir,
'p537==1.0.3',
'-o', pex_file
]
).assert_success()

dists = list(iter_distributions(pex_root=cache_dir, project_name='p537'))
assert 1 == len(dists), (
'Expected to find exactly one distribution, found {}'.format(dists)
)

for directory in cache_dir, pex_root:
safe_rmtree(directory)

# When the options have conflicting values they should be rejected.
run_pex_command(
python=python,
args=[
'--cache-dir', cache_dir,
'--pex-root', pex_root,
'p537==1.0.3',
'-o', pex_file
]
).assert_failure()

assert not os.path.exists(cache_dir)
assert not os.path.exists(pex_root)


def test_disable_cache():
python = ensure_python_interpreter(PY36)
with temporary_dir() as td:
pex_root = os.path.join(td, 'pex_root')
pex_file = os.path.join(td, 'pex_file')
run_pex_command(
python=python,
args=[
'--disable-cache',
'p537==1.0.3',
'-o', pex_file
],
env=make_env(PEX_ROOT=pex_root)
).assert_success()

assert not os.path.exists(pex_root)
Loading

0 comments on commit f2440b7

Please sign in to comment.