From 3a7637b77568dabc5cd10e62b203432556bd4f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Eustace?= Date: Sat, 9 May 2020 01:39:11 +0200 Subject: [PATCH 1/2] Fix resolution of path, url and VCS dependencies --- poetry/mixology/version_solver.py | 10 ++ poetry/packages/directory_dependency.py | 11 ++ poetry/packages/file_dependency.py | 15 +++ poetry/packages/package.py | 1 + poetry/packages/url_dependency.py | 6 + poetry/packages/vcs_dependency.py | 8 ++ poetry/puzzle/provider.py | 73 ++++++++++-- .../pyproject.toml | 1 + .../inner-directory-project/pyproject.toml | 11 ++ .../pyproject.toml | 1 + ...irectory-dependency-poetry-transitive.test | 73 +++++++++++- .../with-file-dependency-transitive.test | 22 +++- tests/installation/test_installer.py | 20 ++-- tests/puzzle/test_provider.py | 17 ++- tests/puzzle/test_solver.py | 111 ++++++++++++++++++ 15 files changed, 346 insertions(+), 34 deletions(-) create mode 100644 tests/fixtures/directory/project_with_transitive_file_dependencies/inner-directory-project/pyproject.toml diff --git a/poetry/mixology/version_solver.py b/poetry/mixology/version_solver.py index d1c78ba7661..02b9a74a1d8 100644 --- a/poetry/mixology/version_solver.py +++ b/poetry/mixology/version_solver.py @@ -339,6 +339,16 @@ def _get_min(dependency): if dependency.name in self._locked: return 1 + # VCS, URL, File or Directory dependencies + # represent a single version + if ( + dependency.is_vcs() + or dependency.is_url() + or dependency.is_file() + or dependency.is_directory() + ): + return 1 + try: return len(self._provider.search_for(dependency)) except ValueError: diff --git a/poetry/packages/directory_dependency.py b/poetry/packages/directory_dependency.py index ac4bca8ea82..978300ca689 100644 --- a/poetry/packages/directory_dependency.py +++ b/poetry/packages/directory_dependency.py @@ -79,3 +79,14 @@ def supports_poetry(self): def is_directory(self): return True + + def __str__(self): + if self.is_root: + return self._pretty_name + + return "{} ({} {})".format( + self._pretty_name, self._pretty_constraint, self._path + ) + + def __hash__(self): + return hash((self._name, self._full_path)) diff --git a/poetry/packages/file_dependency.py b/poetry/packages/file_dependency.py index b91d296a512..751e61997f8 100644 --- a/poetry/packages/file_dependency.py +++ b/poetry/packages/file_dependency.py @@ -41,6 +41,10 @@ def __init__( name, "*", category=category, optional=optional, allows_prereleases=True ) + @property + def base(self): + return self._base + @property def path(self): return self._path @@ -59,3 +63,14 @@ def hash(self): h.update(content) return h.hexdigest() + + def __str__(self): + if self.is_root: + return self._pretty_name + + return "{} ({} {})".format( + self._pretty_name, self._pretty_constraint, self._path + ) + + def __hash__(self): + return hash((self._name, self._full_path)) diff --git a/poetry/packages/package.py b/poetry/packages/package.py index 4940d6f027a..680bf31f596 100644 --- a/poetry/packages/package.py +++ b/poetry/packages/package.py @@ -409,6 +409,7 @@ def with_python_versions(self, python_versions): def clone(self): # type: () -> Package clone = self.__class__(self.pretty_name, self.version) + clone.description = self.description clone.category = self.category clone.optional = self.optional clone.python_versions = self.python_versions diff --git a/poetry/packages/url_dependency.py b/poetry/packages/url_dependency.py index f128dc49366..295fea60e6e 100644 --- a/poetry/packages/url_dependency.py +++ b/poetry/packages/url_dependency.py @@ -38,3 +38,9 @@ def base_pep_508_name(self): # type: () -> str def is_url(self): # type: () -> bool return True + + def __str__(self): + return "{} ({} url)".format(self._pretty_name, self._pretty_constraint) + + def __hash__(self): + return hash((self._name, self._url)) diff --git a/poetry/packages/vcs_dependency.py b/poetry/packages/vcs_dependency.py index 7574b8dbac3..b8f2992b92e 100644 --- a/poetry/packages/vcs_dependency.py +++ b/poetry/packages/vcs_dependency.py @@ -94,3 +94,11 @@ def is_vcs(self): # type: () -> bool def accepts_prereleases(self): # type: () -> bool return True + + def __str__(self): + return "{} ({} {})".format( + self._pretty_name, self._pretty_constraint, self._vcs + ) + + def __hash__(self): + return hash((self._name, self._vcs, self._branch, self._tag, self._rev)) diff --git a/poetry/puzzle/provider.py b/poetry/puzzle/provider.py index 4888b9828b1..3157b2a52da 100644 --- a/poetry/puzzle/provider.py +++ b/poetry/puzzle/provider.py @@ -72,6 +72,7 @@ def __init__(self, package, pool, io): # type: (Package, Pool, Any) -> None self._search_for = {} self._is_debugging = self._io.is_debug() or self._io.is_very_verbose() self._in_progress = False + self._deferred_cache = {} @property def pool(self): # type: () -> Pool @@ -164,6 +165,9 @@ def search_for_vcs(self, dependency): # type: (VCSDependency) -> List[Package] Basically, we clone the repository in a temporary directory and get the information we need by checking out the specified reference. """ + if dependency in self._deferred_cache: + return [self._deferred_cache[dependency]] + package = self.get_package_from_vcs( dependency.vcs, dependency.source, @@ -178,6 +182,11 @@ def search_for_vcs(self, dependency): # type: (VCSDependency) -> List[Package] package.requires += package.extras[extra] + dependency._constraint = package.version + dependency._pretty_constraint = package.version.text + + self._deferred_cache[dependency] = package + return [package] @classmethod @@ -214,7 +223,17 @@ def get_package_from_vcs( return package def search_for_file(self, dependency): # type: (FileDependency) -> List[Package] - package = self.get_package_from_file(dependency.full_path) + if dependency in self._deferred_cache: + dependency, _package = self._deferred_cache[dependency] + + package = _package.clone() + else: + package = self.get_package_from_file(dependency.full_path) + + dependency._constraint = package.version + dependency._pretty_constraint = package.version.text + + self._deferred_cache[dependency] = (dependency, package) if dependency.name != package.name: # For now, the dependency's name must match the actual package's name @@ -224,6 +243,9 @@ def search_for_file(self, dependency): # type: (FileDependency) -> List[Package ) ) + if dependency.base is not None: + package.root_dir = dependency.base + package.source_url = dependency.path.as_posix() package.files = [ {"file": dependency.path.name, "hash": "sha256:" + dependency.hash()} @@ -270,15 +292,25 @@ def get_package_from_file(cls, file_path): # type: (Path) -> Package def search_for_directory( self, dependency ): # type: (DirectoryDependency) -> List[Package] - package = self.get_package_from_directory( - dependency.full_path, name=dependency.name - ) + if dependency in self._deferred_cache: + dependency, _package = self._deferred_cache[dependency] + + package = _package.clone() + else: + package = self.get_package_from_directory( + dependency.full_path, name=dependency.name + ) + + dependency._constraint = package.version + dependency._pretty_constraint = package.version.text + + self._deferred_cache[dependency] = (dependency, package) package.source_url = dependency.path.as_posix() package.develop = dependency.develop if dependency.base is not None: - package.root_dir = dependency.base.as_posix() + package.root_dir = dependency.base for extra in dependency.extras: if extra in package.extras: @@ -434,6 +466,9 @@ def get_package_from_directory( return package def search_for_url(self, dependency): # type: (URLDependency) -> List[Package] + if dependency in self._deferred_cache: + return [self._deferred_cache[dependency]] + package = self.get_package_from_url(dependency.url) if dependency.name != package.name: @@ -451,6 +486,11 @@ def search_for_url(self, dependency): # type: (URLDependency) -> List[Package] package.requires += package.extras[extra] + dependency._constraint = package.version + dependency._pretty_constraint = package.version.text + + self._deferred_cache[dependency] = package + return [package] @classmethod @@ -551,6 +591,17 @@ def complete_package( else: requires = package.requires + # Retrieving constraints for deferred dependencies + for r in requires: + if r.is_directory(): + self.search_for_directory(r) + elif r.is_file(): + self.search_for_file(r) + elif r.is_vcs(): + self.search_for_vcs(r) + elif r.is_url(): + self.search_for_url(r) + dependencies = [ r for r in requires @@ -696,15 +747,15 @@ def complete_package( if (package.dependency.is_directory() or package.dependency.is_file()) and ( dep.is_directory() or dep.is_file() ): - if dep.path.as_posix().startswith(package.source_url): - relative = (Path(package.source_url) / dep.path).relative_to( - package.source_url + relative_path = Path( + os.path.relpath( + dep.full_path.as_posix(), package.root_dir.as_posix() ) - else: - relative = Path(package.source_url) / dep.path + ) # TODO: Improve the way we set the correct relative path for dependencies - dep._path = relative + dep._path = relative_path + clean_dependencies.append(dep) package.requires = clean_dependencies diff --git a/tests/fixtures/directory/project_with_transitive_directory_dependencies/pyproject.toml b/tests/fixtures/directory/project_with_transitive_directory_dependencies/pyproject.toml index 0a97ea02c5f..28678e0d0ae 100644 --- a/tests/fixtures/directory/project_with_transitive_directory_dependencies/pyproject.toml +++ b/tests/fixtures/directory/project_with_transitive_directory_dependencies/pyproject.toml @@ -8,5 +8,6 @@ license = "MIT" [tool.poetry.dependencies] python = "*" project-with-extras = {path = "../../project_with_extras/"} +project-with-transitive-file-dependencies = {path = "../project_with_transitive_file_dependencies/"} [tool.poetry.dev-dependencies] diff --git a/tests/fixtures/directory/project_with_transitive_file_dependencies/inner-directory-project/pyproject.toml b/tests/fixtures/directory/project_with_transitive_file_dependencies/inner-directory-project/pyproject.toml new file mode 100644 index 00000000000..a80113675e6 --- /dev/null +++ b/tests/fixtures/directory/project_with_transitive_file_dependencies/inner-directory-project/pyproject.toml @@ -0,0 +1,11 @@ +[tool.poetry] +name = "inner-directory-project" +version = "1.2.4" +description = "This is a description" +authors = ["Your Name "] +license = "MIT" + +[tool.poetry.dependencies] +python = "*" + +[tool.poetry.dev-dependencies] diff --git a/tests/fixtures/directory/project_with_transitive_file_dependencies/pyproject.toml b/tests/fixtures/directory/project_with_transitive_file_dependencies/pyproject.toml index 14512e2746e..678e42f2fa5 100644 --- a/tests/fixtures/directory/project_with_transitive_file_dependencies/pyproject.toml +++ b/tests/fixtures/directory/project_with_transitive_file_dependencies/pyproject.toml @@ -8,5 +8,6 @@ license = "MIT" [tool.poetry.dependencies] python = "*" demo = {path = "../../distributions/demo-0.1.0-py2.py3-none-any.whl"} +inner-directory-project = {path = "./inner-directory-project"} [tool.poetry.dev-dependencies] diff --git a/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test b/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test index 06f262f74e6..ad29dd7abe6 100644 --- a/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test +++ b/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test @@ -1,3 +1,45 @@ +[[package]] +category = "main" +description = "" +name = "demo" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" +version = "0.1.0" + +[package.dependencies] +pendulum = ">=1.4.4" + +[package.extras] +bar = ["tomlkit"] +foo = ["cleo"] + +[package.source] +reference = "" +type = "file" +url = "../../distributions/demo-0.1.0-py2.py3-none-any.whl" + +[[package]] +category = "main" +description = "" +develop = true +name = "inner-directory-project" +optional = false +python-versions = "*" +version = "1.2.4" + +[package.source] +reference = "" +type = "directory" +url = "../project_with_transitive_file_dependencies/inner-directory-project" + +[[package]] +category = "main" +description = "" +name = "pendulum" +optional = false +python-versions = "*" +version = "1.4.4" + [[package]] category = "main" description = "" @@ -14,7 +56,7 @@ extras_b = ["cachy (>=0.2.0)"] [package.source] reference = "" type = "directory" -url = "tests/fixtures/directory/project_with_transitive_directory_dependencies/../../project_with_extras" +url = "../project_with_extras" [[package]] category = "main" @@ -26,17 +68,42 @@ python-versions = "*" version = "1.2.3" [package.dependencies] -project-with-extras = "*" +project-with-extras = "1.2.3" +project-with-transitive-file-dependencies = "1.2.3" + +[package.source] +reference = "" +type = "directory" +url = "project_with_transitive_directory_dependencies" + +[[package]] +category = "main" +description = "" +develop = true +name = "project-with-transitive-file-dependencies" +optional = false +python-versions = "*" +version = "1.2.3" + +[package.dependencies] +demo = "0.1.0" +inner-directory-project = "1.2.4" [package.source] reference = "" type = "directory" -url = "tests/fixtures/directory/project_with_transitive_directory_dependencies" +url = "project_with_transitive_file_dependencies" [metadata] content-hash = "123456789" python-versions = "*" [metadata.files] +demo = [ + {file = "demo-0.1.0-py2.py3-none-any.whl", hash = "sha256:70e704135718fffbcbf61ed1fc45933cfd86951a744b681000eaaa75da31f17a"}, +] +inner-directory-project = [] +pendulum = [] project-with-extras = [] project-with-transitive-directory-dependencies = [] +project-with-transitive-file-dependencies = [] diff --git a/tests/installation/fixtures/with-file-dependency-transitive.test b/tests/installation/fixtures/with-file-dependency-transitive.test index db3b632694a..2cbd16e1a53 100644 --- a/tests/installation/fixtures/with-file-dependency-transitive.test +++ b/tests/installation/fixtures/with-file-dependency-transitive.test @@ -16,7 +16,21 @@ foo = ["cleo"] [package.source] reference = "" type = "file" -url = "tests/fixtures/directory/project_with_transitive_file_dependencies/../../distributions/demo-0.1.0-py2.py3-none-any.whl" +url = "../distributions/demo-0.1.0-py2.py3-none-any.whl" + +[[package]] +category = "main" +description = "" +develop = true +name = "inner-directory-project" +optional = false +python-versions = "*" +version = "1.2.4" + +[package.source] +reference = "" +type = "directory" +url = "project_with_transitive_file_dependencies/inner-directory-project" [[package]] category = "main" @@ -36,12 +50,13 @@ python-versions = "*" version = "1.2.3" [package.dependencies] -demo = "*" +demo = "0.1.0" +inner-directory-project = "1.2.4" [package.source] reference = "" type = "directory" -url = "tests/fixtures/directory/project_with_transitive_file_dependencies" +url = "project_with_transitive_file_dependencies" [metadata] content-hash = "123456789" @@ -51,5 +66,6 @@ python-versions = "*" demo = [ {file = "demo-0.1.0-py2.py3-none-any.whl", hash = "sha256:70e704135718fffbcbf61ed1fc45933cfd86951a744b681000eaaa75da31f17a"}, ] +inner-directory-project = [] pendulum = [] project-with-transitive-file-dependencies = [] diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index c236141be44..ee5318a5a23 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -716,11 +716,13 @@ def test_run_installs_with_local_poetry_directory_and_extras( def test_run_installs_with_local_poetry_directory_transitive( installer, locker, repo, package, tmpdir ): - file_path = ( - fixtures_dir / "directory/project_with_transitive_directory_dependencies/" + package.root_dir = fixtures_dir.joinpath("directory") + directory = fixtures_dir.joinpath("directory").joinpath( + "project_with_transitive_directory_dependencies" ) package.add_dependency( - "project-with-transitive-directory-dependencies", {"path": str(file_path)} + "project-with-transitive-directory-dependencies", + {"path": str(directory.relative_to(fixtures_dir.joinpath("directory")))}, ) repo.add_package(get_package("pendulum", "1.4.4")) @@ -732,15 +734,19 @@ def test_run_installs_with_local_poetry_directory_transitive( assert locker.written_data == expected - assert len(installer.installer.installs) == 2 + assert len(installer.installer.installs) == 6 def test_run_installs_with_local_poetry_file_transitive( installer, locker, repo, package, tmpdir ): - file_path = fixtures_dir / "directory/project_with_transitive_file_dependencies/" + package.root_dir = fixtures_dir.joinpath("directory") + directory = fixtures_dir.joinpath("directory").joinpath( + "project_with_transitive_file_dependencies" + ) package.add_dependency( - "project-with-transitive-file-dependencies", {"path": str(file_path)} + "project-with-transitive-file-dependencies", + {"path": str(directory.relative_to(fixtures_dir.joinpath("directory")))}, ) repo.add_package(get_package("pendulum", "1.4.4")) @@ -752,7 +758,7 @@ def test_run_installs_with_local_poetry_file_transitive( assert locker.written_data == expected - assert len(installer.installer.installs) == 3 + assert len(installer.installer.installs) == 4 def test_run_installs_with_local_setuptools_directory( diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py index 4d4463b3b73..3431911866a 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -204,16 +204,13 @@ def test_search_for_directory_setup_with_base(provider, directory): "foo": [get_dependency("cleo")], "bar": [get_dependency("tomlkit")], } - assert ( - package.root_dir - == ( - Path(__file__).parent.parent - / "fixtures" - / "git" - / "github.com" - / "demo" - / directory - ).as_posix() + assert package.root_dir == ( + Path(__file__).parent.parent + / "fixtures" + / "git" + / "github.com" + / "demo" + / directory ) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 415f6bcea09..1c75b59862f 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -1928,3 +1928,114 @@ def test_solver_properly_propagates_markers(solver, repo, package): str(ops[0].package.marker) == 'python_version >= "3.6" and implementation_name != "pypy"' ) + + +def test_solver_cannot_choose_another_version_for_directory_dependencies( + solver, repo, package +): + pendulum = get_package("pendulum", "2.0.3") + demo = get_package("demo", "0.1.0") + foo = get_package("foo", "1.2.3") + foo.add_dependency("demo", "<0.1.2") + repo.add_package(foo) + repo.add_package(demo) + repo.add_package(pendulum) + + path = ( + Path(__file__).parent.parent + / "fixtures" + / "git" + / "github.com" + / "demo" + / "demo" + ).as_posix() + + package.add_dependency("demo", {"path": path}) + package.add_dependency("foo", "^1.2.3") + + # This is not solvable since the demo version is pinned + # via the directory dependency + with pytest.raises(SolverProblemError): + solver.solve() + + +def test_solver_cannot_choose_another_version_for_file_dependencies( + solver, repo, package +): + pendulum = get_package("pendulum", "2.0.3") + demo = get_package("demo", "0.0.8") + foo = get_package("foo", "1.2.3") + foo.add_dependency("demo", "<0.1.0") + repo.add_package(foo) + repo.add_package(demo) + repo.add_package(pendulum) + + path = ( + Path(__file__).parent.parent + / "fixtures" + / "distributions" + / "demo-0.1.0-py2.py3-none-any.whl" + ).as_posix() + + package.add_dependency("demo", {"path": path}) + package.add_dependency("foo", "^1.2.3") + + # This is not solvable since the demo version is pinned + # via the file dependency + with pytest.raises(SolverProblemError): + solver.solve() + + +def test_solver_cannot_choose_another_version_for_git_dependencies( + solver, repo, package +): + pendulum = get_package("pendulum", "2.0.3") + demo = get_package("demo", "0.0.8") + foo = get_package("foo", "1.2.3") + foo.add_dependency("demo", "<0.1.0") + repo.add_package(foo) + repo.add_package(demo) + repo.add_package(pendulum) + + package.add_dependency("demo", {"git": "https://github.com/demo/demo.git"}) + package.add_dependency("foo", "^1.2.3") + + # This is not solvable since the demo version is pinned + # via the file dependency + with pytest.raises(SolverProblemError): + solver.solve() + + +def test_solver_cannot_choose_another_version_for_url_dependencies( + solver, repo, package, http +): + path = ( + Path(__file__).parent.parent + / "fixtures" + / "distributions" + / "demo-0.1.0-py2.py3-none-any.whl" + ) + + http.register_uri( + "GET", + "https://foo.bar/demo-0.1.0-py2.py3-none-any.whl", + body=path.read_bytes(), + streaming=True, + ) + pendulum = get_package("pendulum", "2.0.3") + demo = get_package("demo", "0.0.8") + foo = get_package("foo", "1.2.3") + foo.add_dependency("demo", "<0.1.0") + repo.add_package(foo) + repo.add_package(demo) + repo.add_package(pendulum) + + package.add_dependency( + "demo", {"url": "https://foo.bar/distributions/demo-0.1.0-py2.py3-none-any.whl"} + ) + package.add_dependency("foo", "^1.2.3") + + # This is not solvable since the demo version is pinned + # via the git dependency + with pytest.raises(SolverProblemError): + solver.solve() From 57aed9a348e887ef4859207c54cdcc671d2e5475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Eustace?= Date: Sat, 9 May 2020 12:56:14 +0200 Subject: [PATCH 2/2] Fix editable installation of Poetry packages --- poetry/installation/pip_installer.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/poetry/installation/pip_installer.py b/poetry/installation/pip_installer.py index 5a3e8df7852..331a69b580e 100644 --- a/poetry/installation/pip_installer.py +++ b/poetry/installation/pip_installer.py @@ -1,7 +1,6 @@ import os import tempfile -from io import open from subprocess import CalledProcessError from clikit.api.io import IO @@ -181,9 +180,7 @@ def create_temporary_requirement(self, package): return name def install_directory(self, package): - from poetry.masonry.builder import SdistBuilder from poetry.factory import Factory - from poetry.utils._compat import decode from poetry.utils.env import NullEnv from poetry.utils.toml_file import TomlFile @@ -210,17 +207,20 @@ def install_directory(self, package): setup = os.path.join(req, "setup.py") has_setup = os.path.exists(setup) - if not has_setup and has_poetry and (package.develop or not has_build_system): + if has_poetry and (package.develop or not has_build_system): # We actually need to rely on creating a temporary setup.py # file since pip, as of this comment, does not support # build-system for editable packages # We also need it for non-PEP-517 packages - builder = SdistBuilder( + from poetry.masonry.builders.editable import EditableBuilder + + builder = EditableBuilder( Factory().create_poetry(pyproject.parent), NullEnv(), NullIO() ) - with open(setup, "w", encoding="utf-8") as f: - f.write(decode(builder.build_setup())) + builder.build() + + return if package.develop: args.append("-e")