fix: update, add and remove with --only shall not uninstall dependencies from all other groups#10014
Merged
abn merged 1 commit intopython-poetry:mainfrom Jan 11, 2025
Conversation
…ependencies from all other groups Reintroduce some logic removed in bd3500d and add tests for it. Fix logic with `reresolve = False`, which had the same issue. - The change in installer is the fix for `reresolve = True` (reintroducing old logic). - The change in transaction is the fix for `reresolve = False`.
Reviewer's Guide by SourceryThis pull request fixes a bug where using the Sequence diagram for package installation with --only flagsequenceDiagram
participant User
participant Installer
participant Transaction
participant Repository
User->>Installer: Install with --only flag
Installer->>Transaction: calculate_operations(with_uninstalls)
alt reresolve=True
Transaction->>Repository: Get locked packages
Transaction->>Repository: Get lockfile packages
Transaction->>Transaction: Calculate uninstall ops
Transaction->>Transaction: Calculate install ops
Transaction-->>Installer: Return combined ops
else reresolve=False
Transaction->>Transaction: Check result packages
Transaction->>Transaction: Calculate ops for specified groups
Transaction-->>Installer: Return filtered ops
end
Installer-->>User: Execute operations
Class diagram for Transaction and Installer changesclassDiagram
class Transaction {
-_result_packages: List
-_current_packages: List
-_installed_packages: List
+calculate_operations(with_uninstalls: bool): List
}
class Installer {
-_requires_synchronization: bool
-_update: bool
-_extras: List
+_do_install(): int
}
class Operation {
+job_type: str
}
Transaction ..> Operation
Installer ..> Transaction
note for Transaction "Modified uninstall logic to respect --only flag"
note for Installer "Updated operation calculation based on reresolve flag"
Flow diagram for dependency resolution logicflowchart TD
A[Start Installation] --> B{Reresolve?}
B -->|Yes| C[Calculate operations]
C --> D[Get uninstall operations]
D --> E[Combine with install operations]
B -->|No| F[Calculate operations with filtered uninstalls]
E --> G[Execute operations]
F --> G
G --> H[End Installation]
subgraph Uninstall Logic
D
F
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @radoering - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
2 tasks
abn
approved these changes
Jan 11, 2025
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reintroduce some logic removed in bd3500d and add tests for it. Fix logic with
reresolve = False, which had the same issue.reresolve = True(reintroducing old logic).reresolve = False.Pull Request Check List
Resolves: #9950
(Does not resolve the similar issue #9975. This one requires another fix.)
Summary by Sourcery
Restore the logic for the --only flag to prevent uninstalling dependencies from other groups, and add tests to cover this behavior.
Bug Fixes:
--onlyflag withupdate,add, andremovecommands would uninstall dependencies from all other groups.Tests:
--onlyflag behavior.