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

Fix misuse of pretty_constraint, fix downstream test (poetry-core) #4932

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
10 changes: 5 additions & 5 deletions src/poetry/packages/locker.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,16 +236,16 @@ def __walk_dependency_level(
if locked_package:
# create dependency from locked package to retain dependency metadata
# if this is not done, we can end-up with incorrect nested dependencies
constraint = requirement.constraint
pretty_constraint = requirement.pretty_constraint
marker = requirement.marker
requirement = locked_package.to_dependency()
requirement.marker = requirement.marker.intersect(marker)

key = (requirement.name, requirement.pretty_constraint)
key = (requirement.name, pretty_constraint)

if pinned_versions:
requirement.set_constraint(
locked_package.to_dependency().constraint
)
if not pinned_versions:
requirement.set_constraint(constraint)

for require in locked_package.requires:
if require.marker.is_empty():
Expand Down
19 changes: 14 additions & 5 deletions src/poetry/utils/exporter.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import itertools
import urllib.parse

from typing import TYPE_CHECKING
Expand Down Expand Up @@ -69,13 +70,21 @@ def _export_requirements_txt(
content = ""
dependency_lines = set()

for dependency_package in self._poetry.locker.get_project_dependency_packages(
project_requires=self._poetry.package.all_requires, dev=dev, extras=extras
for package, groups in itertools.groupby(
Copy link
Member

Choose a reason for hiding this comment

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

Can you walk me through this set of changes and why they are needed? I can follow the line-by-line, but I'm not sure I'm groking the bigger picture.

Also, this file is going to be removed in favor of the exporter plugin. Can you please make a PR against that repo as well?

Copy link
Member Author

@radoering radoering Jan 19, 2022

Choose a reason for hiding this comment

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

If you are wondering: I accidentally deleted my branch while porting the fix to the exporter plugin. Sorry about that. 😅

The changes in exporter.py are required because get_project_dependency_packages returns objects of type DependencyPackage, which consist of a Dependency and a Package. There can be multiple dependency packages containing the same package for different dependencies, e.g. package a(1.1) for dependency a(>=1.0) with markers sys_platform == "win32" and package a(1.1) for dependency a(>=1.1) with markers sys_platform == "linux". The changes are required in order to receive only one entry for a==1.1 in requirements.txt instead of two entries with different markers.

This behaviour can be observed in the following test cases:

test_exporter_can_export_requirements_txt_with_nested_packages_and_multiple_markers

expected:

bar==7.8.9 ; platform_system != "Windows" or platform_system == "Windows"

without changes:

bar==7.8.9 ; platform_system != "Windows"
bar==7.8.9 ; platform_system == "Windows"

test_exporter_can_export_requirements_txt_with_nested_packages_and_markers_any

expected:

a==1.2.3

without changes:

a==1.2.3
a==1.2.3 ; python_version < "3.8"

test_exporter_can_export_requirements_txt_pyinstaller

expected:

altgraph==0.17

without changes:

altgraph==0.17
altgraph==0.17 ; sys_platform == "darwin"

self._poetry.locker.get_project_dependency_packages(
project_requires=self._poetry.package.all_requires,
dev=dev,
extras=extras,
),
lambda dependency_package: dependency_package.package,
):
line = ""

dependency = dependency_package.dependency
package = dependency_package.package
dependency_packages = list(groups)
dependency = dependency_packages[0].dependency
marker = dependency.marker
for dep_package in dependency_packages[1:]:
marker = marker.union(dep_package.dependency.marker)
dependency.marker = marker

if package.develop:
line += "-e "
Expand Down