refactor(tool): 拆分构建与运行产物边界#111
Conversation
Split Cargo artifact selection from runtime artifact preparation so build output resolution, ELF stripping, and optional BIN conversion each live behind focused internal helpers. The Cargo pipeline now resolves executable artifacts from JSON messages before handing them to runtime artifact preparation, while Tool keeps the existing public artifact state as a compatibility bridge. Also ignore common local AI/agent tool state directories so generated workspace metadata is less likely to enter review diffs. Validation: cargo fmt --all -- --check; cargo test -p ostool -- --nocapture; cargo check -p ostool; cargo clippy -p ostool --all-targets --all-features; git diff --check. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Niu Zhihong <zhihong@nzhnb.com>
让 CargoBuildPipeline::execute() 只返回 CargoBuildOutcome,不再直接准备 runtime artifact 或写回 Tool runtime state。 runtime artifact 写回和 post-build hook 调用移动到 build orchestration 层,保持现有 cargo build/run 行为,同时让 R1d 的 build facts seam 与 R1e runtime helper 解耦。 验证: - docker exec devbox sh -lc 'cd /workspace/GitHub/MRNIU/ostool && cargo fmt --all -- --check && cargo test -p ostool --lib -- --nocapture && cargo test -p ostool --test public_api -- --nocapture && cargo check -p ostool' - docker exec devbox sh -lc 'cd /workspace/GitHub/MRNIU/ostool && cargo clippy -p ostool --lib --all-features' - docker exec devbox sh -lc 'cd /workspace/GitHub/MRNIU/ostool && cargo check -p ostool --target x86_64-unknown-linux-gnu' - git diff --check Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Niu Zhihong <zhihong@nzhnb.com>
There was a problem hiding this comment.
Review: refactor(tool): 拆分构建与运行产物边界
总体评价
本 PR 将 Cargo 构建流水线与运行时产物准备逻辑做了清晰的职责拆分,重构方向正确,代码质量良好。推荐合并。
架构改动分析
核心拆分思路:
CargoBuildPipeline::execute()现在只返回CargoBuildOutcome(仅包含ResolvedCargoArtifact),不再产生Tool副作用。- 运行时 ELF/BIN 准备逻辑移至独立模块
artifact::runtime,由 orchestration 层(cargo_build、cargo_run、prepare_cargo_runtime_artifacts)消费 outcome 后调用。 Tool::apply_prepared_runtime_artifacts()作为统一的桥接方法,将PreparedRuntimeArtifacts写回既有Tool.ctx.artifacts状态。
三个新模块职责清晰:
build::artifact_selector— Cargo JSON 消息解析与产物选择(210 行,含 7 个单元测试)artifact::runtime— ELF canonicalize、架构识别、strip、BIN 转换(298 行,含 2 个集成风格测试)build::cargo_pipeline— 精简后的 Cargo 构建流水线(execute()仅返回 outcome)
行为兼容性分析
执行顺序保持一致 ✅
旧流程 (CargoBuilder::execute):
pre-build → cargo → handle_output (set_elf + objcopy) → post-build
新流程:
pre-build (pipeline内部) → cargo (pipeline内部) → apply_cargo_build_outcome → run_cargo_post_build_cmds
post-build hook 始终在 runtime artifact 准备之后执行,顺序与旧代码一致。
cargo_build 路径的行为差异 ⚠️
旧的 cargo_build() 直接调用 CargoBuilder::build_auto(...).execute(),其中 execute() 内部会调用 handle_output() 来设置 ELF 路径、objcopy 输出 BIN,以及运行 post-build 命令。
新的 cargo_build() 在 apply_cargo_build_outcome() 中调用 prepare_runtime_artifacts(),strip_elf 参数始终传 false。而旧代码在 cargo_build 路径中走的也是 handle_output(不做 strip),因此行为一致。✅
prepare_elf_artifact 行为保持一致 ✅
旧:set_elf_artifact_path → objcopy_elf(strip)→ 可选 objcopy_output_bin
新:直接调用 prepare_runtime_artifacts,strip_elf: true
效果一致。
objcopy_output_bin 行为保持一致 ✅
旧代码直接在 Tool 上操作 objcopy,新代码委托给 prepare_runtime_artifacts,传入 strip_elf: false,仅做 BIN 转换。等价。
测试变更分析
被移除的测试
| 旧测试 | 原因 | 替代 |
|---|---|---|
select_executable_artifact_*(7 个,原在 cargo_builder.rs) |
函数移至 artifact_selector.rs |
在新位置有完全等价的 7 个测试 |
handle_output_records_runtime_artifact_state_from_resolved_cargo_artifact |
handle_output 方法已删除 |
被 apply_cargo_build_outcome_records_runtime_artifact_state(build/mod.rs)和 execute_returns_resolved_cargo_artifact_outcome(cargo_pipeline.rs)联合覆盖 |
set_elf_artifact_path_updates_dirs_and_arch |
set_elf_artifact_path 方法已删除 |
改为 apply_prepared_runtime_artifacts_updates_dirs_and_arch |
新增的测试
artifact_selector.rs:7 个单元测试,覆盖 explicit bin、package name fallback、default_run、single binary fallback、空输出报错、多 binary 歧义报错 — 与旧测试一一对应,逻辑不变。artifact::runtime:2 个测试prepares_stripped_elf_without_mutating_tool_state— 验证 strip 后 ELF 正确生成,且不修改 Tool 状态prepares_optional_bin_in_custom_output_dir— 验证 BIN 转换到自定义目录
build/mod.rs:apply_cargo_build_outcome_records_runtime_artifact_state— 验证 outcome 写回 Tool artifact 状态cargo_pipeline.rs:execute_returns_resolved_cargo_artifact_outcome— 使用 fake cargo 二进制验证端到端流水线不产生 Tool 副作用
测试覆盖度评估
- 测试总数:旧约 10 个 → 新约 12 个,覆盖度提升
- 新增的端到端测试
execute_returns_resolved_cargo_artifact_outcome使用了 fake cargo script,比旧的handle_output测试更健壮 artifact::runtime测试也使用 fake objcopy,避免了依赖系统工具- 关键改进:新测试验证了
execute()不产生Tool.ctx.artifacts副作用(assert!(tool.ctx.artifacts.elf.is_none())),这正是拆分的核心目标
trybuild fixture 更新
fail_cargo_pipeline.rs 和 .stderr:仅更新了内部类型名 CargoBuilder → CargoBuildPipeline、模块名 cargo_builder → cargo_pipeline。compile-fail 断言(private module error)不变。
其他改动
.gitignore:新增常见 AI/agent 本地状态目录忽略规则(.Codex/、.claude/、.cursor/等),合理。tool.rs:移除了set_elf_artifact_path、objcopy_elf、command()等方法,这些逻辑已迁移到artifact::runtime。Tool的公共 API(prepare_elf_artifact、objcopy_output_bin)行为不变。
小建议(非阻塞)
apply_cargo_build_outcome中objcopy_program硬编码为"rust-objcopy",与prepare_elf_artifact一致,但未来可能需要从配置中读取。PreparedRuntimeArtifacts的字段都是 private,只通过 getter 访问,设计良好。
结论
重构目标明确,将构建(Cargo JSON 解析)与运行时产物准备(ELF strip/BIN 转换)解耦为独立模块。测试充分,行为兼容,外部 API 不变。推荐合并。
Powered by glm-5.1
|
AI,轻而易举哇 |
|
@ZR233 hi,麻烦 review |
摘要
CargoBuildPipeline::execute()只返回CargoBuildOutcome,不直接准备 runtime artifact 或写回Tool状态。Toolartifact 状态作为兼容桥接,避免影响当前 runner 行为。.gitignore规则,包含.Codex/,降低生成目录误入 review diff 的概率。整体思路
构建路径现在拆成两个更清晰的内部职责:Cargo JSON message 解析负责选择最终可执行产物;运行时产物准备负责 ELF canonicalize、架构识别、生成 stripped ELF,以及按需生成 BIN。
CargoBuildPipeline::execute()只负责运行 Cargo、解析 JSON message,并返回CargoBuildOutcome。运行时 ELF/BIN 准备由 build orchestration 层在拿到 outcome 后调用 runtime artifact helper,再写回现有Toolartifact state。Tool仍然持有现有运行时状态。新 helper 返回准备好的 artifact 数据,再由兼容桥接路径写回既有 context 字段,因此外部 CLI、配置和 runner 语义保持不变。PR diff 只包含代码、测试和通用 ignore 规则更新,不包含 fork-only 计划资料。
文件说明
.gitignore:新增.Codex/、.claude/、.codex/、.cursor/、.continue/、.windsurf/、.aider/、.cline/、.roo/、.gemini/、.serena/等常见 AI/agent 本地状态目录。ostool/src/build/artifact_selector.rs:新增 Cargo executable artifact 选择器,覆盖 explicitbin、package-name fallback、default_run、single-binary fallback 和 ambiguity error。ostool/src/build/cargo_pipeline.rs:将原CargoBuilder调整为更明确的 Cargo 构建流水线,并在解析 Cargo JSON 输出后返回CargoBuildOutcome,不再直接处理 runtime artifact state。ostool/src/build/mod.rs:消费CargoBuildOutcome,在 orchestration 层调用 runtime artifact helper,并保持 post-build hook 的执行位置。ostool/src/artifact/runtime.rs:集中处理 ELF canonicalization、架构识别、stripped ELF 生成、可选 BIN 转换,以及 runtime artifact 目录记录。ostool/src/tool.rs:将 artifact 准备委托给新的 runtime artifact helper,同时保留现有ctx::OutputArtifacts写回路径。ostool/tests/public_api.rs和 trybuild fixtures:更新内部类型名相关的 compile-fail 覆盖。兼容性
验证
cargo fmt --all -- --checkcargo test -p ostool --lib -- --nocapturecargo test -p ostool --test public_api -- --nocapturecargo check -p ostoolcargo clippy -p ostool --lib --all-featurescargo check -p ostool --target x86_64-unknown-linux-gnugit diff --check说明:
cargo clippy -p ostool --all-targets --all-features当前会在既有ostool/tests/qemu_byte_stream.rs集成测试上报can't find crate for ostool,不属于本 PR 改动范围;本 PR 使用覆盖本次改动的 lib clippy 和 package check 作为验证。