[GongHeng2017]

-- Change ${CAMKE_INSTALL_BINDIR} to bin.

[deepin-ci-robot]

deepin pr auto review

我来分析这个CMakeLists.txt文件的变更：

1. 变更内容：

   • 安装路径从 ${CMAKE_INSTALL_BINDIR} 改为硬编码的 bin
   • 其他安装命令保持不变

2. 评估意见：

优点：

• 变更简单明了，直接使用了标准的 bin 目录

需要改进的地方：

1. 路径硬编码问题：

   • 建议保持使用 ${CMAKE_INSTALL_BINDIR} 而不是硬编码 bin
   • 理由：${CMAKE_INSTALL_BINDIR} 是CMake提供的标准变量，会根据系统配置自动选择正确的二进制文件安装路径（可能是 bin、usr/bin 等），提高了跨平台兼容性

2. 路径一致性：

   • 其他安装路径（如 share/dbus-1/system.d/）都是硬编码的
   • 建议统一使用CMake的安装路径变量，如：
     • ${CMAKE_INSTALL_DATADIR} 用于数据文件
     • ${CMAKE_INSTALL_LIBDIR} 用于库文件

3. 安全性考虑：

   • 确保所有安装路径都有适当的权限设置
   • 建议在install命令中添加权限设置，例如：

     install(FILES org.deepin.logviewer.ops.conf DESTINATION share/dbus-1/system.d/ PERMISSIONS OWNER_READ GROUP_READ WORLD_READ)

4. 可维护性：

   • 建议在文件开头定义所有安装路径变量，便于统一管理和修改
   • 例如：

     set(INSTALL_BIN_DIR ${CMAKE_INSTALL_BINDIR})
     set(INSTALL_DBUS_CONF_DIR ${CMAKE_INSTALL_DATADIR}/dbus-1/system.d)
     set(INSTALL_DBUS_SERVICE_DIR ${CMAKE_INSTALL_DATADIR}/dbus-1/system-services)
     set(INSTALL_SYSTEMD_DIR ${CMAKE_INSTALL_LIBDIR}/systemd/system)

建议的修改方案：

# 在文件开头定义安装路径变量
set(INSTALL_BIN_DIR ${CMAKE_INSTALL_BINDIR})
set(INSTALL_DBUS_CONF_DIR ${CMAKE_INSTALL_DATADIR}/dbus-1/system.d)
set(INSTALL_DBUS_SERVICE_DIR ${CMAKE_INSTALL_DATADIR}/dbus-1/system-services)
set(INSTALL_SYSTEMD_DIR ${CMAKE_INSTALL_LIBDIR}/systemd/system)

# 使用变量代替硬编码路径
install(TARGETS ${PROJECT_NAME} DESTINATION ${INSTALL_BIN_DIR})
install(FILES org.deepin.logviewer.ops.conf DESTINATION ${INSTALL_DBUS_CONF_DIR} 
    PERMISSIONS OWNER_READ GROUP_READ WORLD_READ)
install(FILES org.deepin.logviewer.ops.service DESTINATION ${INSTALL_DBUS_SERVICE_DIR}
    PERMISSIONS OWNER_READ GROUP_READ WORLD_READ)
install(FILES deepin-log-viewer-ops.service DESTINATION ${INSTALL_SYSTEMD_DIR}
    PERMISSIONS OWNER_READ GROUP_READ WORLD_READ)

这样的修改提高了：

1. 跨平台兼容性
2. 可维护性
3. 安装路径的一致性
4. 文件权限的安全性

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, max-lvs

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[GongHeng2017]

/forcemerge

[deepin-bot]

This pr force merged! (status: unstable)