utils/env: better support system site packages dir

This change improves handling of site-packages under system env, by
gracefully handling fallbacks to user site when required and possible.

Resolves: python-poetry#3079

poetry/installation/pip_installer.py

@@ -113,7 +113,9 @@ def remove(self, package):
             raise
 
         # This is a workaround for https://github.com/pypa/pip/issues/4176
-        nspkg_pth_file = self._env.site_packages / "{}-nspkg.pth".format(package.name)
+        nspkg_pth_file = self._env.site_packages.path / "{}-nspkg.pth".format(
+            package.name
+        )
         if nspkg_pth_file.exists():
             nspkg_pth_file.unlink()
 

poetry/masonry/builders/editable.py

@@ -94,7 +94,6 @@ def _setup_build(self):
                 os.remove(str(setup))
 
     def _add_pth(self):
-        pth_file = Path(self._module.name).with_suffix(".pth")
         paths = set()
         for include in self._module.includes:
             if isinstance(include, PackageInclude) and (
@@ -106,29 +105,25 @@ def _add_pth(self):
         for path in paths:
             content += decode(path + os.linesep)
 
-        for site_package in [self._env.site_packages, self._env.usersite]:
-            if not site_package:
-                continue
-
-            try:
-                site_package.mkdir(parents=True, exist_ok=True)
-                path = site_package.joinpath(pth_file)
-                self._debug(
-                    "  - Adding <c2>{}</c2> to <b>{}</b> for {}".format(
-                        path.name, site_package, self._poetry.file.parent
-                    )
+        pth_file = Path(self._module.name).with_suffix(".pth")
+        try:
+            pth_file = self._env.site_packages.write_text(
+                pth_file, content, encoding="utf-8"
+            )
+            self._debug(
+                "  - Adding <c2>{}</c2> to <b>{}</b> for {}".format(
+                    pth_file.name, pth_file.parent, self._poetry.file.parent
                 )
-                path.write_text(content, encoding="utf-8")
-                return [path]
-            except PermissionError:
-                self._debug("- <b>{}</b> is not writable trying next available site")
-
-        self._io.error_line(
-            "  - Failed to create <c2>{}</c2> for {}".format(
-                pth_file.name, self._poetry.file.parent
             )
-        )
-        return []
+            return [pth_file]
+        except OSError:
+            # TODO: Replace with PermissionError
+            self._io.error_line(
+                "  - Failed to create <c2>{}</c2> for {}".format(
+                    pth_file.name, self._poetry.file.parent
+                )
+            )
+            return []
 
     def _add_scripts(self):
         added = []
@@ -187,19 +182,27 @@ def _add_dist_info(self, added_files):
         added_files = added_files[:]
 
         builder = WheelBuilder(self._poetry)
-        dist_info = self._env.site_packages.joinpath(builder.dist_info)
+
+        dist_info_path = Path(builder.dist_info)
+        for dist_info in self._env.site_packages.find(
+            dist_info_path, writable_only=True
+        ):
+            if dist_info.exists():
+                self._debug(
+                    "  - Removing existing <c2>{}</c2> directory from <b>{}</b>".format(
+                        dist_info.name, dist_info.parent
+                    )
+                )
+                shutil.rmtree(str(dist_info))
+
+        dist_info = self._env.site_packages.mkdir(dist_info_path)
 
         self._debug(
             "  - Adding the <c2>{}</c2> directory to <b>{}</b>".format(
-                dist_info.name, self._env.site_packages
+                dist_info.name, dist_info.parent
             )
         )
 
-        if dist_info.exists():
-            shutil.rmtree(str(dist_info))
-
-        dist_info.mkdir()
-
         with dist_info.joinpath("METADATA").open("w", encoding="utf-8") as f:
             builder._write_metadata_file(f)
 

poetry/utils/env.py

@@ -7,6 +7,7 @@
 import shutil
 import sys
 import sysconfig
+import tempfile
 import textwrap
 
 from contextlib import contextmanager
@@ -39,6 +40,7 @@
 from poetry.utils._compat import encode
 from poetry.utils._compat import list_to_shell_command
 from poetry.utils._compat import subprocess
+from poetry.utils.helpers import paths_csv
 
 
 GET_ENVIRONMENT_INFO = """\
@@ -143,6 +145,125 @@ def _version_nodot(version):
 """
 
 
+class SitePackages:
+    def __init__(
+        self, path, fallbacks=None, skip_write_checks=False
+    ):  # type: (Path, List[Path], bool) -> None
+        self._path = path
+        self._fallbacks = fallbacks or []
+        self._skip_write_checks = skip_write_checks
+        self._candidates = [self._path] + self._fallbacks
+        self._writable_candidates = None if not skip_write_checks else self._candidates
+
+    @property
+    def path(self):  # type: () -> Path
+        return self._path
+
+    @property
+    def candidates(self):  # type: () -> List[Path]
+        return self._candidates
+
+    @property
+    def writable_candidates(self):  # type: () -> List[Path]
+        if self._writable_candidates is not None:
+            return self._writable_candidates
+
+        self._writable_candidates = []
+        for candidate in self._candidates:
+            try:
+                if not candidate.exists():
+                    continue
+
+                with tempfile.TemporaryFile(dir=str(candidate)):
+                    self._writable_candidates.append(candidate)
+            except (IOError, OSError):
+                pass
+
+        return self._writable_candidates
+
+    def make_candidates(
+        self, path, writable_only=False
+    ):  # type: (Path, bool) -> List[Path]
+        candidates = self._candidates if not writable_only else self.writable_candidates
+        if path.is_absolute():
+            for candidate in candidates:
+                try:
+                    path.relative_to(candidate)
+                    return [path]
+                except ValueError:
+                    pass
+            else:
+                raise ValueError(
+                    "{} is not relative to any discovered {}sites".format(
+                        path, "writable " if writable_only else ""
+                    )
+                )
+
+        return [candidate / path for candidate in candidates if candidate]
+
+    def _path_method_wrapper(
+        self, path, method, *args, **kwargs
+    ):  # type: (Path, str, *Any, **Any) -> Union[Tuple[Path, Any], List[Tuple[Path, Any]]]
+
+        # TODO: Move to parameters after dropping Python 2.7
+        return_first = kwargs.pop("return_first", True)
+        writable_only = kwargs.pop("writable_only", False)
+
+        candidates = self.make_candidates(path, writable_only=writable_only)
+
+        if not candidates:
+            raise RuntimeError(
+                'Unable to find a suitable destination for "{}" in {}'.format(
+                    str(path), paths_csv(self._candidates)
+                )
+            )
+
+        results = []
+
+        for candidate in candidates:
+            try:
+                result = candidate, getattr(candidate, method)(*args, **kwargs)
+                if return_first:
+                    return result
+                else:
+                    results.append(result)
+            except (IOError, OSError):
+                # TODO: Replace with PermissionError
+                pass
+
+        if results:
+            return results
+
+        raise OSError("Unable to access any of {}".format(paths_csv(candidates)))
+
+    def write_text(self, path, *args, **kwargs):  # type: (Path, *Any, **Any) -> Path
+        return self._path_method_wrapper(path, "write_text", *args, **kwargs)[0]
+
+    def mkdir(self, path, *args, **kwargs):  # type: (Path, *Any, **Any) -> Path
+        return self._path_method_wrapper(path, "mkdir", *args, **kwargs)[0]
+
+    def exists(self, path):  # type: (Path) -> bool
+        return any(
+            value[-1]
+            for value in self._path_method_wrapper(path, "exists", return_first=False)
+        )
+
+    def find(self, path, writable_only=False):  # type: (Path, bool) -> List[Path]
+        return [
+            value[0]
+            for value in self._path_method_wrapper(
+                path, "exists", return_first=False, writable_only=writable_only
+            )
+            if value[-1] is True
+        ]
+
+    def __getattr__(self, item):
+        try:
+            return super(SitePackages, self).__getattribute__(item)
+        except AttributeError:
+            return getattr(self.path, item)
+
+
 class EnvError(Exception):
 
     pass
@@ -825,9 +946,13 @@ def pip_version(self):
         return self._pip_version
 
     @property
-    def site_packages(self):  # type: () -> Path
+    def site_packages(self):  # type: () -> SitePackages
         if self._site_packages is None:
-            self._site_packages = self.purelib
+            # we disable write checks if no user site exist
+            fallbacks = [self.usersite] if self.usersite else []
+            self._site_packages = SitePackages(
+                self.purelib, fallbacks, skip_write_checks=False if fallbacks else True
+            )
         return self._site_packages
 
     @property

poetry/utils/helpers.py

@@ -5,6 +5,7 @@
 import tempfile
 
 from contextlib import contextmanager
+from typing import List
 from typing import Optional
 
 import requests
@@ -113,3 +114,7 @@ def get_package_version_display_string(
         )
 
     return package.full_pretty_version
+
+
+def paths_csv(paths):  # type: (List[Path]) -> str
+    return ", ".join('"{}"'.format(str(c)) for c in paths)

tests/installation/test_pip_installer.py

@@ -189,7 +189,9 @@ def test_uninstall_git_package_nspkg_pth_cleanup(mocker, tmp_venv, pool):
     )
 
     # we do this here because the virtual env might not be usable if failure case is triggered
-    pth_file_candidate = tmp_venv.site_packages / "{}-nspkg.pth".format(package.name)
+    pth_file_candidate = tmp_venv.site_packages.path / "{}-nspkg.pth".format(
+        package.name
+    )
 
     # in order to reproduce the scenario where the git source is removed prior to proper
     # clean up of nspkg.pth file, we need to make sure the fixture is copied and not

tests/masonry/builders/test_editable_builder.py

@@ -76,14 +76,14 @@ def test_builder_installs_proper_files_for_standard_packages(simple_poetry, tmp_
     builder.build()
 
     assert tmp_venv._bin_dir.joinpath("foo").exists()
-    assert tmp_venv.site_packages.joinpath("simple_project.pth").exists()
-    assert simple_poetry.file.parent.resolve().as_posix() == tmp_venv.site_packages.joinpath(
+    assert tmp_venv.site_packages.path.joinpath("simple_project.pth").exists()
+    assert simple_poetry.file.parent.resolve().as_posix() == tmp_venv.site_packages.path.joinpath(
         "simple_project.pth"
     ).read_text().strip(
         os.linesep
     )
 
-    dist_info = tmp_venv.site_packages.joinpath("simple_project-1.2.3.dist-info")
+    dist_info = tmp_venv.site_packages.path.joinpath("simple_project-1.2.3.dist-info")
     assert dist_info.exists()
     assert dist_info.joinpath("INSTALLER").exists()
     assert dist_info.joinpath("METADATA").exists()
@@ -130,7 +130,7 @@ def test_builder_installs_proper_files_for_standard_packages(simple_poetry, tmp_
     assert metadata == dist_info.joinpath("METADATA").read_text(encoding="utf-8")
 
     records = dist_info.joinpath("RECORD").read_text()
-    assert str(tmp_venv.site_packages.joinpath("simple_project.pth")) in records
+    assert str(tmp_venv.site_packages.path.joinpath("simple_project.pth")) in records
     assert str(tmp_venv._bin_dir.joinpath("foo")) in records
     assert str(tmp_venv._bin_dir.joinpath("baz")) in records
     assert str(dist_info.joinpath("METADATA")) in records
@@ -202,7 +202,7 @@ def test_builder_installs_proper_files_when_packages_configured(
     builder = EditableBuilder(project_with_include, tmp_venv, NullIO())
     builder.build()
 
-    pth_file = tmp_venv.site_packages.joinpath("with_include.pth")
+    pth_file = tmp_venv.site_packages.path.joinpath("with_include.pth")
     assert pth_file.is_file()
 
     paths = set()

tests/utils/test_env.py

@@ -866,7 +866,7 @@ def test_system_env_has_correct_paths():
     assert paths.get("purelib") is not None
     assert paths.get("platlib") is not None
     assert paths.get("scripts") is not None
-    assert env.site_packages == Path(paths["purelib"])
+    assert env.site_packages.path == Path(paths["purelib"])
 
 
 @pytest.mark.parametrize(
@@ -886,4 +886,4 @@ def test_venv_has_correct_paths(tmp_venv):
     assert paths.get("purelib") is not None
     assert paths.get("platlib") is not None
     assert paths.get("scripts") is not None
-    assert tmp_venv.site_packages == Path(paths["purelib"])
+    assert tmp_venv.site_packages.path == Path(paths["purelib"])

tests/utils/test_env_site.py

@@ -0,0 +1,41 @@
+import uuid
+
+from poetry.utils._compat import Path
+from poetry.utils._compat import decode
+from poetry.utils.env import SitePackages
+
+
+def test_env_site_simple(tmp_dir):
+    site_packages = SitePackages(Path("/non-existent"), fallbacks=[Path(tmp_dir)])
+    candidates = site_packages.make_candidates(Path("hello.txt"), writable_only=True)
+    hello = Path(tmp_dir) / "hello.txt"
+
+    assert len(candidates) == 1
+    assert candidates[0].as_posix() == hello.as_posix()
+
+    content = decode(str(uuid.uuid4()))
+    site_packages.write_text(Path("hello.txt"), content, encoding="utf-8")
+
+    assert hello.read_text(encoding="utf-8") == content
+
+    assert not (site_packages.path / "hello.txt").exists()
+
+
+def test_env_site_select_first(tmp_dir):
+    path = Path(tmp_dir)
+    fallback = path / "fallback"
+    fallback.mkdir(parents=True)
+
+    site_packages = SitePackages(path, fallbacks=[fallback])
+    candidates = site_packages.make_candidates(Path("hello.txt"), writable_only=True)
+
+    assert len(candidates) == 2
+    assert len(site_packages.find(Path("hello.txt"))) == 0
+
+    content = decode(str(uuid.uuid4()))
+    site_packages.write_text(Path("hello.txt"), content, encoding="utf-8")
+
+    assert (site_packages.path / "hello.txt").exists()
+    assert not (fallback / "hello.txt").exists()
+
+    assert len(site_packages.find(Path("hello.txt"))) == 1