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

installer: use locked version of vcs dependency with extras instead of latest #6185

Merged
Merged
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
18 changes: 8 additions & 10 deletions src/poetry/mixology/version_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,11 +418,6 @@ def _get_min(dependency: Dependency) -> tuple[bool, int]:
return complete_name

package = None
if dependency.name not in self._use_latest:
# prefer locked version of compatible (not exact same) dependency;
# required in order to not unnecessarily update dependencies with
# extras, e.g. "coverage" vs. "coverage[toml]"
locked = self._get_locked(dependency, allow_similar=True)
if locked is not None:
package = next(
(
Expand Down Expand Up @@ -504,18 +499,21 @@ def _add_incompatibility(self, incompatibility: Incompatibility) -> None:
incompatibility
)

def _get_locked(
self, dependency: Dependency, *, allow_similar: bool = False
) -> DependencyPackage | None:
def _get_locked(self, dependency: Dependency) -> DependencyPackage | None:
if dependency.name in self._use_latest:
return None

locked = self._locked.get(dependency.name, [])
for dependency_package in locked:
package = dependency_package.package
if (
allow_similar or dependency.is_same_package_as(package)
) and dependency.constraint.allows(package.version):
# Locked dependencies are always without features.
# Thus, we can't use is_same_package_as() here because it compares
# the complete_name (including features).
dependency.name == package.name
and dependency.is_same_source_as(package)
and dependency.constraint.allows(package.version)
):
return DependencyPackage(dependency, package)
return None

Expand Down
47 changes: 47 additions & 0 deletions tests/installation/fixtures/with-vcs-dependency-with-extras.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
[[package]]
name = "demo"
version = "0.1.2"
description = ""
category = "main"
optional = false
python-versions = "*"
develop = false

[package.dependencies]
cleo = {version = "*", optional = true, markers = "extra == \"foo\""}
pendulum = ">=1.4.4"

[package.extras]
foo = ["cleo"]

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

[[package]]
name = "pendulum"
version = "1.4.4"
description = ""
category = "main"
optional = false
python-versions = "*"

[[package]]
name = "cleo"
version = "1.0.0"
description = ""
category = "main"
optional = false
python-versions = "*"

[metadata]
python-versions = "*"
lock-version = "1.1"
content-hash = "123456789"

[metadata.files]
demo = []
pendulum = []
cleo = []
42 changes: 42 additions & 0 deletions tests/installation/test_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2473,3 +2473,45 @@ def test_installer_should_use_the_locked_version_of_git_dependencies(
source_reference="master",
source_resolved_reference="123456",
)


@pytest.mark.parametrize("is_locked", [False, True])
def test_installer_should_use_the_locked_version_of_git_dependencies_with_extras(
installer: Installer,
locker: Locker,
package: ProjectPackage,
repo: Repository,
is_locked: bool,
):
if is_locked:
locker.locked(True)
locker.mock_lock_data(fixture("with-vcs-dependency-with-extras"))
expected_reference = "123456"
else:
expected_reference = "9cf87a285a2d3fbb0b9fa621997b3acc3631ed24"
Copy link
Member

Choose a reason for hiding this comment

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

nit: Given our growing usage of this value in tests, a follow-up PR that defines it in a reusable constant may make it clearer where it comes from (I'd be happy to do the change).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good.


package.add_dependency(
Factory.create_dependency(
"demo",
{
"git": "https://github.com/demo/demo.git",
"branch": "master",
"extras": ["foo"],
},
)
)

repo.add_package(get_package("pendulum", "1.4.4"))
repo.add_package(get_package("cleo", "1.0.0"))

installer.run()

assert len(installer.executor.installations) == 3
assert installer.executor.installations[-1] == Package(
"demo",
"0.1.2",
source_type="git",
source_url="https://github.com/demo/demo.git",
source_reference="master",
source_resolved_reference=expected_reference,
)