diff --git a/include/pup/cli/context.hpp b/include/pup/cli/context.hpp index 26e52dd..051c5eb 100644 --- a/include/pup/cli/context.hpp +++ b/include/pup/cli/context.hpp @@ -81,6 +81,8 @@ class BuildContext { auto graph() const -> graph::BuildGraph const&; [[nodiscard]] auto graph() -> graph::BuildGraph&; + /// Root-level config vars (from output_root/tup.config + -D overrides). + /// Per-directory Tupfile evaluation uses scoped merged configs instead. [[nodiscard]] auto config_vars() const -> parser::VarDb const&; [[nodiscard]] diff --git a/src/cli/context.cpp b/src/cli/context.cpp index 667702b..ddc1dce 100644 --- a/src/cli/context.cpp +++ b/src/cli/context.cpp @@ -104,7 +104,8 @@ struct TupfileParseState { std::set available; std::set parsed; std::set parsing; - std::map scoped_configs; // Cache of per-dir configs + std::map parsed_configs; // Cache of parsed tup.config files (by path) + std::map scoped_configs; // Cache of merged per-dir configs std::vector> const* config_defines = nullptr; // CLI overrides }; @@ -223,8 +224,30 @@ auto apply_config_overrides( } } -/// Find the tup.config for a directory by walking up the tree -/// Returns pointer to the cached VarDb for that directory +/// Parse a tup.config file, returning a cached result on repeat calls. +auto get_or_parse_config( + std::filesystem::path const& path, + TupfileParseState& state +) -> parser::VarDb const* +{ + if (auto it = state.parsed_configs.find(path); it != state.parsed_configs.end()) { + return &it->second; + } + + auto result = parser::parse_config(path); + if (!result) { + fprintf(stderr, "Warning: Failed to parse %s: %s\n", path.string().c_str(), result.error().message.c_str()); + return nullptr; + } + + auto [it, _] = state.parsed_configs.emplace(path, std::move(*result)); + return &it->second; +} + +/// Merge all tup.config files from root down to target directory. +/// Parent configs override child configs on collision (same authority +/// model as Tuprules.tup ?= defaults). +/// Returns pointer to the cached VarDb for that directory. auto find_config_for_dir( std::filesystem::path const& rel_dir, std::filesystem::path const& output_root, @@ -238,35 +261,44 @@ auto find_config_for_dir( return &it->second; } - // Walk up from output_root/dir/ looking for tup.config - auto search_path = output_root / normalized; - while (true) { - auto config_path = search_path / "tup.config"; - if (std::filesystem::exists(config_path)) { - // Found a config - load and cache it - auto config_result = parser::parse_config(config_path); - if (config_result) { - apply_config_overrides(*config_result, state.config_defines); - auto [it, _] = state.scoped_configs.emplace(normalized, std::move(*config_result)); - return &it->second; + // Collect all tup.config paths from root down to target directory + auto config_paths = std::vector {}; + + auto root_config = output_root / "tup.config"; + if (std::filesystem::exists(root_config)) { + config_paths.push_back(root_config); + } + + if (!normalized.empty()) { + auto accumulated = output_root; + for (auto const& component : normalized) { + accumulated /= component; + auto config_path = accumulated / "tup.config"; + if (std::filesystem::exists(config_path)) { + config_paths.push_back(config_path); } - // Parse failed - warn user and return empty config (blocks inheritance) - fprintf(stderr, "Warning: Failed to parse %s: %s\n", config_path.string().c_str(), config_result.error().message.c_str()); - auto [it, _] = state.scoped_configs.emplace(normalized, parser::VarDb {}); - return &it->second; } + } - // Check if we've reached the output_root - if (search_path == output_root || !search_path.has_parent_path() - || search_path.parent_path() == search_path) { - break; - } + if (config_paths.empty()) { + auto [it, _] = state.scoped_configs.emplace(normalized, parser::VarDb {}); + return &it->second; + } - search_path = search_path.parent_path(); + // Merge leaf first (defaults), then each parent on top (overrides). + // config_paths is root-to-leaf, so reverse iteration gives leaf→root. + auto merged = parser::VarDb {}; + for (auto it = config_paths.rbegin(); it != config_paths.rend(); ++it) { + auto const* cfg = get_or_parse_config(*it, state); + if (cfg) { + for (auto const& name : cfg->names()) { + merged.set(std::string { name }, std::string { cfg->get(name) }); + } + } } - // No config found - cache empty config - auto [it, _] = state.scoped_configs.emplace(normalized, parser::VarDb {}); + apply_config_overrides(merged, state.config_defines); + auto [it, _] = state.scoped_configs.emplace(normalized, std::move(merged)); return &it->second; } @@ -624,12 +656,12 @@ auto build_context( printf("Found %zu directories with Tupfiles\n", ctx.impl_->state.available.size()); } - // 4. Load config + // 4. Load config (seeds the per-file parse cache for find_config_for_dir) auto config_path = ctx.impl_->layout.output_root / "tup.config"; if (std::filesystem::exists(config_path)) { - auto config_result = Result { parser::parse_config(config_path) }; - if (config_result) { - ctx.impl_->config_vars = std::move(*config_result); + auto const* root_cfg = get_or_parse_config(config_path, ctx.impl_->state); + if (root_cfg) { + ctx.impl_->config_vars = *root_cfg; if (ctx_opts.verbose) { printf("Loaded %zu config variables from %s\n", ctx.impl_->config_vars.names().size(), config_path.string().c_str()); } diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 902799f..1e6242d 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -2887,9 +2887,9 @@ SCENARIO("Target-based build with variant", "[e2e][target][variant]") // Scoped tup.config tests // ============================================================================= -SCENARIO("Subdir uses its own tup.config", "[e2e][scoped-config]") +SCENARIO("Subdir merges parent and local config", "[e2e][scoped-config]") { - GIVEN("a project with sub/Tupfile using @(SUB_VAR)") + GIVEN("a project with root and sub configs defining different vars") { auto f = E2EFixture { "scoped_config" }; f.mkdir("build/sub"); @@ -2901,15 +2901,15 @@ SCENARIO("Subdir uses its own tup.config", "[e2e][scoped-config]") { auto result = f.build({ "-B", "build" }); - THEN("@(SUB_VAR) resolves to 'from_sub'") + THEN("@(SUB_VAR) resolves to 'from_sub' from local config") { REQUIRE(result.success()); REQUIRE(f.read_file("build/sub/sub.txt") == "from_sub\n"); } - THEN("@(ROOT_VAR) in sub/ resolves to '' (not inherited)") + THEN("@(ROOT_VAR) in sub/ resolves to 'from_root' (merged from parent)") { - REQUIRE(f.read_file("build/sub/root_from_sub.txt") == "\n"); + REQUIRE(f.read_file("build/sub/root_from_sub.txt") == "from_root\n"); } } } @@ -2962,24 +2962,124 @@ SCENARIO("Root config used when no intermediate configs", "[e2e][scoped-config]" } } -SCENARIO("Empty subdir config blocks inheritance", "[e2e][scoped-config]") +SCENARIO("Empty subdir config does not block parent merge", "[e2e][scoped-config]") { - GIVEN("a project with sub/Tupfile using @(ROOT_VAR)") + GIVEN("a project with an empty sub config and a populated root config") { auto f = E2EFixture { "scoped_config" }; f.mkdir("build/sub"); f.write_file("build/tup.config", "CONFIG_ROOT_VAR=from_root\n"); - f.write_file("build/sub/tup.config", ""); // Empty config blocks lookup + f.write_file("build/sub/tup.config", ""); // Empty — parent vars merge through + REQUIRE(f.init().success()); + + WHEN("pup builds the project") + { + auto result = f.build({ "-B", "build" }); + + THEN("@(ROOT_VAR) in sub/ resolves to 'from_root' (parent merges through)") + { + REQUIRE(result.success()); + REQUIRE(f.read_file("build/sub/root_from_sub.txt") == "from_root\n"); + } + } + } +} + +SCENARIO("Parent config overrides child on collision", "[e2e][scoped-config]") +{ + GIVEN("root and sub configs both define SUB_VAR") + { + auto f = E2EFixture { "scoped_config" }; + f.mkdir("build/sub"); + f.write_file("build/tup.config", "CONFIG_SUB_VAR=from_root_override\n"); + f.write_file("build/sub/tup.config", "CONFIG_SUB_VAR=from_sub\n"); + REQUIRE(f.init().success()); + + WHEN("pup builds the project") + { + auto result = f.build({ "-B", "build" }); + + THEN("@(SUB_VAR) in sub/ resolves to root's value (parent wins)") + { + REQUIRE(result.success()); + REQUIRE(f.read_file("build/sub/sub.txt") == "from_root_override\n"); + } + } + } +} + +SCENARIO("Multi-level config merge", "[e2e][scoped-config]") +{ + GIVEN("configs at root, sub, and sub/deep levels") + { + auto f = E2EFixture { "scoped_config" }; + f.mkdir("build/sub/deep"); + f.write_file("build/tup.config", "CONFIG_ROOT_VAR=from_root\n"); + f.write_file("build/sub/tup.config", "CONFIG_SUB_VAR=from_sub\n"); + f.write_file("build/sub/deep/tup.config", "CONFIG_DEEP_VAR=from_deep\n"); + // Custom Tupfile to test all three vars at the deep level + f.write_file("sub/deep/Tupfile", + ": |> echo \"@(ROOT_VAR)\" > %o |> root_from_deep.txt\n" + ": |> echo \"@(SUB_VAR)\" > %o |> sub_from_deep.txt\n" + ": |> echo \"@(DEEP_VAR)\" > %o |> deep.txt\n"); REQUIRE(f.init().success()); WHEN("pup builds the project") { auto result = f.build({ "-B", "build" }); - THEN("@(ROOT_VAR) in sub/ resolves to '' (empty config blocks lookup)") + THEN("all three levels merge into sub/deep/") + { + REQUIRE(result.success()); + REQUIRE(f.read_file("build/sub/deep/root_from_deep.txt") == "from_root\n"); + REQUIRE(f.read_file("build/sub/deep/sub_from_deep.txt") == "from_sub\n"); + REQUIRE(f.read_file("build/sub/deep/deep.txt") == "from_deep\n"); + } + } + } +} + +SCENARIO("Parent can explicitly clear a child config var", "[e2e][scoped-config]") +{ + GIVEN("sub defines SUB_VAR and root clears it with empty value") + { + auto f = E2EFixture { "scoped_config" }; + f.mkdir("build/sub"); + f.write_file("build/tup.config", "CONFIG_SUB_VAR=\n"); + f.write_file("build/sub/tup.config", "CONFIG_SUB_VAR=default_value\n"); + REQUIRE(f.init().success()); + + WHEN("pup builds the project") + { + auto result = f.build({ "-B", "build" }); + + THEN("@(SUB_VAR) in sub/ is empty (parent's explicit clear wins)") + { + REQUIRE(result.success()); + REQUIRE(f.read_file("build/sub/sub.txt") == "\n"); + } + } + } +} + +SCENARIO("-D config overrides win over all config files", "[e2e][scoped-config]") +{ + GIVEN("root and sub configs both define SUB_VAR") + { + auto f = E2EFixture { "scoped_config" }; + f.mkdir("build/sub"); + f.write_file("build/tup.config", "CONFIG_SUB_VAR=from_root\n"); + f.write_file("build/sub/tup.config", "CONFIG_SUB_VAR=from_sub\n"); + REQUIRE(f.init().success()); + + WHEN("pup builds with -D SUB_VAR=from_cli") + { + auto result = f.build({ "-B", "build", "-D", "SUB_VAR=from_cli" }); + + THEN("@(SUB_VAR) in sub/ resolves to CLI value (highest precedence)") { REQUIRE(result.success()); - REQUIRE(f.read_file("build/sub/root_from_sub.txt") == "\n"); + REQUIRE(f.read_file("build/sub/sub.txt") == "from_cli\n"); } } }