[OPT] 改进 install_deps_windows.ps1：自动读取 vcpkg.json 安装依赖 - #19
Open
OFFMN-SHARP wants to merge 1 commit into
Open
Conversation
|
✅ Review Complete! The code review has been posted. View Review → |
审阅者指南重构 Windows 依赖安装脚本，使其直接从 更新后的 Install-VCPKG-Dependency PowerShell 函数流程图flowchart TD
A[Start Install-VCPKG-Dependency] --> B[Set vcpkgExe from script:vcpkgPath]
B --> C[Build jsonPath to vcpkg.json relative to script root]
C --> D{Does vcpkg.json exist at jsonPath?}
D -- No --> E[Write-Error: vcpkg.json not found]
E --> F[Return]
D -- Yes --> G[Read vcpkg.json and ConvertFrom-Json]
G --> H[Set depsList to json.dependencies]
H --> I{Is depsList.Count equal to 0?}
I -- Yes --> J[Write-Output: No dependencies listed]
J --> F
I -- No --> K[Iterate over each dep in depsList]
K --> L[Write-Output: Installing dep via vcpkg]
L --> M[Invoke vcpkgExe install dep]
M --> N{LASTEXITCODE not equal to 0?}
N -- Yes --> O[Write-Warning: Failed to install dep]
N -- No --> P[Proceed to next dep]
O --> P
P --> Q{More deps?}
Q -- Yes --> K
Q -- No --> R[End Install-VCPKG-Dependency]
文件级变更
可能关联的 Issue
技巧与命令与 Sourcery 交互
自定义你的使用体验访问你的 控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideRefactors the Windows dependency installation script to read dependencies directly from vcpkg.json and invoke vcpkg.exe from a shared vcpkgPath, adding basic validation and error handling around dependency installation. Flow diagram for updated Install-VCPKG-Dependency PowerShell functionflowchart TD
A[Start Install-VCPKG-Dependency] --> B[Set vcpkgExe from script:vcpkgPath]
B --> C[Build jsonPath to vcpkg.json relative to script root]
C --> D{Does vcpkg.json exist at jsonPath?}
D -- No --> E[Write-Error: vcpkg.json not found]
E --> F[Return]
D -- Yes --> G[Read vcpkg.json and ConvertFrom-Json]
G --> H[Set depsList to json.dependencies]
H --> I{Is depsList.Count equal to 0?}
I -- Yes --> J[Write-Output: No dependencies listed]
J --> F
I -- No --> K[Iterate over each dep in depsList]
K --> L[Write-Output: Installing dep via vcpkg]
L --> M[Invoke vcpkgExe install dep]
M --> N{LASTEXITCODE not equal to 0?}
N -- Yes --> O[Write-Warning: Failed to install dep]
N -- No --> P[Proceed to next dep]
O --> P
P --> Q{More deps?}
Q -- Yes --> K
Q -- No --> R[End Install-VCPKG-Dependency]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
你好——我给了一些整体性的反馈：
- 现在 Install-VCPKG-Dependency 依赖
$script:vcpkgPath已被设置；建议将 vcpkg 路径作为参数传入，或者在使用前先校验它，以避免在函数被单独调用时发生失败。 - vcpkg.json 的
dependencies字段可以是字符串数组也可以是对象数组；建议对$depsList做规范化处理（例如在需要时提取.name），并在访问.Count之前先防御性地检查$depsList是否为$null。 - 在调用 vcpkg 时，你可能希望更明确地暴露失败信息（例如，当任意安装失败时抛出异常或返回非零退出码），而不是只记录警告日志，这样调用方才能对依赖安装错误做出响应。
供 AI 代理使用的提示
Please address the comments from this code review:
## Overall Comments
- Install-VCPKG-Dependency now depends on `$script:vcpkgPath` being set; consider passing the vcpkg path as a parameter or validating it before use to avoid failures when the function is called in isolation.
- The vcpkg.json `dependencies` field can be either an array of strings or objects; consider normalizing `$depsList` (e.g., extracting `.name` when needed) and guarding against `$depsList` being `$null` before accessing `.Count`.
- When invoking vcpkg, you may want to surface failures more explicitly (e.g., throwing or returning a non-zero exit code when any install fails) instead of only logging a warning, so callers can react to dependency installation errors.帮我变得更有用！请在每条评论上点一下 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- Install-VCPKG-Dependency now depends on
$script:vcpkgPathbeing set; consider passing the vcpkg path as a parameter or validating it before use to avoid failures when the function is called in isolation. - The vcpkg.json
dependenciesfield can be either an array of strings or objects; consider normalizing$depsList(e.g., extracting.namewhen needed) and guarding against$depsListbeing$nullbefore accessing.Count. - When invoking vcpkg, you may want to surface failures more explicitly (e.g., throwing or returning a non-zero exit code when any install fails) instead of only logging a warning, so callers can react to dependency installation errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Install-VCPKG-Dependency now depends on `$script:vcpkgPath` being set; consider passing the vcpkg path as a parameter or validating it before use to avoid failures when the function is called in isolation.
- The vcpkg.json `dependencies` field can be either an array of strings or objects; consider normalizing `$depsList` (e.g., extracting `.name` when needed) and guarding against `$depsList` being `$null` before accessing `.Count`.
- When invoking vcpkg, you may want to surface failures more explicitly (e.g., throwing or returning a non-zero exit code when any install fails) instead of only logging a warning, so callers can react to dependency installation errors.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![arknightsautomachine-ai-reviewer[bot]](https://avatars.githubusercontent.com/in/3271960?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Summary
本次 PR 改进了 Windows 依赖安装脚本，将原本硬编码且空缺的依赖列表（含 TODO 注释）改为从 vcpkg.json 动态读取，并添加了基本的错误处理和退出码检查。这是一个有价值的修复，使脚本真正可用。
| File | Changes | Risk Level | Status |
|---|---|---|---|
| scripts/setup/install_deps_windows.ps1 | +22/-6 | 🟢 Low | Modified |
⚠️ Warnings
- 文件头信息：由于修改了文件，请确认文件头是否包含
@author OFFMN-SHARP和 AGPL-3.0 许可证声明（如原文件缺失，建议补全）。
💡 Suggestions
- JSON 解析容错：建议添加 try-catch 块处理
ConvertFrom-Json，防止 JSON 格式错误时脚本崩溃。 - 变量存在性检查：使用
$script:vcpkgPath前建议验证该变量是否存在（如if (-not $script:vcpkgPath)），避免在Install-VCPKG未执行时调用此函数导致空路径错误。 - 路径健壮性：相对路径
..\..依赖脚本存放位置，建议使用(Split-Path (Split-Path $PSScriptRoot -Parent) -Parent)显式获取项目根目录。
📍 Inline Comments
Open Questions
- 请确认
scripts/setup/install_deps_windows.ps1文件头部是否已包含 AGPL-3.0 许可证声明和@author OFFMN-SHARP标记？如缺失，建议在本 PR 中补全。 -
$script:vcpkgPath变量是否在脚本的其他位置（如Install-VCPKG函数）中定义？建议确认该变量的作用域和存在性检查。
统计: 0 🚨 | 2
This review was generated by ArknightsAutoMachine AI Reviewer
| ) | ||
| function Install-VCPKG-Dependency { | ||
| $vcpkgExe = "$script:vcpkgPath\vcpkg.exe" | ||
| $jsonPath = Join-Path $PSScriptRoot "..\..\vcpkg.json" |
There was a problem hiding this comment.
..\..\vcpkg.json 假设脚本始终位于 scripts/setup/ 目录。建议使用 (Split-Path (Split-Path $PSScriptRoot -Parent) -Parent) 显式获取仓库根目录，避免因脚本调用位置不同导致路径解析失败。
Contributor
Author
There was a problem hiding this comment.
emmm，如果需求健壮性，我其实比较喜欢C#，并且……这玩意是内置在用户手册里的，so……如果用户根本看不懂文件放在哪里，到时候咱俩都得遭殃
| } | ||
|
|
||
|
|
||
| $json = Get-Content $jsonPath -Raw | ConvertFrom-Json |
There was a problem hiding this comment.
💡 [错误处理] 建议添加 try-catch 块包裹 JSON 解析，防止文件格式错误时抛出未处理异常：
try {
$json = Get-Content $jsonPath -Raw | ConvertFrom-Json
} catch {
Write-Error "Failed to parse vcpkg.json: $_"
return
}
Contributor
Author
There was a problem hiding this comment.
emmm，对哦，OK，下个版本改
| foreach ($dep in $depsList) { | ||
| Write-Output "Installing $dep via vcpkg..." | ||
| & "$vcpkgPath" install "$dep" | ||
| & $vcpkgExe install $dep |
There was a problem hiding this comment.
💡 [安全性] 建议验证 $dep 格式后再执行，防止潜在的命令注入（尽管 vcpkg 包名通常受限）：
if ($dep -notmatch '^[\w\-\.]+$') {
Write-Warning "Invalid dependency name format: $dep"
continue
}
Contributor
Author
There was a problem hiding this comment.
你的意思是不信任Coredev们喽，你这是指桑骂槐，有反叛之心啊
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
我修复了Install-VCPKG-Dependency函数无法使用的问题
顺带一提
接下来我会找时间精简脚本
Summary by Sourcery
增强功能：
Install-VCPKG-DependencyPowerShell 函数，以解析vcpkg.json并自动安装其中列出的依赖项，同时在配置缺失、依赖列表为空以及安装失败时输出相应提示信息。Original summary in English
Summary by Sourcery
Enhancements: