fix(xlings): the env list separator is the platform's, not a literal ':'

Windows CI caught this; no amount of reading would have. resolve_env joined
and split its lists on ':', which on Windows is both the wrong separator (it
is ';') and a character that appears INSIDE every absolute path. So
de-duplication split "C:\x" into "C" and "\x", matched nothing, and the
joined value came back as "C:\...\x:C:\...\x" -- a list that grows on every
nested invocation and that no consumer can parse.

This repository has made the same mistake before, in the other direction:
find_first_of(";:") over a Windows PATH cuts at the drive-letter colon.

The three test expectations that failed were also wrong, but differently, and
the difference matters: they compared against a path JOIN while the code does
a literal substitution. The literal one is correct -- the separator inside a
declaration belongs to the subos manifest, and rewriting it to the host's
spelling would be editing a value we do not own. The assertions now say so,
and the dedup test additionally asserts the result contains NO separator at
all, which is the property that actually failed.

Author: speak-agent

src/xlings/subos_info.cppm

@@ -37,6 +37,7 @@ export module mcpp.xlings.subos_info;
 
 import std;
 import mcpp.libs.json;
+import mcpp.platform;
 
 export namespace mcpp::xlings::subos {
 
@@ -177,10 +178,17 @@ Info read(const std::filesystem::path& subosDir) {
 // Resolve the declarations into concrete (var, value) pairs with
 // `${subosdir}` expanded.
 //
-// `prepend` joins with ':' in provider order and de-duplicates; `set`
-// replaces. That is xlings's own precedence, and the de-duplication matters
-// because these variables are inherited: without it a nested invocation
-// grows the list every time.
+// `prepend` joins in provider order and de-duplicates; `set` replaces. That
+// is xlings's own precedence, and the de-duplication matters because these
+// variables are inherited: without it a nested invocation grows the list
+// every time.
+//
+// The separator is the PLATFORM's, never a literal ':'. Hardcoding one is a
+// mistake this repository has made before and it is not cosmetic: on Windows
+// the list separator is ';' and ':' appears INSIDE every absolute path, so a
+// ':'-keyed split cuts "C:\\x" into "C" and "\\x" -- the de-duplication then
+// 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) {
     std::vector<std::pair<std::string, std::string>> out;
@@ -194,17 +202,19 @@ resolve_env(const Info& info, const std::filesystem::path& subosDir) {
         return v;
     };
 
-    // A colon-separated list already contains `value` as a whole element?
-    // Compared element-wise rather than by substring: a plain `find` would
-    // consider "/a/bc" already present in "/a/bcd".
-    auto contains_element = [](std::string_view list, std::string_view value) {
+    const std::string sep = mcpp::platform::env::path_list_separator();
+
+    // Does the list already contain `value` as a WHOLE element? Compared
+    // element-wise rather than by substring: a plain `find` would consider
+    // "/a/bc" already present in "/a/bcd".
+    auto contains_element = [&](std::string_view list, std::string_view value) {
         for (std::size_t i = 0; i <= list.size();) {
-            auto end = list.find(':', i);
+            auto end = list.find(sep, i);
             auto piece = list.substr(i, end == std::string_view::npos
                                             ? std::string_view::npos : end - i);
             if (piece == value) return true;
             if (end == std::string_view::npos) break;
-            i = end + 1;
+            i = end + sep.size();
         }
         return false;
     };
@@ -217,7 +227,7 @@ resolve_env(const Info& info, const std::filesystem::path& subosDir) {
             if (hit == out.end()) { out.emplace_back(d.var, value); continue; }
             if (d.op == "set") { hit->second = value; continue; }
             if (!contains_element(hit->second, value))
-                hit->second = value + ":" + hit->second;
+                hit->second = value + sep + hit->second;
         }
     }
     return out;

tests/unit/test_subos_info.cpp

@@ -10,6 +10,7 @@
 #include <gtest/gtest.h>
 
 import std;
+import mcpp.platform;
 import mcpp.xlings.subos_info;
 
 namespace su = mcpp::xlings::subos;
@@ -68,7 +69,11 @@ TEST(SubosInfo, ResolvesSubosdirPlaceholder) {
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
     EXPECT_EQ(env[0].first, "LIBGL_DRIVERS_PATH");
-    EXPECT_EQ(env[0].second, (t.dir / "usr" / "lib" / "dri").string());
+    // Literal concatenation, NOT a path join. The separator in the declaration
+    // belongs to the subos manifest and is substituted verbatim; turning it
+    // into the host's would rewrite a value we do not own. On Windows the two
+    // spellings differ and the path-join form is the wrong expectation.
+    EXPECT_EQ(env[0].second, t.dir.string() + "/usr/lib/dri");
 }
 
 // Several providers may contribute to one variable — that is the normal
@@ -83,10 +88,10 @@ TEST(SubosInfo, PrependJoinsProvidersInOrder) {
         {"var":"V","op":"prepend","value":"${subosdir}/two"}]}]}})");
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
-    auto one = (t.dir / "one").string();
-    auto two = (t.dir / "two").string();
-    EXPECT_NE(env[0].second.find(one), std::string::npos);
-    EXPECT_NE(env[0].second.find(two), std::string::npos);
+    const auto sep = mcpp::platform::env::path_list_separator();
+    EXPECT_EQ(env[0].second,
+              t.dir.string() + "/two" + sep + t.dir.string() + "/one")
+        << "both providers must survive, later binding front-most";
 }
 
 // The same value arriving twice must not accumulate: nested invocations
@@ -98,7 +103,12 @@ TEST(SubosInfo, PrependDeduplicates) {
       {"binding":"b@1","decls":[{"var":"V","op":"prepend","value":"${subosdir}/x"}]}]}})");
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
-    EXPECT_EQ(env[0].second, (t.dir / "x").string());
+    // One entry, not two. The de-duplication has to split on the PLATFORM's
+    // list separator: keyed on ':' it would cut "C:\\x" apart on Windows,
+    // match nothing, and grow the list on every nested invocation.
+    EXPECT_EQ(env[0].second, t.dir.string() + "/x");
+    EXPECT_EQ(env[0].second.find(mcpp::platform::env::path_list_separator()),
+              std::string::npos);
 }
 
 // `set` replaces rather than joins — xlings's own precedence.