refactor(tool): 移除旧 Tool 兼容 API#118
Open
MRNIU wants to merge 2 commits into
Open
Conversation
Remove the legacy Tool facade and route CLI/library flows through Invocation and module-level helpers. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Niu Zhihong <zhihong@nzhnb.com>
让 build config 加载保持纯解析,不再写入 Invocation active build state,也不再在加载阶段注入 someboot args。CLI 在应用 --package/--bin 后才激活最终 BuildConfig,runner 和 board 的 *_for_cargo helper 直接基于传入的 Cargo 配置计算变量作用域。 删除已经没有 wiring 的 cargo-osrun --show-output 解析,并新增覆盖占位 package 被 CLI selector 覆盖的回归测试。 Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Niu Zhihong <zhihong@nzhnb.com>
There was a problem hiding this comment.
Review 结果:✅ Approve
验证步骤
所有自动化验证均通过:
- ✅
cargo fmt -p ostool -- --check— 格式无误 - ✅
cargo clippy -p ostool --all-targets --all-features -- -D warnings— 无警告 - ✅
cargo test -p ostool— 169 个测试全部通过(144 单元测试 + 2 bin 测试 + 16 main 测试 + 1 trybuild 集成测试 + 3 qemu 测试 + 3 文档测试) - ✅
git diff --check— 无空白问题
代码审查
这是一个高质量的重构 PR,整体设计清晰、执行干净。主要亮点:
-
状态管理简化:移除了
AppContext/legacy_context双重状态桥接,消除了潜在的状态不一致风险。InvocationState现在是唯一的真相来源。 -
构建配置加载与激活分离:
load_build_config变为纯解析,不再写入Invocation的 active build state;activate_build_config显式地将最终配置注册为活跃状态。这使得 CLI--package/--bin覆盖的执行顺序更加清晰正确。 -
CLI 覆盖顺序修复:新增的回归测试
cargo_selector_overrides_config_package_before_activation验证了占位 package 被 CLI selector 覆盖后,board/QEMU/U-Boot 配置变量展开使用的是正确的最终 Cargo selection。 -
todo!()替换:uboot.rs中 FIT image 生成的架构匹配从_ => todo!()改为anyhow::bail!(),避免了生产环境中的 panic。 -
破坏性 API 变更文档完善:PR 描述详细列出了所有破坏性变更、迁移路径和 CLI 兼容性影响。
-
测试覆盖充分:旧
ToolAPI 测试全部替换为Invocation/模块级 API 测试,并新增了覆盖场景。
未发现阻塞性问题。
Powered by mimo-v2.5-pro
Contributor
Author
|
AI,轻而易举哇! |
Contributor
Author
|
@ZR233 hi,麻烦 review |
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.
摘要
Tool/ToolConfigfacade、ctx模块以及 legacyAppContext桥接层。cargo-osrun、build、QEMU、U-Boot、board 和 menuconfig 流程统一改为通过Invocation与模块级 helper 组织。.build.toml加载改为纯解析:调用方先合成最终配置,再显式激活 active build state。--package/--bin覆盖先于 board/QEMU/U-Boot 配置变量展开和 somebootbuild-info.toml参数注入生效。show_output和空的 U-Boot options 结构。ToolAPI fixture。整体思路
这个 PR 是 #117 的后续切片。#117 已经把 QEMU、U-Boot、board 和 TFTP 的核心运行入口拆成模块级函数,同时保留
Tool作为兼容 facade。本 PR 进一步完成迁移:不再同时维护Tool和Invocation两套入口,而是把 public Rust API 收敛到Invocation、InvocationOptions以及 build/run/board 模块级 helper。一次 ostool 调用应该只有一个明确的状态来源。
Invocation负责持有项目布局、运行选项和最终 active build/runtime state;build 与 runner 模块只接收自己需要的显式输入,不再通过 legacyAppContext或Tool::ctx*间接读取状态。build config source path 继续显式传递给需要解析相对路径的地方,但
.build.toml的加载本身不再激活Invocation。CLI 会先加载配置,再应用--package/--bin,最后激活最终BuildConfig。这样 board/QEMU/U-Boot 的${package}展开和 someboot 自动参数注入都基于同一个最终 Cargo selection,避免 raw config package 与 CLI override 不一致。*_for_cargorunner/config helper 也改为直接从传入的 Cargo config 计算 package 变量作用域,不再为了读配置而替换Invocation的 active build state。真正会写 active build state 的入口收敛到activate_build_config、build_with_config、cargo_build、cargo_run和 board run 等构建/运行路径。变更模块
ostool/src/lib.rs: 移除Tool、ToolConfig、ManifestContext和ctx的 public export,只保留新的 invocation/module-level API。ostool/src/tool.rs,ostool/src/ctx.rs,ostool/src/legacy_context.rs: 删除旧兼容 facade、legacy context 和AppContext桥接层。ostool/src/invocation.rs: 让 invocation state 直接承载 build/runtime 所需状态,并提供模块级流程需要的访问入口。ostool/src/build/mod.rs,ostool/src/build/config_loader.rs,ostool/src/build/cargo_pipeline.rs: 拆分 build config 纯加载、最终配置激活和 Cargo pipeline 参数注入。ostool/src/run/qemu.rs,ostool/src/run/uboot.rs: runner config/run helper 改为依赖Invocation与显式输入,移除旧兼容字段和空 options。ostool/src/board/mod.rs,ostool/src/board/config.rs: board run/config flow 改为使用 invocation/module-level helper,不再经由Tool状态桥接。ostool/src/main.rs,ostool/src/bin/cargo-osrun.rs,ostool/src/menuconfig.rs: CLI 与cargo-osrun入口迁移到新的调用方式,并修正 Cargo selector 覆盖顺序。README.md,README.en.md: 说明--package/--bin会影响最终变量展开和 someboot 自动参数注入。ostool/tests/public_api.rs和 trybuild fixtures: 删除旧ToolAPI 覆盖,新增 invocation/module-level API 的通过用例。破坏性 API 变更
本 PR 包含面向 ostool library 调用方的破坏性 Rust API 变更,不只是内部重构。请在 review、合并和后续发布说明中按 breaking change 处理。
Tool、ToolConfig、ManifestContext、ctx和 legacyAppContext访问入口被移除。Invocation、InvocationOptions,以及build、run、board模块提供的显式 helper。load_build_config_from_dir/load_build_config_from_path现在只返回解析后的BuildConfig,不会隐式激活Invocation;需要变量展开或后续运行的调用方应先合成最终配置,再调用activate_build_config或构建/运行 helper。*_for_cargohelper 不再接受build_config_path,也不再替换 active build state;它们只基于传入的 Cargo config 计算变量作用域。RunUbootOptions和 no-op 的show_output字段/flag 被移除;调用方不应再依赖这些兼容入口。CLI / 配置兼容性
.build.toml配置格式保持不变。--package/--bin的最终行为更明确:覆盖后的 Cargo selection 会同时影响 runner config 变量展开和 someboot 参数注入。cargo-osrun --show-output之前不会改变运行时行为;本 PR 删除该 no-op flag 后,继续传入该参数的脚本需要移除它。验证
已运行并通过:
git diff --checkcargo fmt --all -- --checkcargo clippy -p ostool --all-targets --all-features -- -D warningscargo test -p ostoolcargo clippy --target x86_64-unknown-linux-gnu --all-features -- -D warningscargo build --target x86_64-unknown-linux-gnu --all-featurescargo test --target x86_64-unknown-linux-gnu -- --nocapture风险 / 后续
ToolAPI 已删除,并说明 build config load/activate 拆分后的调用方式。