Merged
fix makeself warning messages and remove file locks during JSON build #297
周代金Add on Aug 22
fix makeself warning messages and remove file locks during JSON build #297
Merged
周代金Add on Aug 22
Merged fromDeleted :masterintocann/cmakemaster
周代金Collaborators
Aug 22

当前PR是否有AI参与:

[x] 否

[ ] 是
__1. AI Agent 平台:
__2. AI 模型:
__3. Prompt上下文 :

PR功能描述 / 为什么需要这个合入**:

该PR关联的issue

(格式为fixes #<issue号>, 或者resolves #<issue号>): fixes #

希望检视人员了解:

改动类型 / Change Type

测试信息 / Testing

检查清单 / Checklist

likedislike
Pull Request has been successfully merged by CANN-robot.
(Thank you for 周代金's contribution.)
周周代金Collaborators
Aug 22 Created pull request, commit d642dd63
atomgit-bot
atomgit-bot
Aug 22 Comments:

变更摘要

本 PR 主要修复 makeself 第三方依赖拉取过程中的告警信息,并移除 JSON 第三方库构建时的文件锁。在 third_party/json.cmake 中移除了 DOWNLOAD_COMMAND 里的 flock 文件锁,新增 JSON_PRIVATE_USE 选项以在 CMAKE_BINARY_DIR 下使用私有源码路径规避并发写冲突,并调整了 patch 命令与 include 目录设置;在 third_party/makeself-fetch.cmake 中重构了拉取逻辑,将下载源切换为 gitcode.com,改为 tar 包与独立 patch 文件分开下载,并通过三分支流程处理"已打补丁源码 / 本地缓存 / 网络下载"三种场景。

主要改动

  • 移除 JSON 构建文件锁: 在 third_party/json.cmake 中,DOWNLOAD_COMMAND 删除了 flock ${JSON_SOURCE_PATH}/.json_build.lock 文件锁;同时新增 JSON_PRIVATE_USE 缓存选项,开启后将 JSON_SOURCE_PATH 指向 CMAKE_BINARY_DIR 下的私有路径,以规避并发构建时的写冲突
  • 修复 patch 命令告警: JSON 的 PATCH_COMMAND 由 patch --forward 改为 patch -N,makeself 应用补丁时同样使用 patch -N --batch --quiet -r - -p1,避免重复打补丁产生告警信息
  • makeself 拉取逻辑重构: third_party/makeself-fetch.cmake 由原来的本地 tar 包/下载两分支改为三分支:已存在打过补丁的源码则直接跳过;本地同时存在 tar 与 patch 则走缓存;否则通过 file(DOWNLOAD)(带 TIMEOUT 与状态码校验)分别下载 tar 和 patch 文件,再统一执行解压与打补丁
  • makeself 下载源与包体变更: 下载包由 makeself-release-2.5.0-patch1.tar.gz(华为云 OBS 地址)改为 makeself-release-2.5.0.tar.gz 加独立补丁文件 makeself-2.5.0.patch,下载源切换为 https://gitcode.com/cann-src-third-party/makeself/...
  • JSON 构建细节调整: target_include_directories(json ...) 由 SYSTEM INTERFACE 改为 INTERFACE,并在构建结束时新增 message(STATUS ...) 输出 JSON_SOURCE_PATH 便于定位
likedislike
Incorrect?
atomgit-bot
atomgit-bot
Aug 22 Comments:

代码审查

审查结论

变更文件逐一核对

1. third_party/json.cmake(已审查)

  • 新增 OPEN_PKG_PATH 默认值设置、JSON_PRIVATE_USE 私有源码路径选项、结束日志;PATCH_COMMAND 的 --forward→-N 语义等价,无问题。
  • 发现 2 个问题:
    • P3(line 68):移除 flock 串行锁后,默认配置(JSON_PRIVATE_USE 为 OFF)下并发构建共享 lib_cache/json 目录的拷贝存在竞态,可能损坏源树。
    • P3(line 99):target_include_directories 移除 SYSTEM,nlohmann 头文件告警将暴露到下游,存在 -Werror 编译失败风险。

2. third_party/makeself-fetch.cmake(已审查)

  • 下载/patch/解压流程重写、分支逻辑、错误消息优化已逐行核查。
  • 发现 2 个问题:
    • P2(line 33-41):新 file(DOWNLOAD) 丢失了旧 FetchContent 的 SHA256 校验(tar 与 patch 均无 EXPECTED_HASH),损坏/被篡改的下载内容会被静默接受并应用进构建树。
    • P2(line 79):将 patch 退出码 1(既可能是"已应用被跳过",也可能是"hunk 应用失败")一律当作成功,真正的补丁失败会被静默吞掉,构建使用未打补丁的 makeself。

汇总

  • 发现总数:4 个(P2 × 2,P3 × 2)。
  • 整体风险判断:中低风险,可合入但建议先处理两个 P2。核心风险集中在 makeself 下载链路——旧实现有 SHA256 校验而新实现完全缺失,叠加 patch 退出码 1 的放行逻辑,存在"损坏/被篡改的 tar/patch 被静默接受并应用"的供应链完整性与可靠性隐患;json.cmake 的两个 P3 属行为回归/默认配置下的并发风险,影响有限。改动本身的 CMake 语法与分支逻辑正确,错误处理(下载失败 FATAL_ERROR、提取失败检查)完整,无 P0/P1 问题。
类型 数量
🔴 阻塞 0
🟡 建议 1

💬 仅评论

likedislike
Incorrect?
CANN-robotCANN-robotCollaborators
Aug 22 Added label: cann-cla/yes
CANN-robot
CANN-robotCollaborators
Aug 22 Comments:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here.
You can get sig-info at here.
You can self-configure the PR merge rules for this repository. For more details, please refer to Here.
For more, you also can visit HICANN.


PR Approval Progress

✅ Congratulations! All modules have met the lgtm and approve requirements.

Module Approval Details

module lgtm status approve status
repo-cann/cmake ✅ zhaoyingdong, yangbinhua (2/2) ✅ zhaoyingdong (1/1)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)

CLA Signature Pass

gcw_UQE6G7Ro, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
114 messages are collapsed here。 See more
CANN-robotCANN-robotCollaborators
Aug 25 Added label: ci-pipeline-passed
yangbinhua
yangbinhuaCollaborators
Aug 25 Comments:

/lgtm

likedislike
zhaoyingdongCollaborators
Aug 25 Comments:

/approve

likedislike
CANN-robotCANN-robotCollaborators
Aug 25 Added label: lgtmapproved
CANN-robotCANN-robotCollaborators
Aug 25 Merged pull request