Skip to content

feat(tests): add Qt5/Qt6 compatibility support#51

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
pppanghu77:master
Apr 24, 2026
Merged

feat(tests): add Qt5/Qt6 compatibility support#51
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
pppanghu77:master

Conversation

@pppanghu77
Copy link
Copy Markdown
Contributor

  • Add conditional package finding for Qt6 or Qt5 dependencies
  • Use generator expressions to link appropriate Qt version libraries
  • Add preprocessor conditionals for Qt6 vs Qt5 API usage
  • Maintain backward compatibility with both Qt frameworks

- Add conditional package finding for Qt6 or Qt5 dependencies
- Use generator expressions to link appropriate Qt version libraries
- Add preprocessor conditionals for Qt6 vs Qt5 API usage
- Maintain backward compatibility with both Qt frameworks
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码主要是为了支持项目同时兼容 Qt5 和 Qt6,以及对应的 DTK 版本。我将对这段代码进行详细审查,并提出改进建议。

1. 语法逻辑审查

CMakeLists.txt 部分:

  1. 变量检查逻辑问题

    if(Qt6_FOUND)

    在这个位置检查 Qt6_FOUND 是不恰当的,因为在此之前并没有调用 find_package 来查找 Qt6。这个变量可能未定义或来自父作用域,导致逻辑不可靠。

    建议:应该先尝试查找 Qt6,如果失败再查找 Qt5:

    find_package(Qt6 QUIET COMPONENTS Core Test)
    if(NOT Qt6_FOUND)
        find_package(Qt5 REQUIRED COMPONENTS Core Test)
    endif()
  2. DTK 包名不一致

    find_package(Dtk6 REQUIRED COMPONENTS Core)
    # ...
    find_package(DtkCore REQUIRED)

    Qt6 使用 Dtk6 包名,而 Qt5 使用 DtkCore 包名,这是正确的,但应该确保逻辑与 Qt 版本一致。

main.cpp 部分:

  1. 版本宏检查
    #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
    这是正确的 Qt 版本检查方式,没有问题。

2. 代码质量审查

  1. CMake 变量作用域
    当前代码没有明确设置 Qt6_FOUND 变量,可能导致子目录中的检查不可靠。建议在顶层 CMakeLists.txt 中统一处理 Qt 版本查找。

  2. 错误处理
    在 main.cpp 中,编码转换函数没有返回值检查。如果转换失败,out 可能为空,导致后续处理出现问题。

    建议

    bool success = Dtk::Core::DTextEncoding::convertTextEncodingEx(in, out, "utf-8", fromEncoding);
    if (!success) {
        qWarning() << "Failed to convert text encoding from" << fromEncoding;
        // 处理错误情况
    }

3. 代码性能审查

  1. 重复的版本检查
    在 CMakeLists.txt 中,$<IF:$<BOOL:${Qt6_FOUND}>,...> 重复多次。可以考虑定义一个变量来简化:

    set(QT_CORE_TARGET $<IF:$<BOOL:${Qt6_FOUND}>,Qt6::Core,Qt5::Core>)
    set(QT_TEST_TARGET $<IF:$<BOOL:${Qt6_FOUND}>,Qt6::Test,Qt5::Test>)
    set(DTK_CORE_TARGET $<IF:$<BOOL:${Qt6_FOUND}>,Dtk6::Core,Dtk::Core>)
    
    target_link_libraries(docparser_test PRIVATE docparser ${QT_CORE_TARGET} ${DTK_CORE_TARGET})
  2. 编码转换性能
    在 main.cpp 中,每次编码转换都创建新的 QByteArray 对象。如果频繁调用,可以考虑重用缓冲区。

4. 代码安全审查

  1. 输入验证
    fromEncoding 字符串没有验证,可能导致无效的编码名称传递给转换函数。

    建议

    if (fromEncoding.isEmpty() || fromEncoding.toLower() == "utf-8") {
        // 跳过转换
    } else {
        // 验证编码名称是否有效
        if (!QTextCodec::codecForName(fromEncoding.toUtf8())) {
            qWarning() << "Invalid encoding name:" << fromEncoding;
            // 处理错误
        }
        // 执行转换
    }
  2. 缓冲区溢出风险
    虽然使用了 QByteArray 来处理编码转换,但如果输入数据非常大,仍需注意内存使用情况。

5. 综合改进建议

改进后的 CMakeLists.txt:

set(CMAKE_AUTOMOC ON)

# 尝试查找 Qt6,如果失败则查找 Qt5
find_package(Qt6 QUIET COMPONENTS Core Test)
if(NOT Qt6_FOUND)
    find_package(Qt5 REQUIRED COMPONENTS Core Test)
    set(QT_VERSION_MAJOR 5)
    find_package(DtkCore REQUIRED)
    set(DTK_CORE_TARGET Dtk::Core)
else()
    set(QT_VERSION_MAJOR 6)
    find_package(Dtk6 REQUIRED COMPONENTS Core)
    set(DTK_CORE_TARGET Dtk6::Core)
endif()

# 设置 Qt 相关目标
set(QT_CORE_TARGET Qt${QT_VERSION_MAJOR}::Core)
set(QT_TEST_TARGET Qt${QT_VERSION_MAJOR}::Test)

# Enable testing
enable_testing()

# ... 其余代码保持不变 ...

# 使用预定义的目标变量
target_link_libraries(docparser_test
    PRIVATE
        docparser
        ${QT_CORE_TARGET}
        ${DTK_CORE_TARGET}
)

target_link_libraries(docparser_autotest
    PRIVATE
        docparser
        ${QT_CORE_TARGET}
        ${QT_TEST_TARGET}
)

改进后的 main.cpp:

// SPDX-FileCopyrightText: 2024 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: LGPL-3.0-or-later

#include <QCoreApplication>
#include <QDebug>
#include <QTextCodec>
#include <DTextEncoding>

// ... 其余代码保持不变 ...

int main(int argc, char **argv)
{
    // ... 初始化代码 ...
    
    if (fromEncoding.toLower() != "utf-8") {
        // 验证编码名称
        if (!QTextCodec::codecForName(fromEncoding.toUtf8())) {
            qWarning() << "Invalid encoding name:" << fromEncoding;
            return -1;
        }
        
        auto in = QByteArray::fromStdString(content);
        QByteArray out;
        
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
        bool success = Dtk::Core::DTextEncoding::convertTextEncodingEx(in, out, "utf-8", fromEncoding);
#else
        bool success = Dtk::Core::DTextEncoding::convertTextEncoding(in, out, "utf-8", fromEncoding);
#endif
        
        if (!success) {
            qWarning() << "Failed to convert text encoding from" << fromEncoding;
            return -1;
        }
        
        content = out.toStdString();
    }
    
    // ... 其余代码 ...
}

6. 其他建议

  1. 版权年份范围
    版权年份从 "2024" 改为 "2024 - 2026" 是合理的,表示版权将持续到 2026 年。

  2. 文档注释
    建议在编码转换部分添加注释,说明为什么 Qt5 和 Qt6 使用不同的函数。

  3. 日志记录
    在关键操作(如编码转换)处添加适当的日志记录,便于调试和问题追踪。

这些改进将使代码更加健壮、可维护,并且同时兼容 Qt5 和 Qt6。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs, pppanghu77

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

@pppanghu77
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented Apr 24, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 92c0ae9 into linuxdeepin:master Apr 24, 2026
20 of 22 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