From 819c328289afbcacf300001468828a6aa1228f52 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Wed, 7 Apr 2021 23:11:12 +0200 Subject: [PATCH] pep610: handle pure/plat lib differences cleanly --- poetry/installation/executor.py | 45 +++++++++------- poetry/installation/pip_installer.py | 5 +- poetry/utils/env.py | 52 ++++++++++++++----- tests/installation/test_executor.py | 25 +++++---- tests/installation/test_pip_installer.py | 11 ++-- .../masonry/builders/test_editable_builder.py | 24 +++++---- 6 files changed, 98 insertions(+), 64 deletions(-) diff --git a/poetry/installation/executor.py b/poetry/installation/executor.py index d9f1b4d8981..6533017abcb 100644 --- a/poetry/installation/executor.py +++ b/poetry/installation/executor.py @@ -714,6 +714,19 @@ def _download_archive(self, operation: Union[Install, Update], link: Link) -> Pa def _should_write_operation(self, operation: Operation) -> bool: return not operation.skipped or self._dry_run or self._verbose + @staticmethod + def _package_dist_info_path(package: "Package") -> Path: + from poetry.core.masonry.utils.helpers import escape_name + from poetry.core.masonry.utils.helpers import escape_version + + return Path( + f"{escape_name(package.pretty_name)}-{escape_version(package.version.text)}.dist-info" + ) + + @classmethod + def _direct_url_json_path(cls, package: "Package") -> Path: + return cls._package_dist_info_path(package) / "direct_url.json" + def _save_url_reference(self, operation: "OperationTypes") -> None: """ Create and store a PEP-610 `direct_url.json` file, if needed. @@ -721,9 +734,6 @@ def _save_url_reference(self, operation: "OperationTypes") -> None: if operation.job_type not in {"install", "update"}: return - from poetry.core.masonry.utils.helpers import escape_name - from poetry.core.masonry.utils.helpers import escape_version - package = operation.package if not package.source_url: @@ -732,14 +742,10 @@ def _save_url_reference(self, operation: "OperationTypes") -> None: # distribution. # That's not what we want so we remove the direct_url.json file, # if it exists. - dist_info = self._env.site_packages.path.joinpath( - "{}-{}.dist-info".format( - escape_name(package.pretty_name), - escape_version(package.version.text), - ) - ) - if dist_info.exists() and dist_info.joinpath("direct_url.json").exists(): - dist_info.joinpath("direct_url.json").unlink() + for direct_url in self._env.site_packages.find( + self._direct_url_json_path(package), True + ): + direct_url.unlink() return @@ -755,16 +761,15 @@ def _save_url_reference(self, operation: "OperationTypes") -> None: url_reference = self._create_file_url_reference(package) if url_reference: - dist_info = self._env.site_packages.path.joinpath( - "{}-{}.dist-info".format( - escape_name(package.name), escape_version(package.version.text) - ) - ) - - if dist_info.exists(): - dist_info.joinpath("direct_url.json").write_text( - json.dumps(url_reference), encoding="utf-8" + for path in self._env.site_packages.find( + self._package_dist_info_path(package), writable_only=True + ): + self._env.site_packages.write_text( + path / "direct_url.json", + json.dumps(url_reference), + encoding="utf-8", ) + break def _create_git_url_reference( self, package: "Package" diff --git a/poetry/installation/pip_installer.py b/poetry/installation/pip_installer.py index 7ed81dd2c91..77cf0b79259 100644 --- a/poetry/installation/pip_installer.py +++ b/poetry/installation/pip_installer.py @@ -117,10 +117,7 @@ def remove(self, package: "Package") -> None: raise # This is a workaround for https://github.com/pypa/pip/issues/4176 - nspkg_pth_file = self._env.site_packages.path / "{}-nspkg.pth".format( - package.name - ) - if nspkg_pth_file.exists(): + for nspkg_pth_file in self._env.site_packages.find(f"{package.name}-nspkg.pth"): nspkg_pth_file.unlink() # If we have a VCS package, remove its source directory diff --git a/poetry/utils/env.py b/poetry/utils/env.py index 3df76495acc..390bf428c30 100644 --- a/poetry/utils/env.py +++ b/poetry/utils/env.py @@ -152,17 +152,34 @@ def _version_nodot(version): class SitePackages: def __init__( - self, path: Path, fallbacks: List[Path] = None, skip_write_checks: bool = False + self, + purelib: Path, + platlib: Optional[Path] = None, + fallbacks: List[Path] = None, + skip_write_checks: bool = False, ) -> None: - self._path = path + self._purelib = purelib + self._platlib = platlib or purelib + + if platlib and platlib.resolve() == purelib.resolve(): + self._platlib = purelib + self._fallbacks = fallbacks or [] self._skip_write_checks = skip_write_checks - self._candidates = [self._path] + self._fallbacks + self._candidates = list({self._purelib, self._platlib}) + self._fallbacks self._writable_candidates = None if not skip_write_checks else self._candidates @property def path(self) -> Path: - return self._path + return self._purelib + + @property + def purelib(self) -> Path: + return self._purelib + + @property + def platlib(self) -> Path: + return self._platlib @property def candidates(self) -> List[Path]: @@ -200,12 +217,16 @@ def make_candidates(self, path: Path, writable_only: bool = False) -> List[Path] return [candidate / path for candidate in candidates if candidate] def _path_method_wrapper( - self, path: Path, method: str, *args: Any, **kwargs: Any + self, + path: Union[str, Path], + method: str, + *args: Any, + return_first: bool = True, + writable_only: bool = False, + **kwargs: 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) + if isinstance(path, str): + path = Path(path) candidates = self.make_candidates(path, writable_only=writable_only) @@ -234,19 +255,19 @@ def _path_method_wrapper( raise OSError("Unable to access any of {}".format(paths_csv(candidates))) - def write_text(self, path: Path, *args: Any, **kwargs: Any) -> Path: + def write_text(self, path: Union[str, Path], *args: Any, **kwargs: Any) -> Path: return self._path_method_wrapper(path, "write_text", *args, **kwargs)[0] - def mkdir(self, path: Path, *args: Any, **kwargs: Any) -> Path: + def mkdir(self, path: Union[str, Path], *args: Any, **kwargs: Any) -> Path: return self._path_method_wrapper(path, "mkdir", *args, **kwargs)[0] - def exists(self, path: Path) -> bool: + def exists(self, path: Union[str, Path]) -> bool: return any( value[-1] for value in self._path_method_wrapper(path, "exists", return_first=False) ) - def find(self, path: Path, writable_only: bool = False) -> List[Path]: + def find(self, path: Union[str, Path], writable_only: bool = False) -> List[Path]: return [ value[0] for value in self._path_method_wrapper( @@ -990,7 +1011,10 @@ def site_packages(self) -> SitePackages: # 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 + self.purelib, + self.platlib, + fallbacks, + skip_write_checks=False if fallbacks else True, ) return self._site_packages diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index f074078638d..07054ee1eb8 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -280,9 +280,10 @@ def test_executor_should_write_pep610_url_references_for_files( executor = Executor(tmp_venv, pool, config, io) executor.execute([Install(package)]) - dist_info = tmp_venv.site_packages.path.joinpath("demo-0.1.0.dist-info") - assert dist_info.exists() + dist_info = "demo-0.1.0.dist-info" + assert tmp_venv.site_packages.exists(dist_info) + dist_info = tmp_venv.site_packages.find(dist_info)[0] direct_url_file = dist_info.joinpath("direct_url.json") assert direct_url_file.exists() @@ -303,9 +304,10 @@ def test_executor_should_write_pep610_url_references_for_directories( executor = Executor(tmp_venv, pool, config, io) executor.execute([Install(package)]) - dist_info = tmp_venv.site_packages.path.joinpath("simple_project-1.2.3.dist-info") - assert dist_info.exists() + dist_info = "simple_project-1.2.3.dist-info" + assert tmp_venv.site_packages.exists(dist_info) + dist_info = tmp_venv.site_packages.find(dist_info)[0] direct_url_file = dist_info.joinpath("direct_url.json") assert direct_url_file.exists() @@ -330,9 +332,10 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( executor = Executor(tmp_venv, pool, config, io) executor.execute([Install(package)]) - dist_info = tmp_venv.site_packages.path.joinpath("simple_project-1.2.3.dist-info") - assert dist_info.exists() + dist_info_dir = "simple_project-1.2.3.dist-info" + assert tmp_venv.site_packages.exists(dist_info_dir) + dist_info = tmp_venv.site_packages.find(dist_info_dir)[0] direct_url_file = dist_info.joinpath("direct_url.json") assert direct_url_file.exists() @@ -355,9 +358,10 @@ def test_executor_should_write_pep610_url_references_for_urls( executor = Executor(tmp_venv, pool, config, io) executor.execute([Install(package)]) - dist_info = tmp_venv.site_packages.path.joinpath("demo-0.1.0.dist-info") - assert dist_info.exists() + dist_info = "demo-0.1.0.dist-info" + assert tmp_venv.site_packages.exists(dist_info) + dist_info = tmp_venv.site_packages.find(dist_info)[0] direct_url_file = dist_info.joinpath("direct_url.json") assert direct_url_file.exists() @@ -385,9 +389,10 @@ def test_executor_should_write_pep610_url_references_for_git( executor = Executor(tmp_venv, pool, config, io) executor.execute([Install(package)]) - dist_info = tmp_venv.site_packages.path.joinpath("demo-0.1.2.dist-info") - assert dist_info.exists() + dist_info = "demo-0.1.2.dist-info" + assert tmp_venv.site_packages.exists(dist_info) + dist_info = tmp_venv.site_packages.find(dist_info)[0] direct_url_file = dist_info.joinpath("direct_url.json") assert direct_url_file.exists() diff --git a/tests/installation/test_pip_installer.py b/tests/installation/test_pip_installer.py index 34c9cc6b4bd..20fc9d0a9d3 100644 --- a/tests/installation/test_pip_installer.py +++ b/tests/installation/test_pip_installer.py @@ -1,3 +1,4 @@ +import re import shutil from pathlib import Path @@ -190,11 +191,6 @@ def test_uninstall_git_package_nspkg_pth_cleanup(mocker, tmp_venv, pool): source_reference="master", ) - # we do this here because the virtual env might not be usable if failure case is triggered - 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 # symlinked into the git src directory @@ -213,8 +209,9 @@ def copy_only(source, dest): installer.install(package) installer.remove(package) - assert not Path(pth_file_candidate).exists() + pth_file = f"{package.name}-nspkg.pth" + assert not tmp_venv.site_packages.exists(pth_file) # any command in the virtual environment should trigger the error message output = tmp_venv.run("python", "-m", "site") - assert "Error processing line 1 of {}".format(pth_file_candidate) not in output + assert not re.match(rf"Error processing line 1 of .*{pth_file}", output) diff --git a/tests/masonry/builders/test_editable_builder.py b/tests/masonry/builders/test_editable_builder.py index 05e798012d5..86ae73ece5b 100644 --- a/tests/masonry/builders/test_editable_builder.py +++ b/tests/masonry/builders/test_editable_builder.py @@ -78,16 +78,18 @@ 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.path.joinpath("simple_project.pth").exists() + pth_file = "simple_project.pth" + assert tmp_venv.site_packages.exists(pth_file) assert ( simple_poetry.file.parent.resolve().as_posix() - == tmp_venv.site_packages.path.joinpath("simple_project.pth") - .read_text() - .strip(os.linesep) + == tmp_venv.site_packages.find(pth_file)[0].read_text().strip(os.linesep) ) - dist_info = tmp_venv.site_packages.path.joinpath("simple_project-1.2.3.dist-info") - assert dist_info.exists() + dist_info = "simple_project-1.2.3.dist-info" + assert tmp_venv.site_packages.exists(dist_info) + + dist_info = tmp_venv.site_packages.find(dist_info)[0] + assert dist_info.joinpath("INSTALLER").exists() assert dist_info.joinpath("METADATA").exists() assert dist_info.joinpath("RECORD").exists() @@ -134,7 +136,9 @@ 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.path.joinpath("simple_project.pth")) in records + pth_file = "simple_project.pth" + assert tmp_venv.site_packages.exists(pth_file) + assert str(tmp_venv.site_packages.find(pth_file)[0]) 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 @@ -201,8 +205,10 @@ 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.path.joinpath("with_include.pth") - assert pth_file.is_file() + pth_file = "with_include.pth" + assert tmp_venv.site_packages.exists(pth_file) + + pth_file = tmp_venv.site_packages.find(pth_file)[0] paths = set() with pth_file.open() as f: