-
Notifications
You must be signed in to change notification settings - Fork 34
build(cmake): refactor Qt version compatibility #190
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
Conversation
- 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 - remove Qt5 fallback build dependencies - adjust Qt private include paths to use modern Qt6 syntax - add conditional Qt6 WidgetsPrivate linking - refactor DTK version variable usage throughout build system - maintain NO_DBUS_CALLER_AUTH_CHECK for Qt5 builds 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
Reviewer's GuideRefactors CMake and Debian packaging to auto-detect Qt (Qt6/Qt5), align DTK version handling, modernize Qt private include and WidgetsPrivate usage, and split Debian control files so V25 (Qt6) and V20 (Qt5) builds use appropriate dependencies. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这份代码变更主要是将项目从 Qt5/Qt6 混合支持迁移到以 Qt6 为主,并统一了 CMake 的版本要求和构建逻辑。以下是对该 1. 语法逻辑审查
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
5. 构建系统与依赖审查
总结与改进建议
总体而言,这次变更将构建系统向现代 CMake 标准和 Qt6 迈进了重要的一步,修复了明显的 Bug(如 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Using
Qt${QT_VERSION_MAJOR}::GuiPrivateinsidetarget_include_directories(e.g. in application, service, basestruct, log, tests) is not the intended pattern for Qt’s private modules; instead, you should link against theGuiPrivatetarget viatarget_link_librariesand rely on its usage requirements to propagate include directories. - The new
DTK_VERSION_MAJORmapping leaves it empty for Qt5 (QT_VERSION_MAJOR MATCHES 6→ 6, else empty), which changes the generated package/target names toDtkWidget,DtkGui, etc.; please double-check this matches the actual DTK target and package naming for Qt5, or make the Qt5/Qt6 DTK mapping explicit to avoid silent mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `Qt${QT_VERSION_MAJOR}::GuiPrivate` inside `target_include_directories` (e.g. in application, service, basestruct, log, tests) is not the intended pattern for Qt’s private modules; instead, you should link against the `GuiPrivate` target via `target_link_libraries` and rely on its usage requirements to propagate include directories.
- The new `DTK_VERSION_MAJOR` mapping leaves it empty for Qt5 (`QT_VERSION_MAJOR MATCHES 6` → 6, else empty), which changes the generated package/target names to `DtkWidget`, `DtkGui`, etc.; please double-check this matches the actual DTK target and package naming for Qt5, or make the Qt5/Qt6 DTK mapping explicit to avoid silent mismatches.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[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 |
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
Refactor the CMake-based build to support both Qt5 and Qt6/DTK6 via auto-detection, modern Qt private include usage, and updated Debian packaging for separate Qt5/Qt6 build configurations.
Build:
Tests:
Chores: