Skip to content

feat: Issues of compile on v20 Qt5.#439

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
lichaofan2008:master
Dec 18, 2025
Merged

feat: Issues of compile on v20 Qt5.#439
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
lichaofan2008:master

Conversation

@lichaofan2008
Copy link
Contributor

@lichaofan2008 lichaofan2008 commented Dec 18, 2025

适配v20系统,完成Qt5环境的编译、打包流程。
Adapted to the v20 system, issues of compile on Qt5.

v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html

Summary by Sourcery

Add support for building and packaging the application with either Qt6 or Qt5, improving compatibility with the v20 system.

New Features:

  • Enable conditional configuration to build the project with Qt5 when Qt6 is not available.

Bug Fixes:

  • Fix current camera device name tracking by using the selected device name instead of the device description.

Enhancements:

  • Adjust Qt module selection, include paths, translation generation, and link libraries based on whether Qt6 or Qt5 is used.
  • Simplify debug logging statements to avoid usage incompatible with Qt5.

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 18, 2025

Reviewer's Guide

Adds conditional Qt5/Qt6 build support for deepin-camera on v20 by detecting Qt6 first and falling back to Qt5, adjusting modules, include paths, translation generation, and link libraries accordingly, and makes small runtime/logging fixes for camera selection and debug output.

Updated class diagram for Camera and CMainWindow logging and device handling

classDiagram
    class Camera {
        QString m_curDevName
        void startCamera(QString devName)
    }

    class CMainWindow {
        void onSwitchCameraSuccess(QString cameraName)
        void onTimeoutLock(QString serviceName, QVariantMap key2value, QStringList argList)
    }
Loading

File-Level Changes

Change Details Files
Make the CMake project buildable with either Qt6 or Qt5 by detecting Qt6 and conditionally configuring Qt modules, includes, translations, and link libraries.
  • Use find_package(Qt6 QUIET) and set QT_VERSION_MAJOR to 6 only if Qt6 is found, otherwise fall back to 5
  • Define QtModule differently depending on whether Qt6 is found, omitting SvgWidgets and OpenGLWidgets for Qt5
  • Adjust PROJECT_INCLUDE to use system include paths suitable for Qt5 builds on v20 while preserving the previous paths for Qt6
  • Choose between qt_create_translation and qt5_create_translation based on whether Qt6 is found
  • Split target_link_libraries into Qt6 and Qt5 branches, adding/removing module targets like SvgWidgets, OpenGLWidgets, and Concurrent appropriately
src/CMakeLists.txt
Align debug logging with Qt5 compatibility by removing usage of Qt::endl and ensure correct camera device name tracking.
  • Remove Qt::endl from multiple qDebug() statements so they compile and behave consistently across Qt versions
  • Change Camera::startCamera to store the current device name from the devName argument instead of device.description() to avoid mismatches between selected and reported camera
src/src/mainwindow.cpp
src/src/camera.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The Qt5 vs Qt6 handling in CMake duplicates quite a bit of logic (especially in QtModule and target_link_libraries); consider factoring out the common parts and only branching on the modules/targets that actually differ to keep the build configuration easier to maintain.
  • In the Qt5 branch you now hardcode system include paths (/usr/include, /usr/include/libusb-1.0, etc.); if this needs to work in non-default sysroots or cross‑compile environments, it would be more robust to derive these from pkg-config variables or CMake package discovery instead of fixed paths.
  • Changing m_curDevName from device.description() to devName in Camera::startCamera alters what is stored and potentially displayed as the current camera name; if UI or settings expect a human‑readable description, it may be safer to keep using the description or clearly separate the internal device ID from the user‑visible label.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The Qt5 vs Qt6 handling in CMake duplicates quite a bit of logic (especially in QtModule and target_link_libraries); consider factoring out the common parts and only branching on the modules/targets that actually differ to keep the build configuration easier to maintain.
- In the Qt5 branch you now hardcode system include paths (/usr/include, /usr/include/libusb-1.0, etc.); if this needs to work in non-default sysroots or cross‑compile environments, it would be more robust to derive these from pkg-config variables or CMake package discovery instead of fixed paths.
- Changing m_curDevName from device.description() to devName in Camera::startCamera alters what is stored and potentially displayed as the current camera name; if UI or settings expect a human‑readable description, it may be safer to keep using the description or clearly separate the internal device ID from the user‑visible label.

## Individual Comments

### Comment 1
<location> `src/src/mainwindow.cpp:715` </location>
<code_context>
                                             tem_fontmetrics.height(), pathEditWidth);
                         videoPathLineEdit->setText(pi);
-                        qDebug() << "picPathLineEdit text:" << videoPathLineEdit->text() << Qt::endl;
+                        qDebug() << "picPathLineEdit text:" << videoPathLineEdit->text();
                         lastVideoPath = value.toString();
                         option->setValue(value.toString());
</code_context>

<issue_to_address>
**suggestion:** Log label refers to `picPathLineEdit` while actually logging `videoPathLineEdit`.

The log label still says `"picPathLineEdit text:"` while outputting `videoPathLineEdit->text()`, which can be misleading when debugging. Please update the label to match `videoPathLineEdit`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

适配v20系统,完成Qt5环境的编译、打包流程。
Adapted to the v20 system, issues of compile on Qt5.

v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来帮你审查这些代码变更。主要从以下几个方面进行分析:

  1. debian/control 文件变更:
  • 优点:

    • 依赖项列表格式化更清晰,每行一个包
    • 添加了 Qt5 和 Qt6 的兼容性支持
    • 对 DTK 相关依赖提供了版本选择
  • 改进建议:

    • 可以考虑对相似依赖进行分组,比如将 Qt 相关依赖放在一起
    • 建议在注释中说明 Qt5 和 Qt6 版本的兼容性策略
  1. CMakeLists.txt 文件变更:
  • 优点:

    • 添加了 Qt5/Qt6 的自动检测和兼容性支持
    • 根据不同 Qt 版本动态设置模块和链接库
    • 代码结构清晰,条件判断合理
  • 改进建议:

    • 可以考虑将 Qt 版本检测封装为一个函数
    • 建议在设置 QtModule 时使用更模块化的方式,避免重复代码
    • 可以添加对 Qt 版本的最低要求检查
  1. camera.cpp 文件变更:
  • 优点:
    • 修复了设备名称设置的问题,直接使用传入的 devName 而不是 device.description()
  1. mainwindow.cpp 文件变更:
  • 优点:

    • 移除了 qDebug 中多余的 Qt::endl,因为 qDebug 默认会换行
  • 改进建议:

    • 可以考虑使用 qCDebug 替代 qDebug,以便更好地控制调试输出
    • 建议统一调试信息的格式和风格

总体建议:

  1. 版本兼容性:

    • 考虑创建一个版本兼容性配置文件,集中管理 Qt5/Qt6 的差异
    • 建议添加对 Qt 版本的详细文档说明
  2. 代码维护性:

    • 考虑将版本相关的配置提取到单独的配置文件中
    • 建议添加更多的注释说明版本兼容性的处理逻辑
  3. 安全性:

    • 建议对依赖包的版本进行更严格的控制
    • 考虑添加对 Qt 版本的完整性检查
  4. 性能:

    • 当前的版本检测逻辑是合理的,没有明显的性能问题
    • 可以考虑缓存 Qt 版本检测结果,避免重复检测

这些变更整体上是积极正面的,主要是为了支持 Qt5 和 Qt6 的兼容性,实现方式也比较合理。建议在后续维护中注意文档更新和代码注释的完善。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lichaofan2008
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 7deea1f into linuxdeepin:master Dec 18, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants