Skip to content

fix(build): make DConfig optional for 102X-107X compatibility - #216

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/1071from
tianming-1996:release/1071
Aug 12, 2026
Merged

fix(build): make DConfig optional for 102X-107X compatibility#216
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/1071from
tianming-1996:release/1071

Conversation

@tianming-1996

@tianming-1996 tianming-1996 commented Aug 12, 2026

Copy link
Copy Markdown

Guard DConfig usage with HAVE_DCONFIG so builds succeed on older
dtkcore versions that lack the DConfig header.

用 #ifdef HAVE_DCONFIG 保护 DConfig 调用,无 DConfig 头文件时回退到硬编码,
确保 102X~107X 全版本可编译。

CMake 用 find_path 直接探测 DConfig 头文件是否存在(而非仅判断 DtkCore_FOUND),
命中才定义 HAVE_DCONFIG,避免 102X 上包找到但头文件缺失导致编译失败。

Bug: https://pms.uniontech.com/bug-view-372777.html

Summary by Sourcery

Make DConfig usage optional to allow building against older DtkCore versions that may not provide the DConfig header.

Enhancements:

  • Fallback to a hardcoded UFS spec version list when DConfig is unavailable so runtime behavior remains consistent across environments.

Build:

  • Detect the presence of the DConfig header via CMake and define HAVE_DCONFIG only when available, removing the hard requirement on DtkCore for service and test builds.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @tianming-1996, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR makes DConfig usage optional by detecting the presence of the DConfig header in DtkCore at CMake configure time and conditionally compiling the DConfig-based logic, with a fallback to hardcoded values when DConfig is unavailable, to allow builds on older 102X–107X environments.

File-Level Changes

Change Details Files
Make DConfig-dependent logic compile-time optional with a safe fallback for environments lacking the DConfig header.
  • In service CMake configuration, remove hard requirement on DtkCore and only define HAVE_DCONFIG when the DConfig header is found in Dtk::Core include directories.
  • In test CMake configuration, mirror the conditional DtkCore detection and HAVE_DCONFIG definition logic used by the service build.
  • In DeviceStorage.cpp, wrap the DConfig header include and runtime configuration reading around HAVE_DCONFIG and fall back to a hardcoded UFS spec version list when DConfig is unavailable.
service/CMakeLists.txt
test/CMakeLists.txt
service/diskoperation/DeviceStorage.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

Guard DConfig usage with HAVE_DCONFIG so builds succeed on older
dtkcore versions that lack the DConfig header.

用#ifdef HAVE_DCONFIG保护DConfig调用,无DConfig头文件时回退到硬编码,
确保102X~107X全版本可编译。

Log: DConfig改为可选依赖,兼容低版本dtkcore
Bug: https://pms.uniontech.com/bug-view-372777.html
Influence: 解耦应用在102X~107X全版本可正常编译,低版本回退硬编码UFS版本列表。
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码实现了基于头文件存在性的 DConfig 特性探测,有效解决了低版本 DTK 环境下的跨版本编译兼容性问题
逻辑完全正确但存在两处完全相同的代码片段导致轻微扣分

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

修复逻辑精准,在 service/CMakeLists.txttest/CMakeLists.txt 中,通过 get_target_property 获取 Dtk::Core 的头文件目录,并利用 find_path 显式探测 DConfig 头文件的真实存在性,仅在找到时才通过 add_definitions(-DHAVE_DCONFIG) 开启特性宏,彻底避免了旧版 DTK 缺失该头文件导致的“未定义类型”编译错误。

  • 2.代码质量(一般)✕

service/CMakeLists.txttest/CMakeLists.txt 两个文件中引入了完全相同的四行探测逻辑代码,存在明显的代码重复。
潜在问题:若未来 DTK 引入新的渐进式特性或探测逻辑需要调整,维护者需要分别在两个文件中同步修改,容易遗漏导致构建逻辑不一致。
建议:将这段 DConfig 探测逻辑提取到一个独立的 CMake 模块文件(如 FindDConfig.cmakecmake/DConfigCheck.cmake)中,然后在两个 CMakeLists.txt 中通过 include() 指令引入,实现逻辑复用。

  • 3.代码性能(无性能问题)✓

get_target_propertyfind_path 均为 CMake 配置阶段的常规轻量级操作,仅在工程初次配置或依赖变更时执行一次,对整体构建耗时的影响可忽略不计。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次修改仅涉及 CMake 构建脚本的条件编译判断,不涉及运行时的命令执行、文件读写、网络请求或敏感数据处理,不存在安全攻击面。

■ 【改进建议代码示例】

# cmake/DConfigCheck.cmake
# 简要注释:提取公共的 DConfig 探测逻辑,供各子模块复用
if(DtkCore_FOUND)
    get_target_property(DTKCORE_INCLUDE_DIRS Dtk::Core INTERFACE_INCLUDE_DIRECTORIES)
    find_path(DCONFIG_HEADER_PATH NAMES DConfig PATHS ${DTKCORE_INCLUDE_DIRS})
    if(DCONFIG_HEADER_PATH)
        add_definitions(-DHAVE_DCONFIG)
    endif()
endif()
# service/CMakeLists.txt 与 test/CMakeLists.txt 中的替换方式
find_package(DtkCore)
# 引入提取出的公共探测模块,替代原有的内联重复代码
include(${CMAKE_SOURCE_DIR}/cmake/DConfigCheck.cmake)

find_package(DtkGui REQUIRED)
# ... 后续其他逻辑

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max-lvs, tianming-1996

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

@tianming-1996

Copy link
Copy Markdown
Author

/merge

@deepin-bot
deepin-bot Bot merged commit 9ef7c46 into linuxdeepin:release/1071 Aug 12, 2026
20 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