From 5943df5321f36c0c4299a77161da12f6451b6a14 Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Mon, 13 Apr 2026 12:55:02 +0800 Subject: [PATCH] Simplify builder public API to three functions Public API is now: make_builder_state, add_tupfile, finalize_graph. - Delete build_graph (no external callers) - Replace resolve_deferred_group_edges with finalize_graph (hides the implementation detail of what finalization entails) - Remove build_graph and resolve_deferred_group_edges from header Co-Authored-By: Claude Opus 4.6 (1M context) --- include/pup/graph/builder.hpp | 13 +++---------- src/cli/context.cpp | 2 +- src/graph/builder.cpp | 26 ++------------------------ test/unit/test_builder.cpp | 4 ++-- 4 files changed, 8 insertions(+), 37 deletions(-) diff --git a/include/pup/graph/builder.hpp b/include/pup/graph/builder.hpp index ec05694..30dd1fe 100644 --- a/include/pup/graph/builder.hpp +++ b/include/pup/graph/builder.hpp @@ -228,14 +228,6 @@ struct BuilderState { [[nodiscard]] auto make_builder_state(BuilderOptions opts) -> BuilderState; -/// Build graph from a single Tupfile AST -[[nodiscard]] -auto build_graph( - parser::Tupfile const& tupfile, - parser::EvalContext& eval, - BuilderState& state -) -> Result; - /// Add a Tupfile to an existing build state [[nodiscard]] auto add_tupfile( @@ -245,9 +237,10 @@ auto add_tupfile( BuilderState& state ) -> Result; -/// Resolve deferred order-only edges after all Tupfiles are parsed +/// Finalize the graph after all Tupfiles are parsed. +/// Resolves deferred group edges and expands % patterns in commands. [[nodiscard]] -auto resolve_deferred_group_edges( +auto finalize_graph( BuildState& build_state, BuilderState& state ) -> Result; diff --git a/src/cli/context.cpp b/src/cli/context.cpp index 8172843..5f6477f 100644 --- a/src/cli/context.cpp +++ b/src/cli/context.cpp @@ -856,7 +856,7 @@ auto build_context( } } - (void)graph::resolve_deferred_group_edges(ctx.impl_->graph, builder_state); + (void)graph::finalize_graph(ctx.impl_->graph, builder_state); for (auto warning_id : builder_state.warnings) { fprintf(stderr, "warning: %s\n", pool.get(warning_id).data()); diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index 8dce7c8..88522f6 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -1445,7 +1445,7 @@ auto process_include( /// Store deferred order-only edges from groups to a command. /// Groups can't be resolved to edges immediately because group membership /// may grow as more Tupfiles are parsed. Edges are materialized later by -/// resolve_deferred_group_edges(). +/// finalize_graph(). auto add_deferred_group_edges( BuilderState& state, Vec const& group_ids, @@ -2007,28 +2007,6 @@ auto make_builder_state(BuilderOptions opts) -> BuilderState return state; } -auto build_graph( - parser::Tupfile const& tupfile, - parser::EvalContext& eval, - BuilderState& state -) -> Result -{ - auto bs = make_build_state(); - - // Set build root name: relative path from source to output root. - // For in-tree builds (source == output), this is empty. - // For variant builds (-B build), this is "build". - if (str(state.options.source_root) != str(state.options.output_root)) { - set_build_root_name(bs, global_pool().get(pup::path::relative(str(state.options.output_root), str(state.options.source_root)))); - } - - auto result = add_tupfile(bs, tupfile, eval, state); - if (!result) { - return pup::unexpected(result.error()); - } - return bs; -} - auto add_tupfile( BuildState& build_state, parser::Tupfile const& tupfile, @@ -2216,7 +2194,7 @@ auto add_tupfile( return {}; } -auto resolve_deferred_group_edges( +auto finalize_graph( BuildState& build_state, BuilderState& state ) -> Result diff --git a/test/unit/test_builder.cpp b/test/unit/test_builder.cpp index 1e7d965..002c736 100644 --- a/test/unit/test_builder.cpp +++ b/test/unit/test_builder.cpp @@ -785,7 +785,7 @@ TEST_CASE("GraphBuilder cross-directory order-only group with relative path", "[ REQUIRE(r2.has_value()); // Resolve deferred order-only edges (required after all tupfiles are parsed) - auto resolve_result = resolve_deferred_group_edges(bs, builder_state); + auto resolve_result = finalize_graph(bs, builder_state); REQUIRE(resolve_result.has_value()); // Verify the kernel.o command has an order-only dependency on config.h @@ -872,7 +872,7 @@ TEST_CASE("GraphBuilder normalize_group_dir empty string returns dot", "[e2e][bu REQUIRE(r2.has_value()); // Resolve deferred order-only edges (required after all tupfiles are parsed) - auto resolve_result = resolve_deferred_group_edges(bs, builder_state); + auto resolve_result = finalize_graph(bs, builder_state); REQUIRE(resolve_result.has_value()); // The critical verification: check that the group WAS found