ci: pin GitHub Actions runners to explicit OS versions#508
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
.github/workflows/_build.yml
Outdated
| ( | ||
| contains(inputs.os_json, 'linux') || | ||
| contains(inputs.os_json, 'ubuntu-latest') || | ||
| contains(inputs.os_json, 'ubuntu-24.04') || |
There was a problem hiding this comment.
[Bug] contains(inputs.os_json, 'ubuntu-24.04') 会与 ubuntu-24.04-arm 产生子串误匹配,导致构建矩阵错误。
问题详述: contains() 是纯字符串子串匹配,不是 JSON 数组元素精确匹配。当用户手动触发工作流仅传入 os_json = '["ubuntu-24.04-arm"]'(只想构建 ARM 架构)时:
- 旧代码:
contains('["ubuntu-24.04-arm"]', 'ubuntu-latest')→false✅ 不会误匹配 - 新代码:
contains('["ubuntu-24.04-arm"]', 'ubuntu-24.04')→true❌ 子串误匹配!
这会导致:
build-linuxjob 的if条件提前为 true(L122)runs-onfallback 解析为ubuntu-24.04(L127),产生非预期的 x86_64 构建- arch 矩阵表达式(L136)中
contains(..., 'ubuntu-24.04') && contains(..., 'ubuntu-24.04-arm')均为 true,错误地构建 x86_64 + aarch64,而用户只想构建 aarch64
修复建议: 利用 JSON 数组中的双引号做精确匹配,将所有 contains(inputs.os_json, 'ubuntu-24.04') 替换为 contains(inputs.os_json, '"ubuntu-24.04"'):
# 修复前
contains(inputs.os_json, 'ubuntu-24.04')
# 修复后
contains(inputs.os_json, '"ubuntu-24.04"')contains('["ubuntu-24.04-arm"]', '"ubuntu-24.04"') → false,因为 JSON 中 "ubuntu-24.04-arm" 不包含子串 "ubuntu-24.04"(后面紧跟的是 -arm" 而非 ")。
此问题同样影响本文件 L127 和 L136。
.github/workflows/_build.yml
Outdated
| # Run only when non-Linux runners are explicitly requested | ||
| if: inputs.build_wheels && (contains(inputs.os_json, 'macos') || contains(inputs.os_json, 'windows')) | ||
| runs-on: ${{ matrix.os || 'ubuntu-latest' }} | ||
| runs-on: ${{ matrix.os || 'ubuntu-24.04' }} |
There was a problem hiding this comment.
[Suggestion] build-other job 的 fallback runs-on: ${{ matrix.os || 'ubuntu-24.04' }} 中的 ubuntu-24.04 在实际使用中不会被触发(因为 exclude 已排除所有 ubuntu 变体,matrix.os 始终有值),但语义上有误导性。建议移除 fallback 或添加注释说明。
445d5ad to
3cdbefd
Compare
Description
Related Issue
Type of Change
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes