cherry-pick 2 commits of release/eagle#353
Closed
LiHua000 wants to merge 2 commits into
Closed
Conversation
log: 修复因冲突包无法安装的问题
…inuxdeepin#318) 1. Version check for package removal: - Add version comparison logic in setMarkedPackages method - Only add packages to removal list when version conditions are met 2. Improve dependency handling: - Fix incorrect handling of Breaks/Conflicts relationships - Ensure proper version comparison for all relation types Log: Fix incorrect "Will remove" warning when installing packages with Breaks/Conflicts dependencies where installed version is higher than specified version. Bug: https://pms.uniontech.com/bug-view-311145.html
Reviewer's GuideCherry-pick of two commits that correct package removal logic for Breaks/Conflicts dependencies and add null/empty value checks by extracting version-based removal logic into a helper and ensuring empty conflict versions are treated appropriately. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来审查这段代码，并提出一些改进意见：
总体来说，这段代码的改进提高了版本检查的准确性和健壮性，但在性能优化和错误处理方面还有提升空间。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/deb-installer/utils/deb_package.cpp:29` </location>
<code_context>
+ const int result = QApt::Package::compareVersion(installed_version, conflict_version);
+
+ // 根据关系类型判断是否需要移除
+ switch (type) {
+ case QApt::LessOrEqual:
+ return result <= 0;
+ case QApt::GreaterOrEqual:
+ return result >= 0;
+ case QApt::LessThan:
+ return result < 0;
+ case QApt::GreaterThan:
+ return result > 0;
+ case QApt::Equals:
+ return result == 0;
+ case QApt::NotEqual:
+ return result != 0;
+ default:
+ return true;
+ }
</code_context>
<issue_to_address>
Default case in switch returns true, which may mask unexpected relation types.
Returning true for unknown relation types may cause unintended package removals. Please handle unexpected types explicitly or add logging to prevent silent errors.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
switch (type) {
case QApt::LessOrEqual:
return result <= 0;
case QApt::GreaterOrEqual:
return result >= 0;
case QApt::LessThan:
return result < 0;
case QApt::GreaterThan:
return result > 0;
case QApt::Equals:
return result == 0;
case QApt::NotEqual:
return result != 0;
default:
return true;
}
=======
switch (type) {
case QApt::LessOrEqual:
return result <= 0;
case QApt::GreaterOrEqual:
return result >= 0;
case QApt::LessThan:
return result < 0;
case QApt::GreaterThan:
return result > 0;
case QApt::Equals:
return result == 0;
case QApt::NotEqual:
return result != 0;
default:
qWarning() << "Unknown relation type in deb_package.cpp:" << type << ". No removal performed.";
return false;
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+29
to
+44
| switch (type) { | ||
| case QApt::LessOrEqual: | ||
| return result <= 0; | ||
| case QApt::GreaterOrEqual: | ||
| return result >= 0; | ||
| case QApt::LessThan: | ||
| return result < 0; | ||
| case QApt::GreaterThan: | ||
| return result > 0; | ||
| case QApt::Equals: | ||
| return result == 0; | ||
| case QApt::NotEqual: | ||
| return result != 0; | ||
| default: | ||
| return true; | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Default case in switch returns true, which may mask unexpected relation types.
Returning true for unknown relation types may cause unintended package removals. Please handle unexpected types explicitly or add logging to prevent silent errors.
Suggested change
| switch (type) { | |
| case QApt::LessOrEqual: | |
| return result <= 0; | |
| case QApt::GreaterOrEqual: | |
| return result >= 0; | |
| case QApt::LessThan: | |
| return result < 0; | |
| case QApt::GreaterThan: | |
| return result > 0; | |
| case QApt::Equals: | |
| return result == 0; | |
| case QApt::NotEqual: | |
| return result != 0; | |
| default: | |
| return true; | |
| } | |
| switch (type) { | |
| case QApt::LessOrEqual: | |
| return result <= 0; | |
| case QApt::GreaterOrEqual: | |
| return result >= 0; | |
| case QApt::LessThan: | |
| return result < 0; | |
| case QApt::GreaterThan: | |
| return result > 0; | |
| case QApt::Equals: | |
| return result == 0; | |
| case QApt::NotEqual: | |
| return result != 0; | |
| default: | |
| qWarning() << "Unknown relation type in deb_package.cpp:" << type << ". No removal performed."; | |
| return false; | |
| } |
lzwind
approved these changes
Sep 12, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
fix: correct package removal logic for Breaks/Conflicts dependencies (#318)
fix: 空值判断 (#348)
Summary by Sourcery
Fix package removal and conflict satisfaction logic by properly handling empty version constraints and centralising version-based comparisons
Bug Fixes:
Enhancements: