handle application data folder is read only (#1661)

* fixed FileNotFoundError when directory isn't writable (#1640)

 - when using docker, if `user_data_dir()` isn't writable directory,
   `default_data_dir()` use `system temp directory` + `virtualenv`.
   for example, tempdir is `/tmp`, it use `/tmp/virtualenv`

* start making the app-data more explicit and robust

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>

* fix Windows

* fix docs

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>

Co-authored-by: Bernát Gábor <gaborjbernat@gmail.com>

docs/_static/custom.css

@@ -14,8 +14,8 @@
 }
 
 .wy-table-responsive table {
-    width: calc(100% + 3em);
-    margin-left: 20px !important;
+    width: 100%;
+    margin-left: 0 !important;
 }
 
 .rst-content table.docutils td ol {

docs/changelog/1640.bugfix.rst

@@ -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`.

docs/cli_interface.rst

@@ -33,16 +33,16 @@ Configuration file
 ^^^^^^^^^^^^^^^^^^
 
 virtualenv looks for a standard ini configuration file. The exact location depends on the operating system you're using,
-as determined by :pypi:`appdirs` application data definition. The configuration file location is printed as at the end of
-the output when ``--help`` is passed.
+as determined by :pypi:`appdirs` application configuration definition. The configuration file location is printed as at
+the end of the output when ``--help`` is passed.
 
 The keys of the settings are derived from the long command line option. For example, :option:`--python <python>`
 would be specified as:
 
 .. code-block:: ini
 
   [virtualenv]
-  python = /opt/python-3.3/bin/python
+  python = /opt/python-3.8/bin/python
 
 Options that take multiple values, like :option:`extra-search-dir` can be specified as:
 

docs/render_cli.py

@@ -166,7 +166,7 @@ def _get_targeted_names(self, row):
     @staticmethod
     def _get_help_text(row):
         name = row.names[0]
-        if name in ("--creator", "--clear-app-data"):
+        if name in ("--creator",):
             content = row.help[: row.help.index("(") - 1]
         else:
             content = row.help
@@ -196,6 +196,8 @@ def _get_default(row):
         name = row.names[0]
         if name == "-p":
             default_body = n.Text("the python executable virtualenv is installed into")
+        elif name == "--app-data":
+            default_body = n.Text("platform specific application data folder")
         elif name == "--activators":
             default_body = n.Text("comma separated list of activators supported")
         elif name == "--creator":

src/virtualenv/config/cli/parser.py

@@ -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:
@@ -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

src/virtualenv/config/ini.py

@@ -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
@@ -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

src/virtualenv/create/creator.py

@@ -3,20 +3,18 @@
 import json
 import logging
 import os
-import shutil
 import sys
 from abc import ABCMeta, abstractmethod
 from argparse import ArgumentTypeError
 from ast import literal_eval
 from collections import OrderedDict
-from stat import S_IWUSR
 
 from six import add_metaclass
 
 from virtualenv.discovery.cached_py_info import LogCmd
 from virtualenv.info import WIN_CPYTHON_2
 from virtualenv.pyenv_cfg import PyEnvCfg
-from virtualenv.util.path import Path
+from virtualenv.util.path import Path, safe_delete
 from virtualenv.util.six import ensure_str, ensure_text
 from virtualenv.util.subprocess import run_cmd
 from virtualenv.util.zipapp import ensure_file_on_disk
@@ -41,6 +39,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__())
@@ -65,7 +64,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
@@ -147,15 +146,7 @@ def non_write_able(dest, value):
     def run(self):
         if self.dest.exists() and self.clear:
             logging.debug("delete %s", self.dest)
-
-            def onerror(func, path, exc_info):
-                if not os.access(path, os.W_OK):
-                    os.chmod(path, S_IWUSR)
-                    func(path)
-                else:
-                    raise
-
-            shutil.rmtree(str(self.dest), ignore_errors=True, onerror=onerror)
+            safe_delete(self.dest)
         self.create()
         self.set_pyenv_cfg()
 
@@ -172,19 +163,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]

src/virtualenv/create/via_global_ref/api.py

@@ -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,
@@ -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"

src/virtualenv/create/via_global_ref/venv.py

@@ -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()

src/virtualenv/discovery/builtin.py

@@ -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):
@@ -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__())
@@ -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
@@ -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()
@@ -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