fix: [build]add Qt5/Qt6 compatibility and improve build system config…#114
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Dec 2, 2025
Merged
Conversation
Reviewer's GuideThis PR makes the project buildable with both Qt5 and Qt6 by centralizing Qt/DTK detection in the top-level CMake, conditionally using the correct Qt moc/resource macros throughout subdirectories, introducing a reusable translation generation CMake module, and adapting logging/IO code to API differences between Qt5 and Qt6, alongside updating Debian packaging to declare dual Qt build dependencies. Flow diagram for Qt version specific moc/resource handlingflowchart TB
Start["Configure subdirectory CMakeLists.txt"] --> CheckQt["Check QT_VERSION or QT_VERSION_MAJOR"]
CheckQt -->|"Qt >= 6.0"| Qt6Path
CheckQt -->|"Qt < 6.0"| Qt5Path
subgraph Qt6Path[Qt6 path]
Qt6MocUi["ui/CMakeLists: qt_wrap_cpp(UDLC_UI_HEADERS_MOC)"]
Qt6ResUi["ui/CMakeLists: qt_add_resources(QRC_FILES)"]
Qt6MocExt["extensionService/CMakeLists: qt_wrap_cpp(DWM_EXTENSIONSERVICE_HEADERS_MOC)"]
Qt6MocAria["aria2/CMakeLists: qt_wrap_cpp(DWM_ARIA_HEADERS_MOC)"]
Qt6MocDb["database/CMakeLists: qt_wrap_cpp(DWM_DATABASE_HEADERS_MOC)"]
end
subgraph Qt5Path[Qt5 path]
Qt5MocUi["ui/CMakeLists: qt5_wrap_cpp(UDLC_UI_HEADERS_MOC)"]
Qt5ResUi["ui/CMakeLists: qt5_add_resources(QRC_FILES)"]
Qt5MocExt["extensionService/CMakeLists: qt5_wrap_cpp(DWM_EXTENSIONSERVICE_HEADERS_MOC)"]
Qt5MocAria["aria2/CMakeLists: qt5_wrap_cpp(DWM_ARIA_HEADERS_MOC)"]
Qt5MocDb["database/CMakeLists: qt5_wrap_cpp(DWM_DATABASE_HEADERS_MOC)"]
end
Qt6Path --> End["Targets built with Qt6-compatible macros"]
Qt5Path --> 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 there - I've reviewed your changes - here's some feedback:
- The CMake conditionals like
if (QT_VERSION VERSION_GREATER_EQUAL "6.0")rely on aQT_VERSIONvariable that doesn't seem to be set anywhere; consider basing these checks onQT_VERSION_MAJOR(e.g.if (QT_VERSION_MAJOR GREATER_EQUAL 6)) or the Qt package's own version variables to avoid configuration-time errors. - The
ENDL/QT_ENDLmacros currently map to bareendlfor Qt < 6, which is astdmanipulator and may not be visible withoutstd::endlorusing std::endl; to keep behavior consistent and avoid namespace issues, either map both branches toQt::endlor explicitly usestd::endlwith the appropriate header. - In
translation-generate.cmake, the fallbackQT_LRELEASEpath/lib/qt${QT_VERSION_MAJOR}/bin/lreleaseis hard-coded and may not exist on many systems; it’d be more robust to usefind_program(with common search paths as hints) or make this a cache variable the packager can override.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CMake conditionals like `if (QT_VERSION VERSION_GREATER_EQUAL "6.0")` rely on a `QT_VERSION` variable that doesn't seem to be set anywhere; consider basing these checks on `QT_VERSION_MAJOR` (e.g. `if (QT_VERSION_MAJOR GREATER_EQUAL 6)`) or the Qt package's own version variables to avoid configuration-time errors.
- The `ENDL`/`QT_ENDL` macros currently map to bare `endl` for Qt < 6, which is a `std` manipulator and may not be visible without `std::endl` or `using std::endl`; to keep behavior consistent and avoid namespace issues, either map both branches to `Qt::endl` or explicitly use `std::endl` with the appropriate header.
- In `translation-generate.cmake`, the fallback `QT_LRELEASE` path `/lib/qt${QT_VERSION_MAJOR}/bin/lrelease` is hard-coded and may not exist on many systems; it’d be more robust to use `find_program` (with common search paths as hints) or make this a cache variable the packager can override.
## Individual Comments
### Comment 1
<location> `cmake/translation-generate.cmake:4-5` </location>
<code_context>
+function(TRANSLATION_GENERATE QMS)
+ find_package(Qt${QT_VERSION_MAJOR}LinguistTools QUIET)
+
+ if (NOT Qt${QT_VERSION_MAJOR}_LRELEASE_EXECUTABLE)
+ set(QT_LRELEASE "/lib/qt${QT_VERSION_MAJOR}/bin/lrelease")
+ message(STATUS "NOT found lrelease, set QT_LRELEASE = ${QT_LRELEASE}")
+ else()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Hard-coding a fallback lrelease path is brittle; prefer a search or an explicit failure.
This fallback to `/lib/qt${QT_VERSION_MAJOR}/bin/lrelease` will fail on many setups (non-standard prefixes, Windows, custom Qt installs) and may do so silently. Instead, consider using `find_program(QT_LRELEASE lrelease)` (optionally with hints) and, if still not found, emit a `FATAL_ERROR` so configuration fails clearly rather than producing missing or stale translations.
Suggested implementation:
```
function(TRANSLATION_GENERATE QMS)
find_package(Qt${QT_VERSION_MAJOR}LinguistTools QUIET)
if (Qt${QT_VERSION_MAJOR}_LRELEASE_EXECUTABLE)
# Prefer the lrelease executable provided by the found Qt package
set(QT_LRELEASE "${Qt${QT_VERSION_MAJOR}_LRELEASE_EXECUTABLE}")
else()
# Fallback: search lrelease on PATH (optionally extended by the caller)
find_program(QT_LRELEASE NAMES lrelease)
if (NOT QT_LRELEASE)
message(FATAL_ERROR
"Could not find Qt lrelease executable. "
"Ensure Qt Linguist tools are installed and lrelease is available on PATH, "
"or provide Qt${QT_VERSION_MAJOR}_LRELEASE_EXECUTABLE via find_package(Qt${QT_VERSION_MAJOR}LinguistTools)."
)
else()
message(STATUS "Using lrelease found at: ${QT_LRELEASE}")
endif()
endif()
```
```
if(NOT ARGN)
message(SEND_ERROR "Error: TRANSLATION_GENERATE() called without any .ts path")
return()
endif()
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…uration - Update CMakeLists.txt to dynamically detect Qt version and use appropriate DTK versions - Modify debian/control to support both Qt5 and Qt6 build dependencies Log: Fix Qt5/Qt6 compatibility building issue.
Contributor
deepin pr auto review这是一个包含多个文件修改的 Git diff，主要涉及构建系统和 Qt 版本兼容性的更改。让我逐一分析主要修改：
建议改进：
# 在 CMakeLists.txt 中添加
if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE Release)
endif()
# 添加编译器特定优化
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG")
endif()
# 在 translation-generate.cmake 中添加检查
if(NOT EXISTS "${ARGN}")
message(WARNING "Translation directory does not exist: ${ARGN}")
return()
endif()这些修改提高了代码的可维护性和健壮性，同时保持了与不同 Qt 版本的兼容性。建议在实施这些更改时进行充分的测试，特别是在不同的 Qt 版本环境下。 |
lzwind
approved these changes
Dec 2, 2025
Contributor
|
[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
|
/merge |
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.
…uration
Log: Fix Qt5/Qt6 compatibility building issue.
Summary by Sourcery
Add Qt5/Qt6 compatible build configuration and centralised translation generation while aligning DTK usage with detected Qt version.
New Features:
Bug Fixes:
Enhancements:
Tests: