Skip to content

build(cmake): refactor Qt version compatibility#24

Merged
re2zero merged 1 commit into
linuxdeepin:masterfrom
re2zero:bugfix
Jan 28, 2026
Merged

build(cmake): refactor Qt version compatibility#24
re2zero merged 1 commit into
linuxdeepin:masterfrom
re2zero:bugfix

Conversation

@re2zero
Copy link
Copy Markdown
Contributor

@re2zero re2zero commented Jan 27, 2026

  • 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

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

- 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

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-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码变更主要涉及构建系统(CMake)和打包配置(Debian)的更新,主要目的是优化 Qt 版本检测、规范化版本号处理,以及更新依赖关系。以下是详细的审查意见:

1. 语法逻辑审查

CMakeLists.txt

  • Qt 版本检测逻辑(优秀)
    • 变更前:分别查找 Qt6 和 Qt5,然后检查 Qt5_FOUNDQt6_FOUND。这种方式在逻辑上是正确的,但略显冗余。
    • 变更后:使用了 CMake 的自动检测机制 find_package(QT NAMES Qt6 Qt5 ...),这是 CMake 官方推荐的做法,逻辑更紧凑,自动优先选择 Qt6,回退到 Qt5,非常合理。
  • 版本号规范化(良好)
    • 新增了 if(VERSION MATCHES ...) 逻辑来提取标准语义化版本号。这可以防止因版本号字符串包含额外字符(如 1.0.0+u001)导致后续构建或打包工具出错。
    • 潜在逻辑风险set(VERSION "1.0.0" CACHE STRING ...) 定义了 VERSION 为缓存变量。如果用户在命令行通过 -DVERSION=1.2.3 指定了版本,逻辑是正常的。但如果用户之前构建过,CMakeCache.txt 中保留了旧值,且命令行未指定,代码逻辑能正常运行。不过,正则表达式 ^([0-9]+\.[0-9]+\.[0-9]+) 仅支持 X.Y.Z 格式,如果上游未来只用 X.Y,这里会强制回退到 1.0.0。建议确认上游版本号规范是否严格遵循 SemVer。

debian/control

  • 依赖变更(需注意)
    • 变更前:qt6-base-dev | qtbase5-dev(允许 Qt5 或 Qt6)。
    • 变更后:qt6-base-dev(强制要求 Qt6)。
    • 逻辑冲突:在 CMakeLists.txt 中,代码允许回退到 Qt5,但在 debian/control 中构建依赖强制要求 Qt6。这意味着在 Debian 打包环境下,实际上无法使用 Qt5 构建,CMake 中的 Qt5 回退逻辑在打包环境下是死代码。虽然这可能是有意为之(全面转向 Qt6),但需要确认是否真的要放弃对 Qt5 的打包支持。
  • 拼写错误
    • debian/control.1 中,描述字段 "A libaray about PDF" 存在拼写错误,应为 "library"。

deepin-pdfium.pc.in

  • 路径变量修正(正确)
    • libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@ 修改为 libdir=@CMAKE_INSTALL_LIBDIR@
    • 通常 CMAKE_INSTALL_LIBDIR 在 CMake 中可能是绝对路径(如 /usr/lib/x86_64-linux-gnu)或者相对路径(如 lib)。如果 CMake 配置正确,使用 @CMAKE_INSTALL_LIBDIR@ 直接替换通常更准确,避免多层路径拼接问题。

2. 代码质量审查

  • 可读性
    • 将 Qt 版本检测逻辑从分散的 find_package + if 判断改为 CMake 原生的 NAMES 机制,代码更简洁,可读性提升。
    • 注释从中文改为英文(如 # Qt6必须放前面... 改为 # Auto-detect Qt version...),有助于国际化协作,质量提升。
  • 一致性
    • 统一使用 QT_VERSION_MAJOR 变量替代了之前手动设置的 QT_DESIRED_VERSION,减少了变量定义和同步维护的成本,降低了出错风险。

3. 代码性能审查

  • 构建时间
    • find_package 的优化对构建时间影响极小,几乎可以忽略。
  • 运行时
    • 此部分为构建配置,不涉及运行时性能。

4. 代码安全审查

  • 输入验证
    • VERSION 变量进行了正则匹配验证,防止了非标准版本号字符串可能导致的构建脚本解析错误或潜在的注入风险(虽然 CMake 环境下风险较低,但良好的防御性编程值得肯定)。
  • 依赖安全
    • debian/control 中强制指定了 qt6-base-dev。如果这是为了强制使用更新的、包含更多安全修复的 Qt6 版本,那么这是一个提升安全性的变更。但如果目标环境尚未升级到 Qt6,会导致构建失败。

改进建议

  1. 统一构建策略

    • 建议明确项目是否同时支持 Qt5 和 Qt6。
    • 如果仅支持 Qt6:建议删除 CMakeLists.txt 中关于 Qt5 的查找逻辑(NAMES Qt6 Qt5 改为 NAMES Qt6),保持代码与 debian/control 一致,避免混淆。
    • 如果同时支持:建议修改 debian/control,恢复 qt6-base-dev | qtbase5-dev 的依赖关系,或者针对不同发行版使用不同的 control 文件。
  2. 版本号正则表达式优化

    • 建议根据实际项目版本发布策略调整正则。如果可能存在 1.0 这种两位版本号,可以将正则改为 ^([0-9]+\.[0-9]+(\.[0-9]+)?),并在后续逻辑中补全缺失的 .0,或者直接允许两位版本号。
  3. 修正拼写错误

    • 修正 debian/control.1 中的 "libaray" 为 "library"。
  4. CMake 变量作用域

    • set(VERSION ...) 在代码开头定义,并在 if(DEFINED VERSION) 块中被修改。由于使用了 CACHE,这个修改会写入缓存。这通常是期望的行为,但需注意如果在同一个构建树中多次运行 CMake 并传入不同的 VERSION 字符串,缓存可能不会立即更新,需要清理缓存或使用 FORCE 标志(不过此处逻辑是规范化,通常不需要 FORCE)。

总结

整体来看,这次变更提升了构建系统的现代化程度(使用 CMake 特性)和健壮性(版本号校验)。主要问题在于 debian/controlCMakeLists.txt 关于 Qt 版本支持的策略不一致,以及一处拼写错误。修复上述问题后,代码质量将得到进一步提升。

@deepin-ci-robot
Copy link
Copy Markdown

[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.

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

@re2zero re2zero merged commit d54f455 into linuxdeepin:master Jan 28, 2026
17 checks passed
@re2zero re2zero deleted the bugfix branch January 28, 2026 02:14
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