Scoped config merging: parent overrides child

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]]

src/cli/context.cpp

@@ -104,7 +104,8 @@ struct TupfileParseState {
     std::set<std::filesystem::path> available;
     std::set<std::filesystem::path> parsed;
     std::set<std::filesystem::path> parsing;
-    std::map<std::filesystem::path, parser::VarDb> scoped_configs;                    // Cache of per-dir configs
+    std::map<std::filesystem::path, parser::VarDb> parsed_configs;                    // Cache of parsed tup.config files (by path)
+    std::map<std::filesystem::path, parser::VarDb> scoped_configs;                    // Cache of merged per-dir configs
     std::vector<std::pair<std::string, std::string>> 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<std::filesystem::path> {};
+
+    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::VarDb> { 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());
             }

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");
             }
         }
     }