From d1a8f4626060c093e1e164bc1f8cb700035b87e7 Mon Sep 17 00:00:00 2001
From: Mura Li <2606021+typeless@users.noreply.github.com>
Date: Thu, 28 May 2026 15:02:02 +0800
Subject: [PATCH] Make `import VAR ?= default` revert to default when env-var
 is unset

process_import resolved env-var values in the order env -> cache -> default. The cache was populated unconditionally by ensure_env_var_node on every build, so once a value entered the cache it recycled through every subsequent build and the default branch was unreachable. The user had no way to revert to the written default by unsetting the env var; only putup clean (or hand-editing the index) dropped the cached value.

Swap to env -> default -> cache. When the author wrote `?= default`, that default is the explicit revert-target for the env-unset case and now wins over the cache. Plain `import VAR` without a default still falls through to the cache, so the cross-shell stability that motivated the cache in the first place (build with MY_VAR=foo, open a fresh shell that forgot the export, rebuild without surprise) is preserved.

Reproducer: [e2e][import] test "import ?= default reverts when env is unset on warm index" - sets MY_VAR=from_env, builds, drops the EnvGuard, rebuilds, asserts out.txt reads "default_value" not the cached "from_env". Red before, green after. Full suite green (467 cases, 23679 assertions); [phi], [platform-incremental], [envdep] untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/graph/builder.cpp  | 21 ++++++++++++---------
 test/unit/test_e2e.cpp | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp
index aae31b8..0676fbf 100644
--- a/src/graph/builder.cpp
+++ b/src/graph/builder.cpp
@@ -1192,7 +1192,18 @@ auto process_import(
     if (auto const* env_val = std::getenv(name_buf.c_str())) {
         value_id = intern(env_val);
     }
-    // 2. Fall back to cached value from previous build (passed via options)
+    // 2. If the author wrote `?= default`, that default is the explicit
+    //    revert-target for the env-unset case and wins over the cache.
+    else if (imp.default_value) {
+        auto expanded = parser::expand(*ctx.eval, *imp.default_value);
+        if (!expanded) {
+            return pup::unexpected<Error>(expanded.error());
+        }
+        value_id = *expanded;
+    }
+    // 3. No env, no default — fall back to the cached value from the previous
+    //    build so plain `import VAR` stays stable across shell sessions that
+    //    forget to re-export the variable.
     else if (auto it = std::lower_bound(
                  state.options.cached_env_vars.begin(),
                  state.options.cached_env_vars.end(),
@@ -1202,14 +1213,6 @@ auto process_import(
              it != state.options.cached_env_vars.end() && str(it->first) == var_name_sv) {
         value_id = it->second;
     }
-    // 3. Fall back to default value
-    else if (imp.default_value) {
-        auto expanded = parser::expand(*ctx.eval, *imp.default_value);
-        if (!expanded) {
-            return pup::unexpected<Error>(expanded.error());
-        }
-        value_id = *expanded;
-    }
 
     auto value_sv = str(value_id);
 
diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp
index 740648f..dd78ce6 100644
--- a/test/unit/test_e2e.cpp
+++ b/test/unit/test_e2e.cpp
@@ -4343,6 +4343,39 @@ SCENARIO("import with ?= operator", "[e2e][import]")
     }
 }
 
+SCENARIO("import ?= default reverts when env is unset on warm index", "[e2e][import]")
+{
+    GIVEN("a Tupfile with import VAR ?= default, after a build that captured the env value")
+    {
+        auto f = E2EFixture { "import_soft_set" };
+
+        {
+            auto env = EnvGuard { "MY_VAR", "from_env" };
+            REQUIRE(f.init().success());
+            auto build1 = f.build();
+            REQUIRE(build1.success());
+            REQUIRE(f.read_file("out.txt") == "from_env\n");
+        }
+
+        WHEN("the env var is unset and the project is rebuilt")
+        {
+            auto build2 = f.build();
+
+            THEN("build succeeds")
+            {
+                INFO("stdout: " << build2.stdout_output);
+                INFO("stderr: " << build2.stderr_output);
+                REQUIRE(build2.success());
+            }
+
+            THEN("the default value is restored, not the cached env value")
+            {
+                REQUIRE(f.read_file("out.txt") == "default_value\n");
+            }
+        }
+    }
+}
+
 // =============================================================================
 // Error Handling Tests
 // =============================================================================