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

Support for multiple-constraint direct origin dependencies with same version #5715

Merged
merged 4 commits into from
May 30, 2022

Conversation

radoering
Copy link
Member

Resolves: #4810
Resolves: #5447
Resolves: #5714

  • Added tests for changed code.
  • Updated documentation for changed code.

Prior to #5640 dependencies with same version from different sources were merged, which led to incorrect lock files. However, according to #4810 at least in some cases the installer was able to mitigate the incorrect lock files by installing a suitable wheel anyway.

With #5640 instead of generating incomplete lock files a solver error is raised. Although, in terms of correctness that's an improvement, it could also be considered a minor regression by some users. This PR, improves the handling of multiple-constraint direct origin (path, url, git) dependencies fundamentally.

The first commit of this PR enables the provider/solver to handle multiple-constraint direct origin dependencies correctly by considering them as duplicates and not merging them by version anyway. However, that is not enough to create correct lock files because the installer discards direct origin dependencies with same name and version as well.

In order to enable the installer to handle this kind of dependencies a new Repository type, which can distinguish between multiple direct origin dependencies with the same version, is introduced.

Further, a consistent order of direct origin dependencies in the lock file is ensured.

src/poetry/puzzle/provider.py Outdated Show resolved Hide resolved
src/poetry/puzzle/provider.py Outdated Show resolved Hide resolved
src/poetry/installation/installer.py Outdated Show resolved Hide resolved
Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

LGTM, but one last question -- does the base Repository class have any direct instances anymore, or could we migrate it to be an abstract base class?

@radoering
Copy link
Member Author

The base class still has several (more than 10) direct instances.

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants