Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Jan 22, 2026

treeland-dde-shell-v1.xml now requires wl_callback, which is not a part of wayland core.

Summary by Sourcery

Bug Fixes:

  • Fix build failures with newer treeland-protocols by disabling core-only Wayland protocol generation for dde shutdown, dock panel, and dock multitask view components.

treeland-dde-shell-v1.xml now requires wl_callback, which is not a
part of wayland core.

Log:
@BLumia BLumia requested a review from 18202781743 January 22, 2026 05:50
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 22, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds the NO_INCLUDE_CORE_ONLY flag to Wayland protocol client source generation for treeland-related protocols so that required non-core interfaces like wl_callback are included, fixing build failures with newer treeland-protocols.

File-Level Changes

Change Details Files
Ensure Wayland client code generation includes non-core interfaces required by treeland-dde-shell and related protocols.
  • Add NO_INCLUDE_CORE_ONLY to qt_generate_wayland_protocol_client_sources for the dde-shutdown applet Wayland protocol generation.
  • Add NO_INCLUDE_CORE_ONLY to qt_generate_wayland_protocol_client_sources for the dock panel Wayland protocol generation, affecting wallpaper color and dde-shell protocols.
  • Add NO_INCLUDE_CORE_ONLY to qt_generate_wayland_protocol_client_sources for the dock multitaskview Wayland protocol generation.
applets/dde-shutdown/CMakeLists.txt
panels/dock/CMakeLists.txt
panels/dock/multitaskview/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

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改主要涉及 CMake 构建系统中 Wayland 协议生成方式的调整。以下是对该 diff 的详细审查:

1. 语法逻辑审查

  • 结论:语法正确,符合 CMake 和 Qt6 的 qt_generate_wayland_protocol_client_sources 函数规范。
  • 分析NO_INCLUDE_CORE_ONLY 是 Qt 6.5+ 版本引入的一个参数选项。添加此参数意味着生成的协议代码将不再仅仅包含核心头文件(如 <wayland-client-core.h>),而是包含完整的 Wayland 客户端头文件(如 <wayland-client.h>)。这对于需要使用 Wayland 协议中非核心部分(如 wl_surfacewl_output 等全局接口对象)的功能是必要的。

2. 代码质量审查

  • 结论:代码质量良好,修改具有针对性。
  • 分析
    • 修改涉及三个模块:dde-shutdowndockdock-multitaskview
    • 这三个模块都依赖 treeland-dde-shell-v1.xml 协议。通常自定义的 Shell 协议会与 Wayland 核心对象进行交互(例如绑定到输出、表面等)。
    • 添加 NO_INCLUDE_CORE_ONLY 表明开发者意识到代码中需要使用完整的 Wayland 客户端 API,而不仅仅是核心协议 API,这是合理的逻辑判断。

3. 代码性能审查

  • 结论:对运行时性能无直接影响,主要影响编译时的依赖。
  • 分析
    • 编译时:由于包含了完整的 <wayland-client.h> 而非 <wayland-client-core.h>,编译单元可能会引入更多的符号定义。这可能会略微增加编译时间和生成的目标文件大小,但差异通常可以忽略不计。
    • 运行时:生成的二进制文件在运行时动态链接 libwayland-client.so。无论编译时引用的是 core 还是 full 头文件,最终链接的库通常是一致的,因此不会带来运行时性能损耗。

4. 代码安全审查

  • 结论:安全风险低,属于正常的构建配置调整。
  • 分析
    • 此修改仅改变编译器在预处理阶段包含的头文件范围,不涉及敏感数据处理或内存操作逻辑的改变。
    • 确保构建环境中的 Qt 版本(>= 6.5)支持该参数。如果 Qt 版本过旧,CMake 配置阶段可能会报错(未知参数)。建议在主 CMakeLists.txt 中添加对 Qt 版本的最低要求检查,例如 find_package(Qt6 6.5 REQUIRED COMPONENTS WaylandClient)

5. 改进建议

  1. 版本依赖检查
    为了防止在旧版本 Qt 环境下构建失败,建议在项目根目录的 CMakeLists.txt 中明确声明 Qt 最低版本要求。

    find_package(Qt6 6.5 REQUIRED COMPONENTS WaylandClient)
  2. 代码注释
    虽然不是必须的,但建议在添加 NO_INCLUDE_CORE_ONLY 的地方添加一行注释,解释为什么需要这个选项。例如:

    qt_generate_wayland_protocol_client_sources(dde-shutdown
        NO_INCLUDE_CORE_ONLY # 需要使用完整的 Wayland 客户端 API (如 wl_surface)
        FILES
            ${TREELAND_PROTOCOLS_DATA_DIR}/treeland-dde-shell-v1.xml
    )

    这有助于后续维护者理解为何不使用默认的 core-only 模式。

  3. 一致性检查
    请确认项目中其他使用 qt_generate_wayland_protocol_client_sources 的地方是否也需要此修改。如果其他模块也引用了需要完整 Wayland 对象的协议,应保持一致。

总结

这是一个合理的构建系统修改,目的是为了让生成的 Wayland 协议代码能够访问完整的 Wayland 客户端接口。代码逻辑正确,无明显安全隐患。建议补充 Qt 版本依赖检查以增强构建系统的健壮性。

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 left some high level feedback:

  • Since NO_INCLUDE_CORE_ONLY is a relatively new option, consider asserting a minimum Qt/CMake/ECM version (or adding a feature check) to avoid build failures on environments with older tooling.
  • To avoid duplication and keep behavior consistent across targets using treeland-dde-shell-v1.xml, consider centralizing the qt_generate_wayland_protocol_client_sources configuration (including NO_INCLUDE_CORE_ONLY) in a shared CMake module or function.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Since `NO_INCLUDE_CORE_ONLY` is a relatively new option, consider asserting a minimum Qt/CMake/ECM version (or adding a feature check) to avoid build failures on environments with older tooling.
- To avoid duplication and keep behavior consistent across targets using `treeland-dde-shell-v1.xml`, consider centralizing the `qt_generate_wayland_protocol_client_sources` configuration (including `NO_INCLUDE_CORE_ONLY`) in a shared CMake module or function.

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.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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 4111c43 into linuxdeepin:master Jan 22, 2026
10 of 11 checks passed
@BLumia BLumia deleted the ftbfs-tp branch January 22, 2026 06:32
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