Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse locked package info for vcs/file/url deps #2720

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion poetry/installation/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def _do_install(self, local_repo):

locked_repository = Repository()
if self._update:
if self._locker.is_locked() and not self._lock:
if self._locker.is_locked():
locked_repository = self._locker.locked_repository(True)

# If no packages have been whitelisted (The ones we want to update),
Expand Down
20 changes: 18 additions & 2 deletions poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from poetry.packages.package_collection import PackageCollection
from poetry.puzzle.exceptions import OverrideNeeded
from poetry.repositories import Pool
from poetry.repositories import Repository
from poetry.utils._compat import OrderedDict
from poetry.utils._compat import Path
from poetry.utils._compat import urlparse
Expand All @@ -55,8 +56,8 @@ class Provider:
UNSAFE_PACKAGES = {"setuptools", "distribute", "pip"}

def __init__(
self, package, pool, io, env=None
): # type: (Package, Pool, Any, Optional[Env]) -> None
self, package, pool, io, env=None, locked=None
): # type: (Package, Pool, Any, Optional[Env], Optional[Repository]) -> None
self._package = package
self._pool = pool
self._io = io
Expand All @@ -67,6 +68,11 @@ def __init__(
self._in_progress = False
self._overrides = {}
self._deferred_cache = {}
self._locked_packages = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this is the right place to put this.

We already have a locked system in the VersionSolver class, so we might be able to fix and improve it there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a whack at that; I was going for the lowest possible place to put this logic otherwise we will end up re-implementing the logic at multiple places. It might also be a good idea to re-work the structure a bit for 2.x.


if locked:
for package in locked.packages:
self._locked_packages[package.name] = package

@property
def pool(self): # type: () -> Pool
Expand Down Expand Up @@ -164,6 +170,9 @@ def search_for_vcs(self, dependency): # type: (VCSDependency) -> List[Package]
if dependency in self._deferred_cache:
return [self._deferred_cache[dependency]]

if dependency.name in self._locked_packages:
return [self._locked_packages[dependency.name]]

package = self.get_package_from_vcs(
dependency.vcs,
dependency.source,
Expand Down Expand Up @@ -223,6 +232,8 @@ def search_for_file(self, dependency): # type: (FileDependency) -> List[Package
dependency, _package = self._deferred_cache[dependency]

package = _package.clone()
elif dependency.name in self._locked_packages:
package = self._locked_packages[dependency.name].clone()
else:
package = self.get_package_from_file(dependency.full_path)

Expand Down Expand Up @@ -279,6 +290,8 @@ def search_for_directory(
dependency, _package = self._deferred_cache[dependency]

package = _package.clone()
elif dependency.name in self._locked_packages:
package = self._locked_packages[dependency.name].clone()
else:
package = self.get_package_from_directory(
dependency.full_path, name=dependency.name
Expand Down Expand Up @@ -329,6 +342,9 @@ def search_for_url(self, dependency): # type: (URLDependency) -> List[Package]
if dependency in self._deferred_cache:
return [self._deferred_cache[dependency]]

if dependency.name in self._locked_packages:
return [self._locked_packages[dependency.name]]

package = self.get_package_from_url(dependency.url)

if dependency.name != package.name:
Expand Down
4 changes: 3 additions & 1 deletion poetry/puzzle/solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def __init__(
self._io = io

if provider is None:
provider = Provider(self._package, self._pool, self._io)
provider = Provider(
self._package, self._pool, self._io, locked=self._locked
)

self._provider = provider
self._overrides = []
Expand Down
21 changes: 21 additions & 0 deletions tests/installation/fixtures/with-reusable-package-info-file.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[[package]]
name = "demo"
version = "0.1.0"
description = ""
category = "main"
optional = false
python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*"

[package.source]
type = "file"
reference = ""
url = "tests/fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl"

[package.dependencies]

[package.extras]

[metadata.files]
demo = [
{file = "demo-0.1.0-py2.py3-none-any.whl", hash = "sha256:70e704135718fffbcbf61ed1fc45933cfd86951a744b681000eaaa75da31f17a"},
]
23 changes: 23 additions & 0 deletions tests/installation/fixtures/with-reusable-package-info-git.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[[package]]
name = "demo"
version = "0.1.0"
description = ""
category = "main"
optional = false
python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*"

[package.source]
type = "git"
reference = "f64843797eb954d530b6bfd229bfd6a55d13fe68"
url = "https://github.com/demo/demo.git"

[package.dependencies]

[package.extras]

[metadata.files]
demo = []

type = "url"
reference = ""
url = "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
19 changes: 19 additions & 0 deletions tests/installation/fixtures/with-reusable-package-info-url.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[[package]]
name = "demo"
version = "0.1.0"
description = ""
category = "main"
optional = false
python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*"

[package.source]
type = "url"
reference = ""
url = "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"

[package.dependencies]

[package.extras]

[metadata.files]
demo = []
65 changes: 65 additions & 0 deletions tests/installation/test_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from poetry.installation.executor import Executor as BaseExecutor
from poetry.installation.noop_installer import NoopInstaller
from poetry.packages import Locker as BaseLocker
from poetry.puzzle.provider import Provider
from poetry.repositories import Pool
from poetry.repositories import Repository
from poetry.repositories.installed_repository import InstalledRepository
Expand Down Expand Up @@ -1757,3 +1758,67 @@ def test_installer_can_handle_old_lock_files(

# colorama will be added
assert 8 == installer.executor.installations_count


def installer_add_with_existing_demo_package(
package, locker, repo, installer, lock_data, demo_constraint
):
locker.locked(True)
locker.mock_lock_data(lock_data)

repo.add_package(get_package("A", "1.0"))

package.add_dependency("demo", demo_constraint)
package.add_dependency("A", "^1.0")

installer.update(True)
installer.run()


def test_installer_reuse_lock_data_file(mocker, installer, locker, repo, package):
spy = mocker.spy(Provider, "get_package_from_file")
data = fixture("with-reusable-package-info-git")
constraint = {
"file": str(fixtures_dir / "distributions/demo-0.1.0-py2.py3-none-any.whl")
}
installer_add_with_existing_demo_package(
package=package,
locker=locker,
repo=repo,
installer=installer,
lock_data=data,
demo_constraint=constraint,
)
spy.assert_not_called()


def test_installer_reuse_lock_data_git(mocker, installer, locker, repo, package):
spy = mocker.spy(Provider, "get_package_from_vcs")
data = fixture("with-reusable-package-info-git")
constraint = {"git": "https://github.com/demo/demo.git"}
installer_add_with_existing_demo_package(
package=package,
locker=locker,
repo=repo,
installer=installer,
lock_data=data,
demo_constraint=constraint,
)
spy.assert_not_called()


def test_installer_reuse_lock_data_url(mocker, installer, locker, repo, package):
spy = mocker.spy(Provider, "get_package_from_url")
data = fixture("with-reusable-package-info-url")
constraint = {
"url": "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
}
installer_add_with_existing_demo_package(
package=package,
locker=locker,
repo=repo,
installer=installer,
lock_data=data,
demo_constraint=constraint,
)
spy.assert_not_called()