fix(subos): prepend 丢掉了调用方的值 (#383)

解析出的键值对**替换**子进程里的变量(作为 extraEnv 进去),而 `prepend` 只发出声明值,把调用方原有内容整个丢掉 —— 对 PATH 形状的变量,丢掉的是用户的整条搜索路径。

现在它接在调用方的值前面,且对已存在元素幂等(嵌套运行不会让列表增长)。空值不算「用户设过」。

`set` 的部分**已撤回**:`set` 与「默认值」是两种意图,而 op 词汇表是 xlings 的契约。#382 想要的逃生通道应由 xlings 新增一个 op,mcpp 认它 —— 且因为 mcpp 会丢弃未知 op,那个 op 必须两侧同时到位。已加断言把「`set` 就是要赢」钉住。

Author: speak-agent

src/build/execute.cppm

@@ -361,7 +361,15 @@ compute_subos_env(const mcpp::build::BuildPlan& plan) {
     // diagnostic path in doctor.
     if (!info.note.empty())
         mcpp::log::verbose("subos", info.note);
-    return mcpp::xlings::subos::resolve_env(info, dir);
+    // Resolved AGAINST the caller's environment, not in a vacuum: these
+    // entries replace the variable in the child, so a `set` that ignores an
+    // exported value overwrites it and a `prepend` that ignores it drops it.
+    return mcpp::xlings::subos::resolve_env(
+        info, dir, [](std::string_view v) -> std::optional<std::string> {
+            if (const char* e = std::getenv(std::string(v).c_str()))
+                return std::string(e);
+            return std::nullopt;
+        });
 }
 
 // Compile a prepared BuildContext. Shared between `mcpp build` and `mcpp run`
@@ -877,7 +885,13 @@ std::optional<int> try_fast_run(const std::filesystem::path& projectRoot,
         auto subosDir = subos_dir_for_run(std::filesystem::path(match->subosDir));
         if (!subosDir.empty()) {
             auto info = mcpp::xlings::subos::read(subosDir);
-            for (auto& kv : mcpp::xlings::subos::resolve_env(info, subosDir))
+            for (auto& kv : mcpp::xlings::subos::resolve_env(
+                     info, subosDir,
+                     [](std::string_view v) -> std::optional<std::string> {
+                         if (const char* e = std::getenv(std::string(v).c_str()))
+                             return std::string(e);
+                         return std::nullopt;
+                     }))
                 childEnv.push_back(std::move(kv));
         }
     }

src/xlings/subos_info.cppm

@@ -201,9 +201,21 @@ Info read(const std::filesystem::path& subosDir) {
 // never matches and the joined value is a corrupt list. Caught by CI on
 // Windows, not by any amount of reading.
 std::vector<std::pair<std::string, std::string>>
-resolve_env(const Info& info, const std::filesystem::path& subosDir) {
+resolve_env(const Info& info, const std::filesystem::path& subosDir,
+            const std::function<std::optional<std::string>(std::string_view)>&
+                ambient = {}) {
     std::vector<std::pair<std::string, std::string>> out;
 
+    // What the caller's environment already says about a variable.
+    //
+    // The declarations are merged against this, not in a vacuum, because the
+    // result REPLACES the variable in the child (it goes in as extraEnv). A
+    // resolution that ignores the ambient value silently discards it.
+    auto ambient_of = [&](std::string_view var) -> std::optional<std::string> {
+        if (!ambient) return std::nullopt;
+        return ambient(var);
+    };
+
     const std::string subos = subosDir.string();
     auto expand = [&](std::string v) {
         constexpr std::string_view kPh = "${subosdir}";
@@ -241,7 +253,45 @@ resolve_env(const Info& info, const std::filesystem::path& subosDir) {
             std::pair<std::string, std::string>* hit = nullptr;
             for (auto& kv : out)
                 if (kv.first == d.var) { hit = &kv; break; }
-            if (!hit) { out.emplace_back(d.var, value); continue; }
+            if (!hit) {
+                auto amb = ambient_of(d.var);
+                if (d.op == "set") {
+                    // `set` wins, ambient or not.
+                    //
+                    // Deliberately NOT "yield to an exported value". That
+                    // reading was written here first and withdrawn: `set` and
+                    // "default" are two different intentions, and a subos has
+                    // real need of the first -- a variable naming its own
+                    // loader configuration must not be overridable by a stale
+                    // value in the caller's shell. Collapsing them here would
+                    // remove the ability to express it.
+                    //
+                    // It is also not mcpp's vocabulary to redefine. `envs` is
+                    // xlings' wire format; a consumer that quietly gives an op
+                    // a second meaning makes the same subos behave differently
+                    // depending on which tool launched the program.
+                    //
+                    // The escape hatch mcpp#382 asks for (a recipe declaring a
+                    // DEFAULT the user can override) therefore wants a new op
+                    // from xlings, not a reinterpretation of this one. When it
+                    // exists, it is honoured here -- unknown ops are dropped
+                    // today, which is why it has to arrive on both sides.
+                    out.emplace_back(d.var, value);
+                    continue;
+                }
+                // `prepend` against the ambient value, not instead of it.
+                // These entries replace the variable in the child, so
+                // emitting the declared value alone DROPS whatever the caller
+                // had -- for a PATH-shaped variable that is the user's whole
+                // search path.
+                if (amb && !amb->empty() && !contains_element(*amb, value))
+                    out.emplace_back(d.var, value + sep + *amb);
+                else if (amb && !amb->empty())
+                    out.emplace_back(d.var, *amb);
+                else
+                    out.emplace_back(d.var, value);
+                continue;
+            }
             if (d.op == "set") { hit->second = value; continue; }
             if (!contains_element(hit->second, value))
                 hit->second = value + sep + hit->second;

tests/unit/test_subos_info.cpp

@@ -249,4 +249,65 @@ TEST(SubosInfo, UnknownOpIsDroppedLikeXlingsDrops) {
     EXPECT_EQ(env[0].second, "/yes");
 }
 
+
+
+
+// `prepend` prepends TO the caller's value rather than replacing it. Same
+// reason: the pair replaces the variable, so emitting the declared value alone
+// discards whatever search path the user had.
+TEST(SubosResolveEnv, PrependKeepsTheExportedValue) {
+    Tmp t;
+    t.write(R"({"workspace":{},"subos_info":{"schema_version":1,"runtime":"glibc@2.39",
+        "envs":{"glibc@2.39":[{"var":"LD_LIBRARY_PATH","op":"prepend","value":"/sub/lib"}]}}})");
+    auto info = su::read(t.dir);
+    auto out = su::resolve_env(
+        info, t.dir, [](std::string_view v) -> std::optional<std::string> {
+            if (v == "LD_LIBRARY_PATH") return std::string("/user/lib");
+            return std::nullopt;
+        });
+    ASSERT_EQ(out.size(), 1u);
+    const auto sep = mcpp::platform::env::path_list_separator();
+    EXPECT_EQ(out[0].second, std::string("/sub/lib") + sep + "/user/lib");
+}
+
+// Already there: prepending again would grow the list on every nested run.
+TEST(SubosResolveEnv, PrependIsIdempotentAgainstTheExportedValue) {
+    Tmp t;
+    t.write(R"({"workspace":{},"subos_info":{"schema_version":1,"runtime":"glibc@2.39",
+        "envs":{"glibc@2.39":[{"var":"LD_LIBRARY_PATH","op":"prepend","value":"/sub/lib"}]}}})");
+    auto info = su::read(t.dir);
+    const auto sep = mcpp::platform::env::path_list_separator();
+    const auto existing = std::string("/sub/lib") + sep + "/user/lib";
+    auto out = su::resolve_env(
+        info, t.dir, [&](std::string_view v) -> std::optional<std::string> {
+            if (v == "LD_LIBRARY_PATH") return existing;
+            return std::nullopt;
+        });
+    ASSERT_EQ(out.size(), 1u);
+    EXPECT_EQ(out[0].second, existing);
+}
+
+// `set` wins over an exported value, and that is deliberate.
+//
+// This assertion exists to stop the opposite reading from being reintroduced
+// -- it was, once, as a fix for mcpp#382, and withdrawn: `set` and "a default
+// the user may override" are two intentions, and a subos needs the first for
+// variables naming its own configuration. The escape hatch that issue wants is
+// a NEW op from xlings, whose wire format this is, not a second meaning for
+// this one applied by one consumer.
+TEST(SubosResolveEnv, SetWinsOverAnExportedValue) {
+    Tmp t;
+    t.write(R"({"workspace":{},"subos_info":{"schema_version":1,
+        "runtime":"glibc@2.39",
+        "envs":{"glibc@2.39":[{"var":"GALLIUM_DRIVER","op":"set","value":"d3d12"}]}}})");
+    auto info = su::read(t.dir);
+    auto out = su::resolve_env(
+        info, t.dir, [](std::string_view v) -> std::optional<std::string> {
+            if (v == "GALLIUM_DRIVER") return std::string("llvmpipe");
+            return std::nullopt;
+        });
+    ASSERT_EQ(out.size(), 1u);
+    EXPECT_EQ(out[0].second, "d3d12");
+}
+
 }  // namespace