From ce980895b25535d654e194008b57cdbb2cae9961 Mon Sep 17 00:00:00 2001 From: Mac Malone Date: Sat, 31 Jan 2026 14:17:26 -0500 Subject: [PATCH] fix: `IO.Process.spawn` empty env var on Windows (#12220) This PR fixes a bug on Windows with `IO.Process.spawn` where setting an environment variable to the empty string would not set the environment variable on the subprocess. --- src/runtime/process.cpp | 11 ++++++----- tests/lake/tests/env/test.sh | 4 +--- tests/lean/run/emptyEnvVar.lean | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 tests/lean/run/emptyEnvVar.lean diff --git a/src/runtime/process.cpp b/src/runtime/process.cpp index b0a7ed0b9f..d0ef167b4a 100644 --- a/src/runtime/process.cpp +++ b/src/runtime/process.cpp @@ -235,9 +235,9 @@ static obj_res spawn(string_ref const & proc_name, array_ref const & char *new_envp = new_env.get(); if (env.size()) { - std::unordered_map new_env_vars; // C++17 gives us no-copy std::string_view for this, much better! + std::unordered_map> new_env_vars; // C++17 gives us no-copy std::string_view for this, much better! for (auto & entry : env) { - new_env_vars[entry.fst().data()] = entry.snd() ? entry.snd().get()->data() : std::string{}; + new_env_vars[entry.fst().data()] = entry.snd(); } // First copy old evars not in new evars. @@ -257,12 +257,13 @@ static obj_res spawn(string_ref const & proc_name, array_ref const & // Then copy new evars if nonempty for(const auto & ev : new_env_vars) { - if (ev.second.empty()) continue; + if (!ev.second) continue; + std::string val = ev.second.get()->data(); // Check if the destination buffer has enough room. - if (new_envp + ev.first.length() + 1 + ev.second.length() + 1 > new_env.get() + env_buf_size - 1) break; + if (new_envp + ev.first.length() + 1 + val.length() + 1 > new_env.get() + env_buf_size - 1) break; new_envp = std::copy(ev.first.cbegin(), ev.first.cend(), new_envp); *new_envp++ = '='; - new_envp = std::copy(ev.second.cbegin(), ev.second.cend(), new_envp); + new_envp = std::copy(val.cbegin(), val.cend(), new_envp); *new_envp++ = '\0'; } } diff --git a/tests/lake/tests/env/test.sh b/tests/lake/tests/env/test.sh index 98c1cee39a..e369d2da2f 100755 --- a/tests/lake/tests/env/test.sh +++ b/tests/lake/tests/env/test.sh @@ -45,9 +45,7 @@ LEAN_CC=foo test_eq "foo" env printenv LEAN_CC # Test `LAKE_ARTIFACT_CACHE` setting and default LAKE_ARTIFACT_CACHE=true test_eq "true" env printenv LAKE_ARTIFACT_CACHE LAKE_ARTIFACT_CACHE=false test_eq "false" env printenv LAKE_ARTIFACT_CACHE -# FIXME: Currently fails on Windows due to a platform inconsistency in how -# Lean configures the environments of spawned processes -# LAKE_ARTIFACT_CACHE= test_eq "" env printenv LAKE_ARTIFACT_CACHE +LAKE_ARTIFACT_CACHE= test_eq "" env printenv LAKE_ARTIFACT_CACHE LAKE_ARTIFACT_CACHE= test_eq "false" -d ../../examples/hello env printenv LAKE_ARTIFACT_CACHE LAKE_ARTIFACT_CACHE= test_eq "true" -f enableArtifactCache.toml env printenv LAKE_ARTIFACT_CACHE test_cmd rm lake-manifest.json diff --git a/tests/lean/run/emptyEnvVar.lean b/tests/lean/run/emptyEnvVar.lean new file mode 100644 index 0000000000..a368efb0d5 --- /dev/null +++ b/tests/lean/run/emptyEnvVar.lean @@ -0,0 +1,18 @@ +/-- +Tests that spawning a process with a environment variable configured to the +empty string correctly sets the environment variable on the subprocess on +all platforms. Previously, this was broken on Windows. +-/ +def test : IO String := do + let var := "TEST_VAR" + let out ← IO.Process.output { + cmd := "printenv" + args := #[var] + env := #[(var, some "")] + } + unless out.exitCode == 0 do + throw <| .userError "environment variable not set" + return out.stdout.trimAsciiEnd.copy -- trim ending newline + +/-- info: "" -/ +#guard_msgs in #eval test