Skip to content

feat: add dbus-activatable desktop entry for dde-shell#1600

Open
18202781743 wants to merge 1 commit into
linuxdeepin:masterfrom
18202781743:desktop
Open

feat: add dbus-activatable desktop entry for dde-shell#1600
18202781743 wants to merge 1 commit into
linuxdeepin:masterfrom
18202781743:desktop

Conversation

@18202781743
Copy link
Copy Markdown
Contributor

  1. Add a new desktop entry file org.deepin.dde-shell.desktop in the
    misc directory
  2. Configure the entry as a DBusActivatable application with Exec and
    TryExec set to /bin/false
  3. Set NoDisplay to true to hide from menus and launchers
  4. Categorize it under System for organization purposes
  5. This enables the DDE shell service to be started on demand via D-
    Bus activation

Log: Added D-Bus activation support for DDE Shell service

Influence:

  1. Verify that dde-shell can be activated via D-Bus when needed
  2. Test that the entry does not appear in application menus or launchers
  3. Confirm that the service starts on demand and stops when no longer
    needed

feat: 为 dde-shell 添加 D-Bus 激活的 desktop 条目

  1. 在 misc 目录中添加新的 desktop 文件 org.deepin.dde-shell.desktop
  2. 配置该条目为 DBusActivatable 应用,Exec 和 TryExec 设置为 /bin/false
  3. 设置 NoDisplay 为 true 以隐藏于菜单和启动器中
  4. 归类到 System 类别以便于组织管理
  5. 这使得 DDE shell 服务可以通过 D-Bus 激活按需启动

Log: 新增 DDE Shell 服务的 D-Bus 激活支持

Influence:

  1. 验证 dde-shell 能否在需要时通过 D-Bus 激活
  2. 测试该条目不会出现在应用菜单或启动器中
  3. 确认服务按需启动并在不需要时自动停止

1. Add a new desktop entry file org.deepin.dde-shell.desktop in the
misc directory
2. Configure the entry as a DBusActivatable application with Exec and
TryExec set to /bin/false
3. Set NoDisplay to true to hide from menus and launchers
4. Categorize it under System for organization purposes
5. This enables the DDE shell service to be started on demand via D-
Bus activation

Log: Added D-Bus activation support for DDE Shell service

Influence:
1. Verify that dde-shell can be activated via D-Bus when needed
2. Test that the entry does not appear in application menus or launchers
3. Confirm that the service starts on demand and stops when no longer
needed

feat: 为 dde-shell 添加 D-Bus 激活的 desktop 条目

1. 在 misc 目录中添加新的 desktop 文件 org.deepin.dde-shell.desktop
2. 配置该条目为 DBusActivatable 应用,Exec 和 TryExec 设置为 /bin/false
3. 设置 NoDisplay 为 true 以隐藏于菜单和启动器中
4. 归类到 System 类别以便于组织管理
5. 这使得 DDE shell 服务可以通过 D-Bus 激活按需启动

Log: 新增 DDE Shell 服务的 D-Bus 激活支持

Influence:
1. 验证 dde-shell 能否在需要时通过 D-Bus 激活
2. 测试该条目不会出现在应用菜单或启动器中
3. 确认服务按需启动并在不需要时自动停止
Copy link
Copy Markdown

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

Sorry @18202781743, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743

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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改的主要目的是为 dde-shell 添加 D-Bus 激活支持,通过创建 .desktop 文件并在代码中设置 desktopFileName,使得应用能够被 D-Bus 会话总线自动拉起。

整体来看,修改方向是正确的,但在代码安全、语法逻辑和代码质量方面有一些值得注意和改进的地方。以下是详细的审查意见:

1. 代码安全

⚠️ 潜在的安全误导:Exec=/bin/false

  • 问题分析:在 org.deepin.dde-shell.desktop 文件中,ExecTryExec 被设置为 /bin/false。虽然对于纯 D-Bus 激活的服务,系统确实会忽略 Exec 字段而依赖 DBusActivatable=true,但 /bin/false 是一个用于返回失败状态的系统命令,将其写在可执行路径上是一种“Hack”做法。
  • 安全隐患:如果未来某个桌面环境或脚本没有严格遵守规范,尝试直接通过 Exec 执行该 .desktop 文件,或者 D-Bus 激活失败后退而执行 Exec,将会静默执行 /bin/false 并返回错误码,这会导致极难排查的静默失败问题。
  • 改进建议:建议将 Exec 指向实际的执行文件(如 /usr/bin/dde-shell),或者如果坚持要明确表示不可直接执行,更安全的做法是留空或使用 /bin/true(至少不会返回异常错误码)。最佳实践是填写真实的执行路径:
    Exec=/usr/bin/dde-shell
    TryExec=/usr/bin/dde-shell

2. 语法逻辑

🔍 Debian Install 文件的通配符逻辑

  • 问题分析:在 debian/dde-shell.install 中添加了 usr/share/applications/*。在 Debian 打包规则中,* 会匹配该目录下的所有文件。
  • 潜在逻辑风险:如果后续该目录下添加了其他不需要被打包进 dde-shell 这个 deb 包的 .desktop 文件,使用通配符会导致误打包。
  • 改进建议:建议明确指定文件名,以增强构建的确定性和健壮性:
    usr/share/applications/org.deepin.dde-shell.desktop
    

🔍 SPDX 版权年份的更新逻辑

  • 问题分析shell/CMakeLists.txtshell/main.cpp 中的版权声明从 2023 修改为了 2023 - 2026
  • 潜在逻辑风险:当前是 2024 年,将结束年份写为 2026 是一种预期性的修改。通常开源项目的版权声明结束年份应该是代码最后修改的年份(即当前年份 2024),而不是未来的年份。写未来的年份在法律和规范上是不严谨的。
  • 改进建议:如果今年修改了该文件,应改为 2023 - 2024;如果是通过脚本自动生成年份,请检查脚本的逻辑。

3. 代码质量

💡 .desktop 文件的本地化质量

  • 问题分析org.deepin.dde-shell.desktop 中的 NameComment 是硬编码的英文字符串。
  • 改进建议:Deepin 桌面环境对国际化要求较高,建议遵循 freedesktop.org 的本地化规范,添加多语言支持。例如:
    Name=Deepin Desktop Shell
    Name[zh_CN]=深度桌面壳
    Comment=Deepin desktop shell service
    Comment[zh_CN]=深度桌面壳服务

💡 main.cpp 中的硬编码字符串

  • 问题分析a.setDesktopFileName(QStringLiteral("org.deepin.dde-shell")); 中的字符串与 .desktop 文件名强耦合。
  • 改进建议:虽然 QStringLiteral 已经保证了编译期优化,但为了防止未来 .desktop 文件名改动导致不同步,可以考虑将应用名称定义为宏或常量,在 CMakeLists.txt 中生成,以便统一管理。不过当前阶段直接使用字面量也是可以接受的,仅作为进阶优化建议。

4. 代码性能

  • 本次修改涉及的 .desktop 文件解析、D-Bus 激活机制和字符串赋值,均属于初始化阶段的一次性操作,对运行时性能没有影响,无性能瓶颈。

综合修改建议

如果采纳上述建议,你的代码可以修改为如下形式:

misc/org.deepin.dde-shell.desktop

[Desktop Entry]
Type=Application
Name=Deepin Desktop Shell
Name[zh_CN]=深度桌面壳
Comment=Deepin desktop shell service
Comment[zh_CN]=深度桌面壳服务
Icon=dde-shell
Exec=/usr/bin/dde-shell
TryExec=/usr/bin/dde-shell
DBusActivatable=true
NoDisplay=true
Terminal=false
StartupNotify=false
Categories=System;

debian/dde-shell.install

# ... 其他内容保持不变 ...
usr/share/applications/org.deepin.dde-shell.desktop
# ... 其他内容保持不变 ...

shell/CMakeLists.txt & shell/main.cpp

# 将 SPDX-FileCopyrightText 的结束年份改为当前修改年份
# SPDX-FileCopyrightText: 2023 - 2024 UnionTech Software Technology Co., Ltd.

Type=Application
Name=Deepin Desktop Shell
Comment=Deepin desktop shell service
Icon=dde-shell
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这个icon在哪,是放到图标主题?

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