build(cmake): refactor Qt version compatibility #600
Merged
Conversation
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
- Add Qt auto-detection using find_package(QT NAMES Qt6 Qt5) - Separate debian/control files: * control: V25 (Qt6) with explicit qt6-base-dev, libdtk6*-dev deps * control.1: V20 (Qt5) with explicit qtbase5-dev, libdtk*-dev deps - Removes legacy macro-based Qt version handling and implements dynamic package name resolution for Qt, DTK, PolkitQt and QApt dependencies. This enables seamless building on both V25 (Qt6) and V20 (Qt5) systems without version-specific conditional dependencies. Log: 分离Qt5/Qt6构建配置，支持V25/V20双版本 PMS: https://pms.uniontech.com/task-view-386321.html
deepin pr auto review这份代码diff主要涉及将项目从Qt5/Qt6双版本支持迁移到仅支持Qt6，并优化了CMake构建系统的版本检测逻辑。以下是对语法逻辑、代码质量、代码性能和代码安全的详细审查意见： 1. 语法逻辑审查优点：
潜在问题：
2. 代码质量审查优点：
改进建议：
3. 代码性能审查优点：
改进建议：
4. 代码安全审查优点：
潜在问题：
5. 其他建议
总结这份代码diff整体质量较高，成功优化了CMake构建系统的版本检测逻辑，并统一了依赖管理。主要改进点包括：
建议的改进方向：
如果确认项目完全迁移到Qt6，这些修改是合理的，且不会引入明显的安全或性能问题。 |
Reviewer's GuideRefactors CMake-based Qt/DTK integration to rely on automatic Qt version detection and dynamic package naming for Qt, DTK, PolkitQt and QApt, and splits Debian packaging metadata into separate Qt5/Qt6 control files to support V20 (Qt5) and V25 (Qt6) builds without manual conditionals. 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 - I've left some high level feedback:
- The new DTK version mapping in the top-level CMakeLists.txt sets
DTK_VERSION_MAJORto an empty string for Qt5, but the sub-CMakeLists now callfind_package(Dtk${DTK_VERSION_MAJOR} ...)(e.g.find_package(Dtk${DTK_VERSION_MAJOR} COMPONENTS Widget Core REQUIRED)), which will resolve tofind_package(Dtk ...)instead of the previousDtkCore/DtkWidgetpackages and leaves the Qt5 branches still using${DtkCore_LIBRARIES}/${DtkWidget_LIBRARIES}without correspondingfind_packagecalls; this likely breaks Qt5 builds and should be aligned (either keep using the old Dtk* packages for Qt5 or introduce a consistent DTK5 package pattern). - In
deepin-devicemanager-server/deepin-deviceinfo/CMakeLists.txt, the Qt6 branch now links${Dtk6Core_LIBRARIES}, butfind_package(Dtk${DTK_VERSION_MAJOR} COMPONENTS Widget Core REQUIRED)typically exposes imported targetsDtk6::Core/Dtk6::Widgetrather thanDtk6Core_LIBRARIES; consider switching to the imported targets consistently or ensuring that the expected *_LIBRARIES variables are actually defined by the DTK find module.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new DTK version mapping in the top-level CMakeLists.txt sets `DTK_VERSION_MAJOR` to an empty string for Qt5, but the sub-CMakeLists now call `find_package(Dtk${DTK_VERSION_MAJOR} ...)` (e.g. `find_package(Dtk${DTK_VERSION_MAJOR} COMPONENTS Widget Core REQUIRED)`), which will resolve to `find_package(Dtk ...)` instead of the previous `DtkCore` / `DtkWidget` packages and leaves the Qt5 branches still using `${DtkCore_LIBRARIES}` / `${DtkWidget_LIBRARIES}` without corresponding `find_package` calls; this likely breaks Qt5 builds and should be aligned (either keep using the old Dtk* packages for Qt5 or introduce a consistent DTK5 package pattern).
- In `deepin-devicemanager-server/deepin-deviceinfo/CMakeLists.txt`, the Qt6 branch now links `${Dtk6Core_LIBRARIES}`, but `find_package(Dtk${DTK_VERSION_MAJOR} COMPONENTS Widget Core REQUIRED)` typically exposes imported targets `Dtk6::Core`/`Dtk6::Widget` rather than `Dtk6Core_LIBRARIES`; consider switching to the imported targets consistently or ensuring that the expected *_LIBRARIES variables are actually defined by the DTK find module.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
lzwind
approved these changes
Jan 27, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, re2zero 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
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
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.
This enables seamless building on both V25 (Qt6) and V20 (Qt5) systems without version-specific conditional dependencies.
Log: 分离Qt5/Qt6构建配置，支持V25/V20双版本
PMS: https://pms.uniontech.com/task-view-386321.html
Summary by Sourcery
Unify CMake-based Qt/DTK configuration to auto-detect the Qt major version and dynamically resolve related dependency packages, and adjust Debian packaging to support separate Qt5 and Qt6 build configurations.
Enhancements:
Build: