Fold env-driven conditionals into command identity

include/pup/graph/builder.hpp

@@ -127,6 +127,12 @@ struct BuilderContext {
     /// Commands inside conditionals need to depend on these vars to rebuild when
     /// the condition's value changes.
     SortedIdVec condition_config_vars = {};
+
+    /// Env variable StringIds used in enclosing conditions (symmetric counterpart of
+    /// condition_config_vars). Without this, a guard flip driven by an env-sourced
+    /// $(TUP_PLATFORM)-style var would not change any command's identity and the
+    /// rebuild would be missed.
+    SortedIdVec condition_env_vars = {};
 };
 
 // ============================================================================

src/graph/builder.cpp

@@ -513,6 +513,16 @@ auto create_command_node(
         }
     }
 
+    // Add sticky edges from condition env variables (symmetric to condition_config_vars
+    // — so an env-driven guard flip changes every guarded command's identity).
+    auto const* cev = ctx.condition_env_vars.data();
+    for (std::size_t i = 0, n = ctx.condition_env_vars.size(); i < n; ++i) {
+        auto const* node_id = state.imported_env_var_nodes.find(cev[i]);
+        if (node_id) {
+            (void)add_edge(ctx.state->graph, *node_id, cmd_id, LinkType::Sticky);
+        }
+    }
+
     // Add sticky edges from used imported env variables (fine-grained dependency tracking)
     auto const* uev = ctx.used_env_vars.data();
     for (std::size_t i = 0, n = ctx.used_env_vars.size(); i < n; ++i) {
@@ -1114,6 +1124,55 @@ auto process_export(
     return {};
 }
 
+/// Find or create a `NodeType::Variable` node under the env-var directory for the
+/// given env var name and value. Updates the existing node's name and content_hash
+/// if the value changed. Returns the node id, or nullopt if the env-var directory
+/// has not been initialized yet. Used by `process_import` and the `on_env_var_used`
+/// callback so that any env var the build depends on — whether explicitly imported
+/// or auto-resolved like TUP_PLATFORM/TUP_ARCH — gets the same tracking node, and
+/// the existing sticky-edge machinery in create_command_node folds it into command
+/// identity.
+auto ensure_env_var_node(
+    BuilderContext& ctx,
+    Builder& state,
+    std::string_view var_name,
+    std::string_view value
+) -> std::optional<NodeId>
+{
+    if (state.env_var_dir_id == INVALID_NODE_ID) {
+        return std::nullopt;
+    }
+    auto var_name_id = to_underlying(intern(var_name));
+    auto node_name_buf = Buf {};
+    node_name_buf += var_name;
+    node_name_buf += '=';
+    node_name_buf += value;
+    auto const name_id = intern(node_name_buf.view());
+    auto content_hash = sha256(value);
+
+    if (auto const* existing_node_id = state.imported_env_var_nodes.find(var_name_id)) {
+        if (auto* existing = get_file_node(ctx.state->graph, *existing_node_id)) {
+            if (existing->name != name_id) {
+                existing->name = name_id;
+                existing->content_hash = content_hash;
+            }
+        }
+        return *existing_node_id;
+    }
+    auto node = FileNode {
+        .type = NodeType::Variable,
+        .name = name_id,
+        .parent_dir = state.env_var_dir_id,
+        .content_hash = content_hash,
+    };
+    auto result = add_file_node(ctx.state->graph, std::move(node));
+    if (!result) {
+        return std::nullopt;
+    }
+    state.imported_env_var_nodes.insert(var_name_id, *result);
+    return *result;
+}
+
 auto process_import(
     BuilderContext& ctx,
     Builder& state,
@@ -1154,41 +1213,14 @@ auto process_import(
 
     auto value_sv = str(value_id);
 
-    auto var_name_id = to_underlying(intern(var_name_sv));
-    if (state.env_var_dir_id != INVALID_NODE_ID) {
-        auto node_name_buf = Buf {};
-        node_name_buf += var_name_sv;
-        node_name_buf += '=';
-        node_name_buf += value_sv;
-        auto content_hash = sha256(value_sv);
-
-        auto const* existing_node_id = state.imported_env_var_nodes.find(var_name_id);
-        auto const name_id = intern(node_name_buf.view());
-        if (existing_node_id) {
-            auto* existing = get_file_node(ctx.state->graph, *existing_node_id);
-            if (existing && existing->name != name_id) {
-                existing->name = name_id;
-                existing->content_hash = content_hash;
-            }
-        } else {
-            auto node = FileNode {
-                .type = NodeType::Variable,
-                .name = name_id,
-                .parent_dir = state.env_var_dir_id,
-                .content_hash = content_hash,
-            };
-            auto result = add_file_node(ctx.state->graph, std::move(node));
-            if (result) {
-                state.imported_env_var_nodes.insert(var_name_id, *result);
-            }
-        }
-    }
+    (void)ensure_env_var_node(ctx, state, var_name_sv, value_sv);
 
     if (ctx.vars) {
         ctx.vars->set(var_name_sv, value_sv);
     }
 
     // Track this as an imported variable for fine-grained dependency tracking
+    auto var_name_id = to_underlying(intern(var_name_sv));
     state.imported_var_names.insert(var_name_id);
 
     return {};
@@ -1348,26 +1380,39 @@ auto process_conditional(
     parser::Conditional const& cond
 ) -> Result<void>
 {
-    // Save and clear used_config_vars to capture which vars the condition uses
+    // Save and clear used_*_vars to capture which vars the condition uses.
+    // Env vars are tracked symmetrically with config vars so that an env-driven
+    // guard flip (e.g. `ifeq ($(TUP_PLATFORM),win32)`) folds into the identity of
+    // every command in the conditional block.
     auto saved_config_vars = std::move(ctx.used_config_vars);
+    auto saved_used_env_vars = std::move(ctx.used_env_vars);
 
-    // Evaluate condition value - this may use config vars like @(MODE)
+    // Evaluate condition value - this may use config vars like @(MODE) or env vars
     auto condition_true = parser::evaluate_condition(*ctx.eval, cond);
 
-    // Capture config vars used in the condition expression
+    // Capture vars used in the condition expression
     auto condition_vars = std::move(ctx.used_config_vars);
+    auto condition_env_vars_used = std::move(ctx.used_env_vars);
     ctx.used_config_vars = std::move(saved_config_vars);
+    ctx.used_env_vars = std::move(saved_used_env_vars);
 
-    // Save condition_config_vars, then merge in condition-specific vars
+    // Save condition_*_vars, then merge in condition-specific vars
     auto saved_condition_vars = std::move(ctx.condition_config_vars);
+    auto saved_condition_env_vars = std::move(ctx.condition_env_vars);
     // Rebuild: copy saved entries + merge condition_vars
     auto const* d = saved_condition_vars.data();
     for (std::size_t i = 0, n = saved_condition_vars.size(); i < n; ++i) {
         ctx.condition_config_vars.insert(d[i]);
     }
     ctx.condition_config_vars.merge_from(condition_vars);
+    auto const* de = saved_condition_env_vars.data();
+    for (std::size_t i = 0, n = saved_condition_env_vars.size(); i < n; ++i) {
+        ctx.condition_env_vars.insert(de[i]);
+    }
+    ctx.condition_env_vars.merge_from(condition_env_vars_used);
     auto restore_condition_vars = ScopeGuard([&] {
         ctx.condition_config_vars = std::move(saved_condition_vars);
+        ctx.condition_env_vars = std::move(saved_condition_env_vars);
     });
 
     // Create condition node for phi-node model
@@ -2136,10 +2181,18 @@ auto add_tupfile(
         ctx.used_config_vars.insert(to_underlying(intern(name)));
     };
 
-    // Set up callback to track which imported env variables are used during expansion
+    // Set up callback to track which imported env variables are used during expansion.
+    // Also ensure an env Variable node exists for any env var that resolves from the
+    // environment, so TUP_PLATFORM/TUP_ARCH (auto-resolved, not via `import`) get the
+    // same tracking node as explicitly imported vars and fold into command identity.
     eval.imported_vars = &state.imported_var_names;
-    eval.on_env_var_used = [&ctx](std::string_view name) {
+    eval.on_env_var_used = [&ctx, &state](std::string_view name) {
         ctx.used_env_vars.insert(to_underlying(intern(name)));
+        auto name_buf = Buf {};
+        name_buf += name;
+        if (auto const* env_val = std::getenv(name_buf.c_str())) {
+            (void)ensure_env_var_node(ctx, state, name, std::string_view { env_val });
+        }
     };
 
     // Wire up transitive dependency trackers for variable tracking

test/e2e/fixtures/platform_conditional_incremental/Tupfile.fixture

@@ -0,0 +1,9 @@
+# Both branches produce the same output file with distinct content. The platform
+# string ("win32"/"linux") does NOT appear in either command's text, so the only
+# way an env-driven branch flip can be detected is via a sticky edge from the
+# env Variable node to the guarded commands (i.e. the condition_env_vars path).
+ifeq ($(TUP_PLATFORM),win32)
+: |> echo WINBUILD > %o |> result.txt
+else
+: |> echo NIXBUILD > %o |> result.txt
+endif

test/unit/test_e2e.cpp

@@ -3930,6 +3930,43 @@ SCENARIO("TUP_PLATFORM env var controls platform conditionals", "[e2e][platform]
     }
 }
 
+SCENARIO("Env var change in a conditional rebuilds the affected branch",
+    "[e2e][platform][incremental][platform-incremental]")
+{
+    // Both branches of `ifeq ($(TUP_PLATFORM),...)` produce the same output file with
+    // distinct content; the platform string does not appear in either command's text.
+    // So the only signal that an env-driven branch flip occurred is a sticky edge from
+    // the env Variable node to the guarded commands (the condition_env_vars path,
+    // symmetric to condition_config_vars). Without it, change detection sees no changed
+    // file and no changed identity, reports "Nothing to do", and the output stays stale.
+    GIVEN("a project whose active branch is selected by an env-sourced $(TUP_PLATFORM) condition")
+    {
+        auto f = E2EFixture { "platform_conditional_incremental" };
+        {
+            auto env = EnvGuard { "TUP_PLATFORM", "win32" };
+            REQUIRE(f.init().success());
+            REQUIRE(f.build().success());
+            REQUIRE(f.read_file("result.txt") == "WINBUILD\n");
+            REQUIRE(f.build().is_noop()); // stable under no change
+        }
+
+        WHEN("the env var flips, selecting the other branch")
+        {
+            auto env = EnvGuard { "TUP_PLATFORM", "linux" };
+            auto result = f.build();
+
+            THEN("the rebuild runs the now-active branch and the output updates")
+            {
+                INFO("stdout: " << result.stdout_output);
+                INFO("stderr: " << result.stderr_output);
+                REQUIRE(result.success());
+                REQUIRE_FALSE(result.is_noop());
+                REQUIRE(f.read_file("result.txt") == "NIXBUILD\n");
+            }
+        }
+    }
+}
+
 SCENARIO("Imported env vars persist across builds", "[e2e][import]")
 {
     GIVEN("a project with import directive")