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 and improve handling of extras while resolving dependencies #2887

Merged
merged 1 commit into from
Sep 18, 2020

Conversation

sdispater
Copy link
Member

@sdispater sdispater commented Sep 6, 2020

Pull Request Check List

Resolves: #2545
Resolves: #2494
Resolves: #2300
Resolves: #2080
Resolves: #1609
Resolves: #1616

This one was bothering for a while and was quite the struggle to fix.

Basically, extras could trip up Poetry if the package already existed in the dependency graph leading to dependencies being ignore or uninstalled when updating dependencies.

To make it work, a rather big refactoring was required (see python-poetry/poetry-core#78) to make dependencies and packages comparison more accurate.

The main trick here is that a package with extras will now have its non-extra version as a dependency to help the resolver figure out what needs to be done.

Overall, it simplifies the code by always adding all dependencies of packages (even optional ones) and it's the Provider that will now decide which ones should be exposed to the resolver based on the extras selected.

One last thing of note is that source urls of file and directory dependencies will now be the fully resolved path, i.e. absolute, to actually be able to make comparisons.

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

@sdispater sdispater added the area/solver Related to the dependency resolver label Sep 6, 2020
@sdispater sdispater force-pushed the fix-transitive-extras branch 10 times, most recently from f9cafca to c08e155 Compare September 7, 2020 20:18
@sdispater sdispater marked this pull request as ready for review September 8, 2020 14:43
@sdispater sdispater requested a review from a team September 8, 2020 14:43
@sdispater sdispater force-pushed the fix-transitive-extras branch 4 times, most recently from 9ad2e5d to d38c901 Compare September 16, 2020 19:31
@sdispater sdispater force-pushed the fix-transitive-extras branch from d38c901 to 4bb3c83 Compare September 18, 2020 12:35
Copy link
Member

@abn abn left a comment

Choose a reason for hiding this comment

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

Looking good. Some minor fixes.

poetry/console/commands/show.py Show resolved Hide resolved
poetry/factory.py Outdated Show resolved Hide resolved
poetry/packages/locker.py Outdated Show resolved Hide resolved
poetry/packages/locker.py Outdated Show resolved Hide resolved
poetry/packages/locker.py Outdated Show resolved Hide resolved
tests/utils/test_extras.py Outdated Show resolved Hide resolved
tests/mixology/helpers.py Outdated Show resolved Hide resolved
poetry/version/version_selector.py Outdated Show resolved Hide resolved
poetry/console/commands/debug/resolve.py Outdated Show resolved Hide resolved
poetry/console/commands/debug/resolve.py Outdated Show resolved Hide resolved
@sdispater sdispater force-pushed the fix-transitive-extras branch 3 times, most recently from 2639c45 to c94d2a6 Compare September 18, 2020 15:12
abn
abn previously approved these changes Sep 18, 2020
@sdispater sdispater merged commit 5acf8fa into master Sep 18, 2020
@sdispater sdispater deleted the fix-transitive-extras branch September 18, 2020 15:42
@sdispater sdispater mentioned this pull request Sep 18, 2020
Copy link

github-actions bot commented Mar 1, 2024

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 Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.