Skip to content

Conversation

@ComixHe
Copy link
Contributor

@ComixHe ComixHe commented Jan 27, 2026

Summary by Sourcery

Build:

  • Update CMake install destination path for systemd user services to remove trailing slash and comply with CMP0177.

…0177

Signed-off-by: ComixHe <heyuming@deepin.org>
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 27, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the systemd user service installation path in CMake to remove a trailing slash from SERVICE_DEST_PATH, aligning with CMP0177 behavior while preserving subdirectory installation for the drop-in service file.

File-Level Changes

Change Details Files
Normalize SERVICE_DEST_PATH in CMake to exclude a trailing slash to comply with CMP0177 while keeping install destinations correct.
  • Change SERVICE_DEST_PATH definition to omit the trailing slash in the base systemd user directory path.
  • Retain use of SERVICE_DEST_PATH for the main systemd user service file installation.
  • Continue constructing the drop-in directory path by appending the app-specific subdirectory to SERVICE_DEST_PATH.
misc/CMakeLists.txt

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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@ComixHe ComixHe requested a review from BLumia January 27, 2026 06:14
BLumia
BLumia previously approved these changes Jan 27, 2026
to prevent cmake generating some build files to source tree.

Signed-off-by: ComixHe <heyuming@deepin.org>
@ComixHe ComixHe changed the title fix(build): remove trailing slash in SERVICE_DEST_PATH to resolve CMP0177 fix some build issues Jan 27, 2026
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码diff包含了两处修改,分别涉及misc/CMakeLists.txttests/CMakeLists.txt。下面是对这两处修改的详细审查和改进意见:

1. misc/CMakeLists.txt 修改分析

修改内容:

-set(SERVICE_DEST_PATH ${CMAKE_INSTALL_PREFIX}/lib/systemd/user/)
+set(SERVICE_DEST_PATH ${CMAKE_INSTALL_PREFIX}/lib/systemd/user)

审查意见:

  • 语法逻辑:修改是正确的。在CMake中,DESTINATION 路径末尾是否带斜杠通常不会影响最终安装路径的构建,因为CMake会自动处理路径拼接。然而,去掉末尾的斜杠是更规范的做法,可以避免在某些特殊情况下(如路径拼接逻辑变更)产生双斜杠(//)的潜在风险,尽管Linux文件系统通常能容忍双斜杠。
  • 代码质量:代码质量良好。保持路径字符串的一致性(统一不带尾部斜杠)有助于提高代码的可读性和可维护性。
  • 代码性能:无影响。
  • 代码安全:无直接影响。虽然去掉了斜杠,但随后的 install 命令中使用了 ${SERVICE_DEST_PATH}/app-DDE-.service.d,这种拼接方式是安全的。

建议:

  • 这是一个良好的清理改动,建议保留。

2. tests/CMakeLists.txt 修改分析

修改内容:

-gtest_discover_tests(${BIN_NAME} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests)
+gtest_discover_tests(${BIN_NAME} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})

审查意见:

  • 语法逻辑:修改在语法上是正确的,但逻辑上发生了重大变化。
    • 原逻辑:测试的运行工作目录被设置为源码目录下的 tests 文件夹。这通常用于测试需要访问源码目录中特定资源文件(如测试数据、配置文件)的情况。
    • 新逻辑:测试的运行工作目录被设置为当前构建目录(即编译输出目录)。
  • 代码质量
    • 如果测试用例依赖于相对路径来读取源码目录下的文件,这个修改将导致测试失败,因为相对路径的基准点变了。
    • 如果测试用例是独立的,不依赖源码目录下的文件,或者测试数据已经被配置为拷贝到了构建目录,那么这个修改是合理的,符合"构建产物应在构建目录中运行"的通常惯例。
  • 代码性能:无影响。
  • 代码安全:无直接影响。

建议:

  • 关键风险提示:请务必确认该测试目录下的测试用例是否依赖 ${PROJECT_SOURCE_DIR}/tests 下的资源文件。
    • 如果依赖,此修改会导致测试找不到文件而失败。解决方法可以是保留原设置,或者在构建时通过 configure_fileadd_custom_command 将必要的资源文件复制到 ${CMAKE_CURRENT_BINARY_DIR}
    • 如果不依赖,此修改是合理的,有助于实现源码目录和构建目录的分离(Out-of-source build)。

总结

  1. misc/CMakeLists.txt 的修改是一个路径规范化的改进,建议接受。
  2. tests/CMakeLists.txt 的修改改变了测试运行时的上下文环境。请务必进行回归测试,确保所有测试用例在新的工作目录下依然能通过,特别是那些涉及文件读写的测试用例。如果测试失败,需要回退此修改或调整测试资源的处理方式。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, ComixHe

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

@BLumia BLumia merged commit fe020af into linuxdeepin:master Jan 27, 2026
15 of 17 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