Skip to content

Commit

Permalink
start making the app-data more explicit and robust
Browse files Browse the repository at this point in the history
Signed-off-by: Bernat Gabor <[email protected]>
  • Loading branch information
gaborbernat committed Feb 25, 2020
1 parent a9277cc commit e8eee79
Show file tree
Hide file tree
Showing 36 changed files with 404 additions and 309 deletions.
9 changes: 9 additions & 0 deletions docs/changelog/1640.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Handle the case when the application data folder is read-only:

- the application data folder is now controllable via :option:`app-data`,
- :option:`clear-app-data` now cleans the entire application data folder, not just the ``app-data`` seeder path,
- check if the application data path passed in does not exist or is read-only, and fallback to a temporary directory,
- temporary directory application data is automatically cleaned up at the end of execution,
- :option:`symlink-app-data` is always ``False`` when the application data is temporary

by :user:`gaborbernat`.
3 changes: 2 additions & 1 deletion src/virtualenv/config/cli/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def __init__(self, options=None, *args, **kwargs):
self._verbosity = None
self._options = options
self._interpreter = None
self._app_data = None

def _fix_defaults(self):
for action in self._actions:
Expand Down Expand Up @@ -56,7 +57,7 @@ def parse_args(self, args=None, namespace=None):

class HelpFormatter(ArgumentDefaultsHelpFormatter):
def __init__(self, prog):
super(HelpFormatter, self).__init__(prog, max_help_position=35, width=240)
super(HelpFormatter, self).__init__(prog, max_help_position=32, width=240)

def _get_help_string(self, action):
# noinspection PyProtectedMember
Expand Down
10 changes: 8 additions & 2 deletions src/virtualenv/config/ini.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import logging
import os

from virtualenv.dirs import default_config_dir
from appdirs import user_config_dir

from virtualenv.info import PY3
from virtualenv.util import ConfigParser
from virtualenv.util.path import Path
Expand All @@ -21,7 +22,12 @@ class IniConfig(object):
def __init__(self):
config_file = os.environ.get(self.VIRTUALENV_CONFIG_FILE_ENV_VAR, None)
self.is_env_var = config_file is not None
self.config_file = Path(config_file) if config_file is not None else (default_config_dir() / "virtualenv.ini")
config_file = (
Path(config_file)
if config_file is not None
else Path(user_config_dir(appname="virtualenv", appauthor="pypa")) / "virtualenv.ini"
)
self.config_file = config_file
self._cache = {}

exception = None
Expand Down
9 changes: 5 additions & 4 deletions src/virtualenv/create/creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def __init__(self, options, interpreter):
self.dest = Path(options.dest)
self.clear = options.clear
self.pyenv_cfg = PyEnvCfg.from_folder(self.dest)
self.app_data = options.app_data.folder

def __repr__(self):
return ensure_str(self.__unicode__())
Expand All @@ -65,7 +66,7 @@ def can_create(cls, interpreter):
return True

@classmethod
def add_parser_arguments(cls, parser, interpreter, meta):
def add_parser_arguments(cls, parser, interpreter, meta, app_data):
"""Add CLI arguments for the creator.
:param parser: the CLI parser
Expand Down Expand Up @@ -172,19 +173,19 @@ def debug(self):
:return: debug information about the virtual environment (only valid after :meth:`create` has run)
"""
if self._debug is None and self.exe is not None:
self._debug = get_env_debug_info(self.exe, self.debug_script())
self._debug = get_env_debug_info(self.exe, self.debug_script(), self.app_data)
return self._debug

# noinspection PyMethodMayBeStatic
def debug_script(self):
return DEBUG_SCRIPT


def get_env_debug_info(env_exe, debug_script):
def get_env_debug_info(env_exe, debug_script, app_data):
env = os.environ.copy()
env.pop(str("PYTHONPATH"), None)

with ensure_file_on_disk(debug_script) as debug_script:
with ensure_file_on_disk(debug_script, app_data) as debug_script:
cmd = [str(env_exe), str(debug_script)]
if WIN_CPYTHON_2:
cmd = [ensure_text(i) for i in cmd]
Expand Down
6 changes: 3 additions & 3 deletions src/virtualenv/create/via_global_ref/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def __init__(self, options, interpreter):
self.enable_system_site_package = options.system_site

@classmethod
def add_parser_arguments(cls, parser, interpreter, meta):
super(ViaGlobalRefApi, cls).add_parser_arguments(parser, interpreter, meta)
def add_parser_arguments(cls, parser, interpreter, meta, app_data):
super(ViaGlobalRefApi, cls).add_parser_arguments(parser, interpreter, meta, app_data)
parser.add_argument(
"--system-site-packages",
default=False,
Expand Down Expand Up @@ -54,7 +54,7 @@ def create(self):
def patch_distutils_via_pth(self):
"""Patch the distutils package to not be derailed by its configuration files"""
patch_file = Path(__file__).parent / "_distutils_patch_virtualenv.py"
with ensure_file_on_disk(patch_file) as resolved_path:
with ensure_file_on_disk(patch_file, self.app_data) as resolved_path:
text = resolved_path.read_text()
text = text.replace('"__SCRIPT_DIR__"', repr(os.path.relpath(str(self.script_dir), str(self.purelib))))
patch_path = self.purelib / "_distutils_patch_virtualenv.py"
Expand Down
3 changes: 0 additions & 3 deletions src/virtualenv/create/via_global_ref/venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ def create(self):
self.create_inline()
else:
self.create_via_sub_process()

# TODO: cleanup activation scripts

for lib in self.libs:
ensure_dir(lib)
super(Venv, self).create()
Expand Down
56 changes: 0 additions & 56 deletions src/virtualenv/dirs.py

This file was deleted.

17 changes: 9 additions & 8 deletions src/virtualenv/discovery/builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class Builtin(Discover):
def __init__(self, options):
super(Builtin, self).__init__(options)
self.python_spec = options.python
self.app_data = options.app_data

@classmethod
def add_parser_arguments(cls, parser):
Expand All @@ -29,7 +30,7 @@ def add_parser_arguments(cls, parser):
)

def run(self):
return get_interpreter(self.python_spec)
return get_interpreter(self.python_spec, self.app_data.folder)

def __repr__(self):
return ensure_str(self.__unicode__())
Expand All @@ -38,11 +39,11 @@ def __unicode__(self):
return "{} discover of python_spec={!r}".format(self.__class__.__name__, self.python_spec)


def get_interpreter(key):
def get_interpreter(key, app_data=None):
spec = PythonSpec.from_string_spec(key)
logging.info("find interpreter for spec %r", spec)
proposed_paths = set()
for interpreter, impl_must_match in propose_interpreters(spec):
for interpreter, impl_must_match in propose_interpreters(spec, app_data):
key = interpreter.system_executable, impl_must_match
if key in proposed_paths:
continue
Expand All @@ -53,19 +54,19 @@ def get_interpreter(key):
proposed_paths.add(key)


def propose_interpreters(spec):
def propose_interpreters(spec, app_data):
# 1. if it's an absolute path and exists, use that
if spec.is_abs and os.path.exists(spec.path):
yield PythonInfo.from_exe(spec.path), True
yield PythonInfo.from_exe(spec.path, app_data), True

# 2. try with the current
yield PythonInfo.current_system(), True
yield PythonInfo.current_system(app_data), True

# 3. otherwise fallback to platform default logic
if IS_WIN:
from .windows import propose_interpreters

for interpreter in propose_interpreters(spec):
for interpreter in propose_interpreters(spec, app_data):
yield interpreter, True

paths = get_paths()
Expand All @@ -80,7 +81,7 @@ def propose_interpreters(spec):
exe = os.path.abspath(found)
if exe not in tested_exes:
tested_exes.add(exe)
interpreter = PathPythonInfo.from_exe(exe, raise_on_error=False)
interpreter = PathPythonInfo.from_exe(exe, app_data, raise_on_error=False)
if interpreter is not None:
yield interpreter, match

Expand Down
64 changes: 30 additions & 34 deletions src/virtualenv/discovery/cached_py_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from collections import OrderedDict
from hashlib import sha256

from virtualenv.dirs import default_data_dir
from virtualenv.discovery.py_info import PythonInfo
from virtualenv.info import PY2, PY3
from virtualenv.util.path import Path
Expand All @@ -25,12 +24,12 @@

_CACHE = OrderedDict()
_CACHE[Path(sys.executable)] = PythonInfo()
_FS_PATH = None


def from_exe(cls, exe, raise_on_error=True, ignore_cache=False):
def from_exe(cls, app_data, exe, raise_on_error=True, ignore_cache=False):
""""""
result = _get_from_cache(cls, exe, ignore_cache=ignore_cache)
py_info_cache = _get_py_info_cache(app_data)
result = _get_from_cache(cls, py_info_cache, app_data, exe, ignore_cache=ignore_cache)
if isinstance(result, Exception):
if raise_on_error:
raise result
Expand All @@ -40,30 +39,35 @@ def from_exe(cls, exe, raise_on_error=True, ignore_cache=False):
return result


def _get_from_cache(cls, exe, ignore_cache=True):
def _get_py_info_cache(app_data):
return None if app_data is None else app_data / "py_info" / __version__


def _get_from_cache(cls, py_info_cache, app_data, exe, ignore_cache=True):
# note here we cannot resolve symlinks, as the symlink may trigger different prefix information if there's a
# pyenv.cfg somewhere alongside on python3.4+
exe_path = Path(exe)
if not ignore_cache and exe_path in _CACHE: # check in the in-memory cache
result = _CACHE[exe_path]
elif py_info_cache is None: # cache disabled
failure, py_info = _run_subprocess(cls, exe, app_data)
result = py_info if failure is None else failure
else: # then check the persisted cache
py_info = _get_via_file_cache(cls, exe_path, exe)
py_info = _get_via_file_cache(cls, py_info_cache, app_data, exe_path, exe)
result = _CACHE[exe_path] = py_info
# independent if it was from the file or in-memory cache fix the original executable location
if isinstance(result, PythonInfo):
result.executable = exe
return result


def _get_via_file_cache(cls, resolved_path, exe):
def _get_via_file_cache(cls, py_info_cache, app_data, resolved_path, exe):
key = sha256(str(resolved_path).encode("utf-8") if PY3 else str(resolved_path)).hexdigest()
py_info = None
resolved_path_text = ensure_text(str(resolved_path))
resolved_path_modified_timestamp = resolved_path.stat().st_mtime
fs_path = _get_fs_path()
data_file = fs_path / "{}.json".format(key)

with fs_path.lock_for_key(key):
data_file = py_info_cache / "{}.json".format(key)
with py_info_cache.lock_for_key(key):
data_file_path = data_file.path
if data_file_path.exists(): # if exists and matches load
try:
Expand All @@ -72,12 +76,12 @@ def _get_via_file_cache(cls, resolved_path, exe):
logging.debug("get PythonInfo from %s for %s", data_file_path, exe)
py_info = cls._from_dict({k: v for k, v in data["content"].items()})
else:
raise ValueError("force cleanup as stale")
raise ValueError("force close as stale")
except (KeyError, ValueError, OSError):
logging.debug("remove PythonInfo %s for %s", data_file_path, exe)
data_file_path.unlink() # cleanup out of date files
data_file_path.unlink() # close out of date files
if py_info is None: # if not loaded run and save
failure, py_info = _run_subprocess(cls, exe)
failure, py_info = _run_subprocess(cls, exe, app_data)
if failure is None:
file_cache_content = {
"st_mtime": resolved_path_modified_timestamp,
Expand All @@ -91,22 +95,13 @@ def _get_via_file_cache(cls, resolved_path, exe):
return py_info


def _get_fs_path():
global _FS_PATH
if _FS_PATH is None:
_FS_PATH = default_data_dir() / "py-info" / __version__
return _FS_PATH


def _run_subprocess(cls, exe):
def _run_subprocess(cls, exe, app_data):
resolved_path = Path(os.path.abspath(__file__)).parent / "py_info.py"
with ensure_file_on_disk(resolved_path) as resolved_path:

cmd = [exe, "-s", str(resolved_path)]
with ensure_file_on_disk(resolved_path, app_data) as resolved_path:
cmd = [exe, str(resolved_path)]
# prevent sys.prefix from leaking into the child process - see https://bugs.python.org/issue22490
env = os.environ.copy()
env.pop("__PYVENV_LAUNCHER__", None)

logging.debug("get interpreter info via cmd: %s", LogCmd(cmd))
try:
process = Popen(
Expand Down Expand Up @@ -156,14 +151,15 @@ def __unicode__(self):
return raw


def clear():
fs_path = _get_fs_path()
with fs_path:
for filename in fs_path.path.iterdir():
if filename.suffix == ".json":
with fs_path.lock_for_key(filename.stem):
if filename.exists():
filename.unlink()
def clear(app_data):
py_info_cache = _get_py_info_cache(app_data)
if py_info_cache is not None:
with py_info_cache:
for filename in py_info_cache.path.iterdir():
if filename.suffix == ".json":
with py_info_cache.lock_for_key(filename.stem):
if filename.exists():
filename.unlink()
_CACHE.clear()


Expand Down
Loading

0 comments on commit e8eee79

Please sign in to comment.