[Infra:Chore] Include VS version in Windows release package name#4581
[Infra:Chore] Include VS version in Windows release package name#4581wangzhaode wants to merge 3 commits into
Conversation
| path: ${{ steps.package.outputs.name }}.zip | ||
|
|
||
| macos-release: | ||
| needs: [setup] |
There was a problem hiding this comment.
整体逻辑合理,通过 vswhere 动态检测 Visual Studio 版本并纳入包名,比硬编码更有可维护性。以下几点建议:
-
冗余的
GITHUB_ENV设置:"PACKAGENAME=$packageName" >> $env:GITHUB_ENV将PACKAGENAME写入环境变量,但后续所有步骤(build / package / upload-zip)均使用${{ steps.package.outputs.name }},没有任何步骤引用${{ env.PACKAGENAME }}。这行代码是冗余的,建议移除以避免混淆。如果保留是为了调试可见性,建议添加注释说明。 -
vswhere输出的健壮性:catalog_productLineVersion通常返回2019、2022这样的字符串,但并非所有 VS 安装都保证该属性存在。建议在赋值后加一层校验,确认$vsYear匹配预期的年份格式(如正则^\d{4}$),避免生成异常的包名(如mnn_xxx_windows_x64_vs_cpu_opencl_vulkan_avx512)。 -
VS 2026 的推测性版本号:
'^18\.' { $vsYear = '2026' }是对未来的推测,目前 VS 18.x 尚未发布。如果 GitHub Actions 的 Windows runner 升级到未知版本,这个映射可能不准确。建议在default分支中记录 warning 或将未知版本映射为unknown而非直接 throw,以避免 CI 因版本号映射失败而中断发布流程。 -
安全性小注意:包名中嵌入了动态变量
$vsYear,虽然来源可信(本地 vswhere / 环境变量),但如果值被意外注入特殊字符,可能影响7z或文件路径操作。当前$vsYear经过Trim()处理,但建议对最终$packageName也做基本的路径安全字符校验。
| if: ${{ needs.setup.outputs.TARGET == 'all' }} | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| PACKAGENAME: mnn_${{ needs.setup.outputs.VERSION }}_linux_x64_cpu_opencl_vulkan |
There was a problem hiding this comment.
在 bash 脚本中直接使用了 ${{ github.event_name }} 和 ${{ github.event.inputs.target }} 的 GitHub Actions 表达式插值。虽然由于 inputs.target 的类型是 choice(限定为 all 或 windows),在此处不存在严重的安全风险,但在 run 脚本中直接插值违背了 GitHub Actions 的安全最佳实践,容易引发脚本注入漏洞。
建议通过 env 环境变量传递这些值,让 bash 在运行时解析:
- name: get-target
id: get_target
env:
EVENT_NAME: ${{ github.event_name }}
INPUT_TARGET: ${{ github.event.inputs.target }}
run: |
if [[ "$EVENT_NAME" == "workflow_dispatch" && -n "$INPUT_TARGET" ]]; then
echo "TARGET=$INPUT_TARGET" >> $GITHUB_OUTPUT
else
echo "TARGET=all" >> $GITHUB_OUTPUT
fi| if: ${{ needs.setup.outputs.TARGET == 'all' }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/download-artifact@v4 |
There was a problem hiding this comment.
当 TARGET 设置为 windows 时,其他平台的构建任务(如 linux, macos 等)会被跳过,而 upload-release 任务的 if 条件限制了它仅在 TARGET == 'all' 时运行。这意味着单独构建 Windows 版本时将不会上传构建产物。
如果单平台构建的初衷仅是为了验证编译是否通过(无需上传 Release),则当前逻辑是合理的;但如果希望单平台构建也能上传产物,则需要调整 if 条件,并处理 actions/download-artifact@v4 在部分 artifact 缺失时的报错问题(例如只在对应平台构建成功时上传,或分开下载 artifact)。
建议在此处添加注释,明确说明当 TARGET != 'all' 时跳过上传的设计意图,以避免后续维护时产生困惑。
Summary
windows-latest.vs20xxtag in the Windows release package and artifact name.Motivation
The Windows release package currently omits the compiler family, while
windows-latestis a floating GitHub runner label. Including the actual VS version in the package name makes the published binary environment clearer without requiring separate builds for every VS version.Validation
ruby -e 'require "yaml"; YAML.load_file(".github/workflows/mnn_release.yml"); puts "yaml ok"'clang-format (changed lines only),check for large files