Skip to content

Feat: [log] Can not build in v20. - #419

Merged
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
GongHeng2017:202511111003-master-feat
Nov 13, 2025
Merged

deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
GongHeng2017:202511111003-master-feat

Conversation

@GongHeng2017

Copy link
Copy Markdown
Contributor

-- Add compatibility code

Log: fix issue
Task: https://pms.uniontech.com/task-view-383495.html

    Ensure the postinst script exits successfully by adding "exit 0" to
    avoid package installation failures. Avoid potential stuck issues by
    adding the -f option to runuser.

 Bug:【ztgd2025072500011 osgd2025072500005】【四级】升腾M460同批次
 Log: fix package installation failure and stuck
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我来对这个diff进行仔细审查:

  1. logallexportthread.cpp 修改:
#if (QT_VERSION <= QT_VERSION_CHECK(5, 15, 0))
    QStringList pathParts = filePath.split('/', QString::SkipEmptyParts);
#else
    QStringList pathParts = filePath.split('/', Qt::SkipEmptyParts);
#endif

改进建议:

  • 这是为了兼容不同Qt版本的正确修改,但建议:
    1. 将版本检查的宏定义移到文件开头或公共头文件中,便于统一管理
    2. 考虑使用QDir::toNativeSeparators()处理路径分隔符,提高跨平台兼容性
    3. 可以考虑使用QFileInfo::absolutePath()来获取目录部分,代码更简洁
  1. deepin-log-viewer.postinst 修改:
serviceStatus=$(runuser -f -l ${userName} -c "XDG_RUNTIME_DIR=\"/run/user/$(id -u ${userName})\" systemctl --user is-active ${sysTimer}")

改进建议:

  • 安全性:

    1. runuser命令增加了-f参数,这是正确的改进,可以确保环境变量正确传递
    2. 建议对userName进行输入验证,防止注入攻击
    3. XDG_RUNTIME_DIR的设置应该考虑使用变量,避免重复
  • 代码质量:

    1. 建议将重复的runuser命令封装成函数
    2. 可以添加日志记录,便于调试
    3. 错误处理可以更细致,目前使用 || true 可能会掩盖一些实际问题
  • 性能:

    1. 可以考虑并行处理多个用户的服务状态检查
    2. 避免重复计算id -u ${userName}
  • 其他改进建议:

    1. 添加脚本开头的set -e,确保遇到错误立即退出
    2. 添加注释说明脚本的功能和注意事项
    3. 考虑使用systemctl的--quiet选项减少输出
  1. 通用建议:
  • 建议添加版本控制信息注释
  • 考虑添加单元测试
  • 文档更新应该同步进行

总的来说,这些修改是合理的,但还有改进空间,特别是在代码复用、错误处理和安全性方面。建议在后续版本中逐步完善。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, lzwind

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

@GongHeng2017

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
deepin-bot Bot merged commit e0cec1f into linuxdeepin:master Nov 13, 2025
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