refactor(tool): 拆分调用、项目与进程上下文#108
Merged
ZR233 merged 1 commit intoMay 22, 2026
Merged
Conversation
619a9b0 to
2b9a34f
Compare
There was a problem hiding this comment.
审查总结
这是一个高质量的边界抽取重构 PR。核心目标是将 Tool 中的静态输入、项目定位、变量展开和子进程执行上下文拆分为显式对象(Invocation、ProjectLayout、VariableScope、ProcessContext),同时保留 Tool 作为兼容门面。
本地验证
- ✅
cargo fmt --all -- --check通过 - ✅
cargo clippy -p ostool --all-targets --all-features无警告 - ✅ 全部 166 个测试通过(143 单元测试 + 14 集成测试 + 3 doctests + 2 trybuild + 3 byte stream + 1 public API)
测试变更分析
新增测试(10 个):
main.rs新增 6 个 CLI 参数解析测试:覆盖 build、run qemu、run uboot、board run with cargo selector 等场景cargo-osrun.rs新增 2 个解析测试:QEMU 默认 runner 参数、U-Boot 子命令tool.rs新增variable_scope_errors_when_selected_package_is_missing:验证缺失包时错误传播tool.rs新增shell_run_cmd_injects_kernel_elf_when_runtime_elf_exists:验证 shell hook 注入 KERNEL_ELF
重命名/重构测试(4 个):
replace_string_*→expand_variables_*/variable_scope_*:反映 API 名称变更handle_output_records_runtime_artifact_state_without_objcopy→handle_output_records_runtime_artifact_state_from_resolved_cargo_artifact:适配新的显式参数传递
修改的测试行为:
board/config.rs中board_run_config_apply_overrides_replaces_board_type_and_server:改用tool.variable_scope().unwrap()而非&tool——语义完全一致cargo_builder.rs中handle_output测试:从内部状态resolved_artifact字段改为显式参数传入——行为等价
行为影响评估
| 变更点 | 原始行为 | 新行为 | 影响 |
|---|---|---|---|
Tool::command() 返回类型 |
Command(不返回错误) |
anyhow::Result<Command> |
内部 API,错误传播更明确 |
| 变量展开占位符 | workspace、workspaceFolder、package、tmpDir、env:* |
完全相同 | 无变化 |
| Shell 命令执行 | OS 分支 + KERNEL_ELF 注入 | 完全相同的逻辑移至 process::shell_run_cmd |
无变化 |
WORKSPACE_FOLDER 环境变量 |
始终设置 | 始终设置 | 无变化 |
resolve_manifest_context |
公开导出 | 仍公开导出,内部委托 resolve_project_layout |
向后兼容 |
| Cargo artifact 传递 | 通过 CargoBuilder.resolved_artifact 内部字段 |
通过显式返回值传递 | 行为等价 |
结论:未发现影响现有用户可见行为的变化。 所有 CLI 参数、配置文件格式、build/QEMU/U-Boot/board 运行时语义均保持不变。
代码质量
- 新模块(
invocation、project/*、process)结构清晰,职责单一 Tool门面的委托模式设计合理,为后续继续收窄Tool职责打下良好基础- 所有公开接口均有文档注释
- 错误传播路径一致使用
anyhow::Result
LGTM,整体设计良好,行为等价性有充分测试覆盖。
Powered by glm-5.1
Contributor
Author
|
AI,轻而易举啊! |
将 CLI 输入、项目路径解析、变量展开和 shell/process 调用准备从 legacy Tool 门面中拆出,便于后续继续收窄 build/run orchestration 的输入边界。 - 新增 Invocation/ProjectLayout,统一 ostool CLI 与 cargo-osrun 的 manifest、workspace 和 debug/bin/build-dir 输入。 - 新增 VariableScope 与 ProcessContext,让 QEMU、U-Boot、board 和 Cargo hook 通过显式上下文完成变量替换和命令构造。 - 保留 Tool 作为兼容门面,并继续同步现有 Cargo resolved artifact 到 runtime artifact state。 - 补充 CLI 参数解析、配置变量替换、缺失 package 错误传播和 runtime artifact state 的行为测试。 验证: - docker run --rm -v "/Users/nzh/Documents/GitHub/MRNIU/ostool":/workspace -w /workspace devbox:latest sh -lc 'cargo fmt --all -- --check && cargo clippy -p ostool --all-targets --all-features && cargo test -p ostool -- --nocapture' AI-assisted-by: OpenAI Codex
2b9a34f to
d231005
Compare
There was a problem hiding this comment.
审查总结
这是一个结构清晰、行为等价性良好的边界抽取重构 PR。将 Tool 中的静态输入、项目定位、变量展开和子进程执行上下文拆分为 Invocation、ProjectLayout、VariableScope、ProcessContext 四个显式对象,同时保留 Tool 作为兼容门面。
本地验证
- ✅
cargo fmt --all -- --check通过 - ✅
cargo clippy -p ostool --all-targets --all-features无警告 - ✅ 全部测试通过(143 单元 + 14 集成 + 1 public API + 2 trybuild + 3 byte stream + 3 doctests)
测例变更分析
新增测试(10 个):
main.rs新增 6 个 CLI 参数解析测试:覆盖 build、run qemu、run uboot、board run with cargo selector 等场景cargo-osrun.rs新增 2 个解析测试:QEMU 默认 runner 参数、U-Boot 子命令tool.rs新增variable_scope_errors_when_selected_package_is_missing:验证缺失包时错误传播tool.rs新增shell_run_cmd_injects_kernel_elf_when_runtime_elf_exists:验证 shell hook 注入 KERNEL_ELF
重命名/重构测试(6 个):
replace_string_*→expand_variables_*/variable_scope_*:反映 API 名称变更handle_output_records_runtime_artifact_state_without_objcopy→...from_resolved_cargo_artifact:适配显式参数传递qemu_config_explicit_path_supports_variables→read_qemu_config_from_variable_path_expands_workspace:测试范围扩大为端到端路径
修改的测试行为:
board/config.rs中board_run_config_apply_overrides_replaces_board_type_and_server:改用tool.variable_scope().unwrap()代替&tool——语义完全一致cargo_builder.rs中handle_output测试:从内部resolved_artifact字段改为显式参数传入——行为等价qemu.rs中read_qemu_config_at_path和ensure_qemu_config_at_path测试:改用tool.variable_scope().unwrap()代替&tool——语义一致uboot.rs中uboot_config_replace_strings_expands_all_fields测试:同上
行为影响评估
| 变更点 | 原始行为 | 新行为 | 影响 |
|---|---|---|---|
Tool::command() 返回类型 |
Command(不返回错误) |
anyhow::Result<Command> |
内部 API,错误传播更明确,所有调用点已适配 |
| 变量展开占位符 | workspace、workspaceFolder、package、tmpDir、env:* |
完全相同 | 无变化 |
| Shell 命令执行 | OS 分支 + KERNEL_ELF 注入 | 完全相同的逻辑移至 process::shell_run_cmd |
无变化 |
WORKSPACE_FOLDER 环境变量 |
始终设置 | 始终设置 | 无变化 |
resolve_manifest_context |
公开导出 | 仍公开导出,内部委托 resolve_project_layout |
向后兼容 |
| Cargo artifact 传递 | 通过 CargoBuilder.resolved_artifact 内部字段 |
通过显式返回值传递 | 行为等价 |
board_power_off_cmd 错误处理 |
let Err(err) = tool.shell_run_cmd(cmd) + warn |
tool.process_context().and_then(...) + warn |
行为等价,仍为日志警告不中断 |
结论:未发现影响现有用户可见行为的变化。 所有 CLI 参数、配置文件格式、build/QEMU/U-Boot/board 运行时语义均保持不变。
代码质量
- 新模块(
invocation、project/*、process)结构清晰,职责单一 Tool门面的委托模式设计合理,为后续继续收窄Tool职责打下良好基础- 所有公开接口均有文档注释
- 错误传播路径一致使用
anyhow::Result
LGTM 👍
Powered by glm-5.1
Merged
MRNIU
added a commit
to MRNIU/ostool
that referenced
this pull request
May 23, 2026
记录 PR drivercraft#108 合入后的 R1 状态,明确 R1a-R1d 在上游友好范围内的完成度,并把实施计划 checkbox 与当前代码事实对齐。 保留尚未拆出的 CargoBuildPipeline 等后续项,避免把 resolved artifact seam 误写成完整 pipeline 重构。 Validation: git diff --check AI-assisted-by: OpenAI Codex
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
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.
Summary
Invocation和ProjectLayout,统一 CLI 与cargo-osrun的 manifest、workspace、debug/bin/build-dir 输入。VariableScope和ProcessContext,让配置变量展开与 shell/process 命令构造使用显式上下文。Tool兼容门面,build、QEMU、U-Boot 和 board runtime 的用户可见行为保持不变。Overall Approach
这个 PR 是一次保持行为不变的边界抽取。核心思路是把一次 ostool 调用中的静态输入、项目定位、变量替换和子进程执行上下文先拆成显式对象,再让现有
Tool继续作为兼容门面承接 build/run 流程。具体来说:
Invocation/ProjectLayout负责把 CLI 或 library caller 传入的 manifest、build/bin 目录和 debug 标志解析成稳定的项目上下文。VariableScope负责${workspace}、${workspaceFolder}、${package}、${tmpDir}和${env:*}的替换输入。ProcessContext负责命令工作目录、WORKSPACE_FOLDER、KERNEL_ELF和参数/环境变量展开。Tool保存 legacy runtime artifact state,避免在本 PR 中改变外部行为。File Details
ostool/src/invocation.rs: 新增InvocationOptions和Invocation,把 CLI/library 输入与解析后的 project layout 绑定起来。ostool/src/project/layout.rs: 新增 manifest 和 workspace 解析逻辑,承接原先Tool中的 manifest path resolution。ostool/src/project/metadata.rs: 新增 Cargo metadata helper,用于读取 workspace metadata 和按 package 查找 manifest directory。ostool/src/project/variables.rs: 新增VariableScope与变量展开函数,集中处理 workspace/package/tmp/env 占位符。ostool/src/project/mod.rs: 汇总 project layout、metadata 和 variables 模块,并仅暴露 crate 内部需要的 project API。ostool/src/process/mod.rs: 新增ProcessContext、command builder 和 shell hook runner,统一子进程工作目录、环境变量和占位符展开。ostool/src/tool.rs: 将 manifest/project/metadata/variable/process 逻辑委托给新模块,同时保留Tool作为 build/run 的兼容门面和 runtime artifact state owner。ostool/src/main.rs: CLI 初始化改为先构造Invocation,再创建Tool;补充 build/run/board 参数解析护栏测试。ostool/src/bin/cargo-osrun.rs:cargo-osrun改为通过Invocation初始化 manifest context 和Tool;补充 QEMU 默认路径和 U-Boot subcommand 的解析测试。ostool/src/build/cargo_builder.rs: Cargo build 过程改为显式返回 resolved artifact,并通过ProcessContext执行 pre/post build hook 和构造 cargo command。ostool/src/build/mod.rs: custom build hook 改用ProcessContext执行 shell command。ostool/src/board/config.rs: board 配置和 CLI override 的字符串替换改为接收VariableScope,不再直接依赖完整Tool。ostool/src/board/mod.rs: board config 路径展开、配置加载和 runtime override 改用显式 variable scope。ostool/src/run/qemu.rs: QEMU 配置读取、默认配置创建、运行时配置替换和 command 构造改用VariableScope/ProcessContext。ostool/src/run/uboot.rs: U-Boot/local/net 配置替换和 reset/power-off hook 执行改用VariableScope/ProcessContext。ostool/src/lib.rs: 对外暴露 invocation 入口,同时将 project/process 保持为 crate-internal 模块。Compatibility
Tool仍然是现有 library caller 的主要入口。${package}相关变量展开会继续返回错误,而不是静默回退到 manifest directory。Tests
cargo fmt --all -- --checkcargo clippy -p ostool --all-targets --all-featurescargo test -p ostool -- --nocapture