From 6057b2fece91707f906426cef7a0d69c9e31e960 Mon Sep 17 00:00:00 2001 From: ota42y Date: Fri, 2 Apr 2021 18:05:25 +0900 Subject: [PATCH 1/3] refactoring temp directory --- poetry/console/commands/init.py | 7 ++-- poetry/puzzle/provider.py | 67 ++++++++++++++++++--------------- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/poetry/console/commands/init.py b/poetry/console/commands/init.py index b2e4fd0a315..ab5c114cb5d 100644 --- a/poetry/console/commands/init.py +++ b/poetry/console/commands/init.py @@ -414,9 +414,10 @@ def _parse_requirements(self, requirements: List[str]) -> List[Dict[str, str]]: if extras: pair["extras"] = extras - package = Provider.get_package_from_vcs( - "git", url.url, rev=pair.get("rev") - ) + with Provider.build_tmp_dir_for_vcs(url.url) as tmp_dir: + package = Provider.get_package_from_vcs( + "git", url.url, tmp_dir, rev=pair.get("rev") + ) pair["name"] = package.name result.append(pair) diff --git a/poetry/puzzle/provider.py b/poetry/puzzle/provider.py index c7427161c2c..6191fe2a332 100644 --- a/poetry/puzzle/provider.py +++ b/poetry/puzzle/provider.py @@ -171,14 +171,16 @@ def search_for_vcs(self, dependency: VCSDependency) -> List[Package]: if dependency in self._deferred_cache: return [self._deferred_cache[dependency]] - package = self.get_package_from_vcs( - dependency.vcs, - dependency.source, - branch=dependency.branch, - tag=dependency.tag, - rev=dependency.rev, - name=dependency.name, - ) + with self.build_tmp_dir_for_vcs(dependency.source) as tmp_dir: + package = self.get_package_from_vcs( + dependency.vcs, + dependency.source, + tmp_dir, + branch=dependency.branch, + tag=dependency.tag, + rev=dependency.rev, + name=dependency.name, + ) package.develop = dependency.develop dependency._constraint = package.version @@ -188,11 +190,25 @@ def search_for_vcs(self, dependency: VCSDependency) -> List[Package]: return [package] + @classmethod + @contextmanager + def build_tmp_dir_for_vcs(cls, url): + tmp_dir = Path( + mkdtemp(prefix="pypoetry-git-{}".format(url.split("/")[-1].rstrip(".git"))) + ) + try: + yield(tmp_dir) + except Exception: + raise + finally: + safe_rmtree(str(tmp_dir)) + @classmethod def get_package_from_vcs( cls, vcs: str, url: str, + tmp_dir: Path, branch: Optional[str] = None, tag: Optional[str] = None, rev: Optional[str] = None, @@ -201,30 +217,21 @@ def get_package_from_vcs( if vcs != "git": raise ValueError(f"Unsupported VCS dependency {vcs}") - tmp_dir = Path( - mkdtemp(prefix="pypoetry-git-{}".format(url.split("/")[-1].rstrip(".git"))) - ) - - try: - git = Git() - git.clone(url, tmp_dir) - reference = branch or tag or rev - if reference is not None: - git.checkout(reference, tmp_dir) - else: - reference = "HEAD" + git = Git() + git.clone(url, tmp_dir) + reference = branch or tag or rev + if reference is not None: + git.checkout(reference, tmp_dir) + else: + reference = "HEAD" - revision = git.rev_parse(reference, tmp_dir).strip() + revision = git.rev_parse(reference, tmp_dir).strip() - package = cls.get_package_from_directory(tmp_dir, name=name) - package._source_type = "git" - package._source_url = url - package._source_reference = reference - package._source_resolved_reference = revision - except Exception: - raise - finally: - safe_rmtree(str(tmp_dir)) + package = cls.get_package_from_directory(tmp_dir, name=name) + package._source_type = "git" + package._source_url = url + package._source_reference = reference + package._source_resolved_reference = revision return package From 0e7fd40391b377dee1dea33fb34999bac31ee249 Mon Sep 17 00:00:00 2001 From: ota42y Date: Fri, 2 Apr 2021 17:35:52 +0900 Subject: [PATCH 2/3] add failed test case --- pytest.ini | 3 +++ tests/conftest.py | 6 +++++- .../demo/relative_dependency/setup.py | 10 ++++++++++ .../subdir_package/setup.py | 7 +++++++ .../subdir_package/subdir_package/__init__.py | 0 tests/puzzle/test_solver.py | 18 ++++++++++++++++++ 6 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 pytest.ini create mode 100644 tests/fixtures/git/github.com/demo/relative_dependency/setup.py create mode 100644 tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/setup.py create mode 100644 tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/subdir_package/__init__.py diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 00000000000..c9551efa53c --- /dev/null +++ b/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +markers = + execute_setup_py: execute setup py by venv diff --git a/tests/conftest.py b/tests/conftest.py index d66ed9ba659..95bc04846dc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -117,7 +117,11 @@ def download_mock(mocker): @pytest.fixture(autouse=True) -def pep517_metadata_mock(mocker): +def pep517_metadata_mock(request, mocker): + marker = request.node.get_closest_marker("execute_setup_py") + if marker: + return + @classmethod # noqa def _pep517_metadata(cls, path): try: diff --git a/tests/fixtures/git/github.com/demo/relative_dependency/setup.py b/tests/fixtures/git/github.com/demo/relative_dependency/setup.py new file mode 100644 index 00000000000..5296923740e --- /dev/null +++ b/tests/fixtures/git/github.com/demo/relative_dependency/setup.py @@ -0,0 +1,10 @@ +import os +from setuptools import setup + +setup( + name="relative_dependency", + version="0.1.0", + install_requires=[ + 'subdir_package @ file://localhost/%s/subdir_package/' % os.getcwd().replace('\\', '/'), + ] +) diff --git a/tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/setup.py b/tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/setup.py new file mode 100644 index 00000000000..d8253f76d6f --- /dev/null +++ b/tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/setup.py @@ -0,0 +1,7 @@ +from setuptools import setup, find_packages + +setup( + name="subdir_package", + version="0.1.0", + packages = find_packages(), +) diff --git a/tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/subdir_package/__init__.py b/tests/fixtures/git/github.com/demo/relative_dependency/subdir_package/subdir_package/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index dd193096d24..61a205acdea 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -2841,3 +2841,21 @@ def test_solver_should_not_raise_errors_for_irrelevant_transitive_python_constra {"job": "install", "package": virtualenv}, ], ) + + +@pytest.mark.execute_setup_py +def test_solver_git_dependencies_with_relative_path(solver, package): + package.add_dependency( + Factory.create_dependency("relative_dependency", {"git": "https://github.com/demo/relative_dependency.git"}) + ) + + ops = solver.solve() + + expecteds = [ + {"job": "install", "package": {"name": "subdir-package", "version": "0.1.0"}}, + {"job": "install", "package": {"name": "relative-dependency", "version": "0.1.0"}}, + ] + + op_list = [{"job": op.job_type, "package": {"name": op.package.name, "version": str(op.package.version)}} for op in ops] + assert expecteds == op_list + From ab4b5283838e4547ffbff22a24478e8144c9b5d9 Mon Sep 17 00:00:00 2001 From: ota42y Date: Sun, 4 Apr 2021 21:23:56 +0900 Subject: [PATCH 3/3] fix install relative path dependency --- poetry/console/commands/init.py | 5 ++- poetry/console/commands/show.py | 6 ++- poetry/mixology/__init__.py | 5 ++- poetry/mixology/version_solver.py | 10 +++-- poetry/puzzle/provider.py | 45 +++++++------------ poetry/utils/helpers.py | 25 +++++++++++ tests/conftest.py | 5 +++ .../demo/relative_dependency/setup.py | 3 +- tests/mixology/helpers.py | 24 +++++----- tests/puzzle/test_provider.py | 24 +++++----- tests/puzzle/test_solver.py | 2 +- 11 files changed, 91 insertions(+), 63 deletions(-) diff --git a/poetry/console/commands/init.py b/poetry/console/commands/init.py index ab5c114cb5d..d155a0e676f 100644 --- a/poetry/console/commands/init.py +++ b/poetry/console/commands/init.py @@ -16,7 +16,7 @@ from .command import Command from .env_command import EnvCommand - +from poetry.utils.helpers import with_temp_directory_manager if TYPE_CHECKING: from poetry.repositories import Pool @@ -414,7 +414,8 @@ def _parse_requirements(self, requirements: List[str]) -> List[Dict[str, str]]: if extras: pair["extras"] = extras - with Provider.build_tmp_dir_for_vcs(url.url) as tmp_dir: + with with_temp_directory_manager() as m: + tmp_dir = m.build_tmp_dir_for_vcs(url.url) package = Provider.get_package_from_vcs( "git", url.url, tmp_dir, rev=pair.get("rev") ) diff --git a/poetry/console/commands/show.py b/poetry/console/commands/show.py index 799fa4d78db..38f744ac9be 100644 --- a/poetry/console/commands/show.py +++ b/poetry/console/commands/show.py @@ -7,7 +7,7 @@ from cleo.helpers import option from .env_command import EnvCommand - +from poetry.utils.helpers import with_temp_directory_manager if TYPE_CHECKING: from cleo.io.io import IO # noqa @@ -395,7 +395,9 @@ def find_latest_package( provider = Provider(self.poetry.package, self.poetry.pool, NullIO()) if dep.is_vcs(): - return provider.search_for_vcs(dep)[0] + with with_temp_directory_manager() as m: + ret = provider.search_for_vcs(dep, m)[0] + return ret if dep.is_file(): return provider.search_for_file(dep)[0] if dep.is_directory(): diff --git a/poetry/mixology/__init__.py b/poetry/mixology/__init__.py index 8cf422203a9..d7f70faa604 100644 --- a/poetry/mixology/__init__.py +++ b/poetry/mixology/__init__.py @@ -3,7 +3,7 @@ from typing import List from .version_solver import VersionSolver - +from poetry.utils.helpers import with_temp_directory_manager if TYPE_CHECKING: from poetry.core.packages.project_package import ProjectPackage @@ -19,6 +19,7 @@ def resolve_version( locked: Dict[str, "DependencyPackage"] = None, use_latest: List[str] = None, ) -> "SolverResult": - solver = VersionSolver(root, provider, locked=locked, use_latest=use_latest) + with with_temp_directory_manager() as m: + solver = VersionSolver(root, provider, m, locked=locked, use_latest=use_latest) return solver.solve() diff --git a/poetry/mixology/version_solver.py b/poetry/mixology/version_solver.py index 2ef0c65268a..f3117cff2f9 100644 --- a/poetry/mixology/version_solver.py +++ b/poetry/mixology/version_solver.py @@ -10,6 +10,7 @@ from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package from poetry.core.packages.project_package import ProjectPackage +from poetry.puzzle.provider import TempDirManager from .failure import SolveFailure from .incompatibility import Incompatibility @@ -43,12 +44,14 @@ def __init__( self, root: ProjectPackage, provider: "Provider", + temp_dir_manager: TempDirManager, locked: Dict[str, Package] = None, use_latest: List[str] = None, ): self._root = root self._provider = provider self._locked = locked or {} + self._temp_dir_manager = temp_dir_manager if use_latest is None: use_latest = [] @@ -355,8 +358,9 @@ def _get_min(dependency: Dependency) -> Tuple[bool, int]: try: return ( not dependency.marker.is_any(), - len(self._provider.search_for(dependency)), + len(self._provider.search_for(dependency, self._temp_dir_manager)), ) + except ValueError: return not dependency.marker.is_any(), 0 @@ -368,7 +372,7 @@ def _get_min(dependency: Dependency) -> Tuple[bool, int]: locked = self._get_locked(dependency) if locked is None or not dependency.constraint.allows(locked.version): try: - packages = self._provider.search_for(dependency) + packages = self._provider.search_for(dependency, self._temp_dir_manager) except ValueError as e: self._add_incompatibility( Incompatibility([Term(dependency, True)], PackageNotFoundCause(e)) @@ -391,7 +395,7 @@ def _get_min(dependency: Dependency) -> Tuple[bool, int]: else: version = locked - version = self._provider.complete_package(version) + version = self._provider.complete_package(version, self._temp_dir_manager) conflict = False for incompatibility in self._provider.incompatibilities_for(version): diff --git a/poetry/puzzle/provider.py b/poetry/puzzle/provider.py index 6191fe2a332..b641fc4a08a 100644 --- a/poetry/puzzle/provider.py +++ b/poetry/puzzle/provider.py @@ -6,7 +6,6 @@ from contextlib import contextmanager from pathlib import Path -from tempfile import mkdtemp from typing import Any from typing import Dict from typing import Iterator @@ -40,7 +39,7 @@ from poetry.utils.helpers import download_file from poetry.utils.helpers import safe_rmtree from poetry.utils.helpers import temporary_directory - +from poetry.utils.helpers import TempDirManager logger = logging.getLogger(__name__) @@ -106,6 +105,7 @@ def search_for( DirectoryDependency, URLDependency, ], + temp_dir_manager: TempDirManager, ) -> List[DependencyPackage]: """ Search for the specifications that match the given dependency. @@ -139,7 +139,7 @@ def search_for( return PackageCollection(dependency, packages) if dependency.is_vcs(): - packages = self.search_for_vcs(dependency) + packages = self.search_for_vcs(dependency, temp_dir_manager) elif dependency.is_file(): packages = self.search_for_file(dependency) elif dependency.is_directory(): @@ -161,7 +161,7 @@ def search_for( return PackageCollection(dependency, packages) - def search_for_vcs(self, dependency: VCSDependency) -> List[Package]: + def search_for_vcs(self, dependency: VCSDependency, temp_dir_manager: TempDirManager) -> List[Package]: """ Search for the specifications that match the given VCS dependency. @@ -171,16 +171,16 @@ def search_for_vcs(self, dependency: VCSDependency) -> List[Package]: if dependency in self._deferred_cache: return [self._deferred_cache[dependency]] - with self.build_tmp_dir_for_vcs(dependency.source) as tmp_dir: - package = self.get_package_from_vcs( - dependency.vcs, - dependency.source, - tmp_dir, - branch=dependency.branch, - tag=dependency.tag, - rev=dependency.rev, - name=dependency.name, - ) + tmp_dir = temp_dir_manager.build_tmp_dir_for_vcs(dependency.source) + package = self.get_package_from_vcs( + dependency.vcs, + dependency.source, + tmp_dir, + branch=dependency.branch, + tag=dependency.tag, + rev=dependency.rev, + name=dependency.name, + ) package.develop = dependency.develop dependency._constraint = package.version @@ -190,19 +190,6 @@ def search_for_vcs(self, dependency: VCSDependency) -> List[Package]: return [package] - @classmethod - @contextmanager - def build_tmp_dir_for_vcs(cls, url): - tmp_dir = Path( - mkdtemp(prefix="pypoetry-git-{}".format(url.split("/")[-1].rstrip(".git"))) - ) - try: - yield(tmp_dir) - except Exception: - raise - finally: - safe_rmtree(str(tmp_dir)) - @classmethod def get_package_from_vcs( cls, @@ -437,7 +424,7 @@ def incompatibilities_for( for dep in dependencies ] - def complete_package(self, package: DependencyPackage) -> DependencyPackage: + def complete_package(self, package: DependencyPackage, temp_dir_manager: TempDirManager) -> DependencyPackage: if package.is_root(): package = package.clone() requires = package.all_requires @@ -468,7 +455,7 @@ def complete_package(self, package: DependencyPackage) -> DependencyPackage: elif r.is_file(): self.search_for_file(r) elif r.is_vcs(): - self.search_for_vcs(r) + self.search_for_vcs(r, temp_dir_manager) elif r.is_url(): self.search_for_url(r) diff --git a/poetry/utils/helpers.py b/poetry/utils/helpers.py index d81d0f15b3d..4b3db4075b9 100644 --- a/poetry/utils/helpers.py +++ b/poetry/utils/helpers.py @@ -135,3 +135,28 @@ def is_dir_writable(path: Path, create: bool = False) -> bool: return False else: return True + +@contextmanager +def with_temp_directory_manager(): + m = TempDirManager() + try: + yield(m) + except Exception: + raise + finally: + m.release() + +class TempDirManager(): + def __init__(self) -> None: + self._tmp_dirs = [] + + def release(self): + for d in self._tmp_dirs: + safe_rmtree(str(d)) + + def build_tmp_dir_for_vcs(self, url): + tmp_dir = Path( + tempfile.mkdtemp(prefix="pypoetry-git-{}".format(url.split("/")[-1].rstrip(".git"))) + ) + self._tmp_dirs.append(tmp_dir) + return tmp_dir diff --git a/tests/conftest.py b/tests/conftest.py index 95bc04846dc..4e2ba38306c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,6 +13,7 @@ from cleo.testers.command_tester import CommandTester +from poetry.utils.helpers import with_temp_directory_manager from poetry.config.config import Config as BaseConfig from poetry.config.dict_config_source import DictConfigSource from poetry.factory import Factory @@ -187,6 +188,10 @@ def tmp_dir(): shutil.rmtree(dir_) +@pytest.fixture +def temp_dir_manager(): + with with_temp_directory_manager() as m: + yield m @pytest.fixture def mocked_open_files(mocker): diff --git a/tests/fixtures/git/github.com/demo/relative_dependency/setup.py b/tests/fixtures/git/github.com/demo/relative_dependency/setup.py index 5296923740e..9fd07775331 100644 --- a/tests/fixtures/git/github.com/demo/relative_dependency/setup.py +++ b/tests/fixtures/git/github.com/demo/relative_dependency/setup.py @@ -1,10 +1,11 @@ import os from setuptools import setup +PKG_DIR = os.path.dirname(os.path.abspath(__file__)) setup( name="relative_dependency", version="0.1.0", install_requires=[ - 'subdir_package @ file://localhost/%s/subdir_package/' % os.getcwd().replace('\\', '/'), + f"subdir_package @ file://{PKG_DIR}/subdir_package" ] ) diff --git a/tests/mixology/helpers.py b/tests/mixology/helpers.py index dc0a48e526b..37d5e82f5ab 100644 --- a/tests/mixology/helpers.py +++ b/tests/mixology/helpers.py @@ -3,7 +3,7 @@ from poetry.mixology.failure import SolveFailure from poetry.mixology.version_solver import VersionSolver from poetry.packages import DependencyPackage - +from poetry.utils.helpers import with_temp_directory_manager def add_to_repo(repository, name, version, deps=None, python=None): package = Package(name, version) @@ -23,20 +23,22 @@ def check_solver_result( if locked is not None: locked = {k: DependencyPackage(l.to_dependency(), l) for k, l in locked.items()} - solver = VersionSolver(root, provider, locked=locked, use_latest=use_latest) + with with_temp_directory_manager() as m: + + solver = VersionSolver(root, provider, m, locked=locked, use_latest=use_latest) - try: - solution = solver.solve() - except SolveFailure as e: - if error: - assert str(e) == error + try: + solution = solver.solve() + except SolveFailure as e: + if error: + assert str(e) == error - if tries is not None: - assert solver.solution.attempted_solutions == tries + if tries is not None: + assert solver.solution.attempted_solutions == tries - return + return - raise + raise packages = {} for package in solution.packages: diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py index b5add046370..9789cc855ef 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -47,18 +47,18 @@ def provider(root, pool): @pytest.mark.parametrize("value", [True, False]) -def test_search_for_vcs_retains_develop_flag(provider, value): +def test_search_for_vcs_retains_develop_flag(provider, value, temp_dir_manager): dependency = VCSDependency( "demo", "git", "https://github.com/demo/demo.git", develop=value ) - package = provider.search_for_vcs(dependency)[0] + package = provider.search_for_vcs(dependency, temp_dir_manager)[0] assert package.develop == value -def test_search_for_vcs_setup_egg_info(provider): +def test_search_for_vcs_setup_egg_info(provider, temp_dir_manager): dependency = VCSDependency("demo", "git", "https://github.com/demo/demo.git") - package = provider.search_for_vcs(dependency)[0] + package = provider.search_for_vcs(dependency, temp_dir_manager)[0] assert package.name == "demo" assert package.version.text == "0.1.2" @@ -73,12 +73,12 @@ def test_search_for_vcs_setup_egg_info(provider): } -def test_search_for_vcs_setup_egg_info_with_extras(provider): +def test_search_for_vcs_setup_egg_info_with_extras(provider, temp_dir_manager): dependency = VCSDependency( "demo", "git", "https://github.com/demo/demo.git", extras=["foo"] ) - package = provider.search_for_vcs(dependency)[0] + package = provider.search_for_vcs(dependency, temp_dir_manager)[0] assert package.name == "demo" assert package.version.text == "0.1.2" @@ -93,12 +93,12 @@ def test_search_for_vcs_setup_egg_info_with_extras(provider): } -def test_search_for_vcs_read_setup(provider, mocker): +def test_search_for_vcs_read_setup(provider, mocker, temp_dir_manager): mocker.patch("poetry.utils.env.EnvManager.get", return_value=MockEnv()) dependency = VCSDependency("demo", "git", "https://github.com/demo/demo.git") - package = provider.search_for_vcs(dependency)[0] + package = provider.search_for_vcs(dependency, temp_dir_manager)[0] assert package.name == "demo" assert package.version.text == "0.1.2" @@ -113,14 +113,14 @@ def test_search_for_vcs_read_setup(provider, mocker): } -def test_search_for_vcs_read_setup_with_extras(provider, mocker): +def test_search_for_vcs_read_setup_with_extras(provider, mocker, temp_dir_manager): mocker.patch("poetry.utils.env.EnvManager.get", return_value=MockEnv()) dependency = VCSDependency( "demo", "git", "https://github.com/demo/demo.git", extras=["foo"] ) - package = provider.search_for_vcs(dependency)[0] + package = provider.search_for_vcs(dependency, temp_dir_manager)[0] assert package.name == "demo" assert package.version.text == "0.1.2" @@ -131,7 +131,7 @@ def test_search_for_vcs_read_setup_with_extras(provider, mocker): assert optional == [get_dependency("tomlkit"), get_dependency("cleo")] -def test_search_for_vcs_read_setup_raises_error_if_no_version(provider, mocker): +def test_search_for_vcs_read_setup_raises_error_if_no_version(provider, mocker, temp_dir_manager): mocker.patch( "poetry.inspection.info.PackageInfo._pep517_metadata", return_value=PackageInfo(name="demo", version=None), @@ -140,7 +140,7 @@ def test_search_for_vcs_read_setup_raises_error_if_no_version(provider, mocker): dependency = VCSDependency("demo", "git", "https://github.com/demo/no-version.git") with pytest.raises(RuntimeError): - provider.search_for_vcs(dependency) + provider.search_for_vcs(dependency, temp_dir_manager) @pytest.mark.parametrize("directory", ["demo", "non-canonical-name"]) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 61a205acdea..e1bd94d919a 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -2850,7 +2850,7 @@ def test_solver_git_dependencies_with_relative_path(solver, package): ) ops = solver.solve() - + expecteds = [ {"job": "install", "package": {"name": "subdir-package", "version": "0.1.0"}}, {"job": "install", "package": {"name": "relative-dependency", "version": "0.1.0"}},