Skip to content

Conversation

@18202781743
Copy link
Contributor

The CMake export configuration was using DTK_VERSION_MAJOR for
the namespace in the installed targets file, but this should use
DTK_NAME_SUFFIX instead. This ensures consistent naming across the
build system and proper namespace resolution when other projects link
against DLog. The change affects the INSTALL command for exporting
targets, updating the namespace from Dtk${DTK_VERSION_MAJOR}:: to
Dtk${DTK_NAME_SUFFIX}::.

Influence:

  1. Verify CMake configuration builds successfully
  2. Test that the exported targets file is generated with the correct
    namespace
  3. Check that dependent projects can find and link against DLog using
    the new namespace
  4. Ensure no regression in installation and packaging processes

fix: 使用 DTK_NAME_SUFFIX 作为 CMake 命名空间

CMake 导出配置原本使用 DTK_VERSION_MAJOR 作为已安装目标文件的命名空间,
现应改为使用 DTK_NAME_SUFFIX。这确保了构建系统中的命名一致性,以及其他项
目链接 DLog 时的正确命名空间解析。此更改影响了导出目标的 INSTALL 命令,
将命名空间从 Dtk${DTK_VERSION_MAJOR}:: 更新为 Dtk${DTK_NAME_SUFFIX}::。

Influence:

  1. 验证 CMake 配置构建成功
  2. 测试导出的目标文件是否使用正确的命名空间生成
  3. 检查依赖项目是否可以使用新的命名空间找到并链接 DLog
  4. 确保安装和打包过程没有回归问题

The CMake export configuration was using DTK_VERSION_MAJOR for
the namespace in the installed targets file, but this should use
DTK_NAME_SUFFIX instead. This ensures consistent naming across the
build system and proper namespace resolution when other projects link
against DLog. The change affects the INSTALL command for exporting
targets, updating the namespace from Dtk${DTK_VERSION_MAJOR}:: to
Dtk${DTK_NAME_SUFFIX}::.

Influence:
1. Verify CMake configuration builds successfully
2. Test that the exported targets file is generated with the correct
namespace
3. Check that dependent projects can find and link against DLog using
the new namespace
4. Ensure no regression in installation and packaging processes

fix: 使用 DTK_NAME_SUFFIX 作为 CMake 命名空间

CMake 导出配置原本使用 DTK_VERSION_MAJOR 作为已安装目标文件的命名空间,
现应改为使用 DTK_NAME_SUFFIX。这确保了构建系统中的命名一致性,以及其他项
目链接 DLog 时的正确命名空间解析。此更改影响了导出目标的 INSTALL 命令,
将命名空间从 Dtk${DTK_VERSION_MAJOR}:: 更新为 Dtk${DTK_NAME_SUFFIX}::。

Influence:
1. 验证 CMake 配置构建成功
2. 测试导出的目标文件是否使用正确的命名空间生成
3. 检查依赖项目是否可以使用新的命名空间找到并链接 DLog
4. 确保安装和打包过程没有回归问题
@18202781743 18202781743 requested review from BLumia and mhduiy January 6, 2026 10:55
deepin-ci-robot added a commit to linuxdeepin/dtk6log that referenced this pull request Jan 6, 2026
Synchronize source files from linuxdeepin/dtklog.

Source-pull-request: linuxdeepin/dtklog#25
@deepin-ci-robot
Copy link

deepin pr auto review

我来分析一下这个CMakeLists.txt的变更:

  1. 语法逻辑:
  • 这个修改将NAMESPACE从"Dtk${DTK_VERSION_MAJOR}::"改为"Dtk${DTK_NAME_SUFFIX}::"
  • 语法本身是正确的,符合CMake的INSTALL(EXPORT)命令格式要求
  1. 代码质量:
  • 这个改动提高了代码的灵活性。使用DTK_NAME_SUFFIX替代DTK_VERSION_MAJOR作为命名空间后缀,允许更灵活的命名策略
  • 建议确保DTK_NAME_SUFFIX变量在代码前面已经正确定义
  1. 代码性能:
  • 这个变更不会对构建性能产生显著影响,因为这只是改变了导出目标的命名空间标识符
  1. 代码安全:
  • 这个修改本身不会带来安全问题
  • 但建议确保DTK_NAME_SUFFIX变量的值是经过验证的,避免在命名空间中使用特殊字符或无效字符

改进建议:

  1. 建议在修改前添加对DTK_NAME_SUFFIX变量的检查:
if(NOT DEFINED DTK_NAME_SUFFIX)
    message(FATAL_ERROR "DTK_NAME_SUFFIX is not defined")
endif()
  1. 可以考虑添加对DTK_NAME_SUFFIX值的验证:
if(NOT DTK_NAME_SUFFIX MATCHES "^[a-zA-Z0-9_]+$")
    message(FATAL_ERROR "DTK_NAME_SUFFIX contains invalid characters")
endif()
  1. 建议在代码注释中说明使用DTK_NAME_SUFFIX的原因,以便其他开发者理解这个设计决策

总体来说,这是一个合理的改动,提高了代码的可维护性和灵活性,但需要确保新变量的正确性和安全性。

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2026

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

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

@18202781743
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Jan 6, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit ec56e33 into linuxdeepin:master Jan 6, 2026
11 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