Cleanup minor environment variable issues (#10464)

* [python] Ensure putEnv properly removes variables

Requires os.unsetenv extern

* [tests] Add extra testing for removing env variables

Previously the tests did not ensure that the variables would
be updated in child processes

* [python] Fix Sys.putEnv for python 3.8 and below

Add a try catch block so that this function doesn't throw when targeting
python versions prior to 3.9, as before that `os.unsetenv` is not
guaranteed to exist

* Document putEnv(var, null) limitation on python

* [python] Remove internal environment variable map

Fixes #10469

Sys.putEnv() now modifies python's `os.environ` dictionary rather than
using `os.putenv` or `os.unsetenv` and modifying an internal map. This
is a better choice as modifying the dictionary calls these functions
automatically if they exist.
This also means that changes made via the Haxe api will be visible also
via python's `os.environ`.

As a result, Sys no longer needs to store an internal map of environment
variables. As of #10460, a copy is made of the map
anyway everytime `Sys.environment()` is called. This means we might as
well directly loop through python's `os.environ` which takes into
account modifications made via the python
api, rather than looping through an internal copy which does not, and
takes the same amout of time.

* [cs] Remove internal copy of environment variables

Fixes #10469

Now, modifications to the environment directly via the
native C# api will be visible in 'Sys.environment()`

* [tests] Update sys tests accordingly

* [python] Allow removing non-existent variable

* [tests] Test removal of non-existent variable

* [python] Remove warning about putEnv(name, null)

This should not be an issue now that Sys.putEnv modifies `os.environ`
directly

* [tests] Only test uppercase environment variables

For compatibility with python

* [tests] Test getEnv case-insensitivity on windows

* [php] Remove incorrect environment variables

Fixes #10471

On windows, ensure only correct and up to date environment variables are
present in `Sys.environment()`

* [sys] Document Sys.environment() capitalization

std/Sys.hx

@@ -66,6 +66,15 @@ extern class Sys {
 	/**
 		Returns a map of the current environment variables and their values
 		as of the invocation of the function.
+
+		(python) On Windows, the variable names are always in upper case.
+
+		(cpp)(hl)(neko) On Windows, the variable names match the last capitalization used when modifying
+		the variable if the variable has been modified, otherwise they match their capitalization at
+		the start of the process.
+
+		On Windows on remaining targets, variable name capitalization matches however they were capitalized
+		at the start of the process or at the moment of their creation.
 	**/
 	static function environment():Map<String, String>;
 

std/cs/_std/Sys.hx

@@ -26,7 +26,6 @@ import cs.system.threading.Thread;
 
 @:coreApi
 class Sys {
-	private static var _env:haxe.ds.StringMap<String>;
 	private static var _args:Array<String>;
 
 	public static inline function print(v:Dynamic):Void {
@@ -52,25 +51,15 @@ class Sys {
 
 	public static function putEnv(s:String, v:Null<String>):Void {
 		Environment.SetEnvironmentVariable(s, v);
-		if (_env == null)
-			return;
-
-		if (v == null)
-			_env.remove(s);
-		else
-			_env.set(s, v);
 	}
 
 	public static function environment():Map<String, String> {
-		if (_env == null) {
-			var e = _env = new haxe.ds.StringMap();
-			var nenv = Environment.GetEnvironmentVariables().GetEnumerator();
-			while (nenv.MoveNext()) {
-				e.set(nenv.Key, nenv.Value);
-			}
+		final env = new haxe.ds.StringMap();
+		final nenv = Environment.GetEnvironmentVariables().GetEnumerator();
+		while (nenv.MoveNext()) {
+			env.set(nenv.Key, nenv.Value);
 		}
-
-		return _env.copy();
+		return env;
 	}
 
 	public static inline function sleep(seconds:Float):Void {

std/php/_std/Sys.hx

@@ -29,6 +29,43 @@ import haxe.SysTools;
 	/** Environment variables set by `Sys.putEnv()` */
 	static var customEnvVars = new NativeAssocArray<String>();
 
+	// we need to keep track of capitalization for windows
+	static final isWindows = systemName() == "Windows";
+
+	static var envCapitalization(get, null):Map<String, String>;
+
+	static function get_envCapitalization():Map<String, String> {
+		if (envCapitalization == null)
+			return envCapitalization = [for (k => _ in SuperGlobal._SERVER) k.toUpperCase() => k];
+		return envCapitalization;
+	}
+
+	static inline function addCustom(name:String, value:String):Void {
+		customEnvVars[name] = value;
+		if (!isWindows)
+			return;
+		final upperCase = name.toUpperCase();
+		if (envCapitalization.exists(upperCase))
+			return;
+		envCapitalization[upperCase] = name;
+	}
+
+	static inline function removeCustom(name:String):Void {
+		Global.unset(customEnvVars[name]);
+		if (!isWindows)
+			return;
+		envCapitalization.remove(name.toUpperCase());
+	}
+
+	static inline function getCapitalization(name:String):String {
+		if (!isWindows)
+			return name;
+		final existing = envCapitalization[name.toUpperCase()];
+		if (existing != null)
+			return existing;
+		return name;
+	}
+
 	public static inline function print(v:Dynamic):Void {
 		Global.echo(Std.string(v));
 	}
@@ -52,10 +89,10 @@ import haxe.SysTools;
 
 	public static function putEnv(s:String, v:Null<String>):Void {
 		if (v == null) {
-			Global.unset(customEnvVars[s]);
+			removeCustom(s);
 			Global.putenv('$s');
 		} else {
-			customEnvVars[s] = '$v'; // in case of `null` it should become `"null"`
+			addCustom(s, v);
 			Global.putenv('$s=$v');
 		}
 	}
@@ -129,7 +166,8 @@ import haxe.SysTools;
 	public static function environment():Map<String, String> {
 		var env = SuperGlobal._SERVER;
 		Syntax.foreach(customEnvVars, function(name:String, value:String) {
-			env[name] = value;
+			final actualName = getCapitalization(name);
+			env[actualName] = value;
 		});
 		return php.Lib.hashOfAssociativeArray(env);
 	}

std/python/_std/Sys.hx

@@ -28,20 +28,6 @@ import haxe.ds.StringMap;
 
 @:coreApi
 class Sys {
-	static var environ(get,default):StringMap<String>;
-	static function get_environ():StringMap<String> {
-		return switch environ {
-			case null:
-				var environ = new StringMap();
-				var env = Os.environ;
-				for (key in env.keys()) {
-					environ.set(key, env.get(key, null));
-				}
-				Sys.environ = environ;
-			case env: env;
-		}
-	}
-
 	public static inline function time():Float {
 		return Time.time();
 	}
@@ -64,20 +50,28 @@ class Sys {
 	}
 
 	public static function getEnv(s:String):String {
-		return environ.get(s);
+		return Os.environ.get(s, null);
 	}
 
 	public static function putEnv(s:String, v:Null<String>):Void {
 		if (v == null) {
-			environ.remove(s);
+			try {
+				Os.environ.remove(s);
+			} catch(e:python.Exceptions.KeyError) {
+				// the variable didn't exist
+			}
 			return;
 		}
-		python.lib.Os.putenv(s, v);
-		environ.set(s, v);
+		Os.environ.set(s, v);
 	}
 
 	public static function environment():Map<String, String> {
-		return environ.copy();
+		final environ = new StringMap();
+		final env = Os.environ;
+		for (key in env.keys()) {
+			environ.set(key, env.get(key, null));
+		}
+		return environ;
 	}
 
 	public static function sleep(seconds:Float):Void {

std/python/lib/Os.hx

@@ -57,6 +57,12 @@ extern class Os {
 
 	static function putenv(name:String, value:String):Void;
 
+	/** Removes the value for the environment variable `name`.
+
+		When targeting python versions prior to 3.9, this function may not exist on some platforms.
+	 **/
+	static function unsetenv(name:String):Void;
+
 	static function chdir(path:String):Void;
 
 	static function unlink(path:String):Void;

tests/sys/src/TestSys.hx

@@ -29,27 +29,73 @@ class TestSys extends TestCommandBase {
 		// environment should not update if env updates
 		env.set(toUpdate, "2");
 		Assert.equals("1", Sys.getEnv(toUpdate));
+
+		// variables set via target specific api should exist
+		#if (cs || python)
+		final toSetNatively = "SET_NATIVELY";
+		#if cs
+		cs.system.Environment.SetEnvironmentVariable(toSetNatively, "1");
+		#elseif python
+		python.lib.Os.environ.set(toSetNatively, "1");
+		#end
+		Assert.equals("1", Sys.environment()[toSetNatively]);
+		#end
+		#end
+	}
+
+	function existsInSubProcess(variable:String, value:String) {
+		#if js
+		return UtilityProcess.runUtilityAsCommand(["checkEnv", variable, value]) == 0;
+		#else
+		return UtilityProcess.runUtility(["checkEnv", variable, value]).exitCode == 0;
 		#end
 	}
 
 	function testGetEnv() {
 		// EXISTS should be set manually via the command line
 		Assert.notNull(Sys.getEnv("EXISTS"));
+
+		// on windows, Sys.getEnv should be case insensitive
+		if (Sys.systemName() == "Windows")
+			Assert.notNull(Sys.getEnv("exists"));
+
 		Assert.isNull(Sys.getEnv("doesn't exist"));
 	}
 
 	#if !java
 	function testPutEnv() {
-		Sys.putEnv("foo", "value");
-		Assert.equals("value", Sys.getEnv("foo"));
+		Sys.putEnv("FOO", "value");
+		Assert.equals("value", Sys.getEnv("FOO"));
+
+		Assert.equals("value", Sys.environment().get("FOO"));
 
-		Assert.equals("value", Sys.environment().get("foo"));
+		Assert.isTrue(existsInSubProcess("FOO", "value"));
+
+		#if python
+		// the variable should also be visible through python's api
+		Assert.equals("value", python.lib.Os.environ.get("FOO"));
+		#end
 
 		// null
-		Sys.putEnv("foo", null);
-		Assert.isNull(Sys.getEnv("foo"));
+		Sys.putEnv("FOO", null);
+		Assert.isNull(Sys.getEnv("FOO"));
+
+		Assert.isFalse(Sys.environment().exists("FOO"));
+
+		Assert.isFalse(existsInSubProcess("FOO", "value"));
+
+		#if python
+		// the variable should also be gone when checking through python's api
+		Assert.isFalse(python.lib.Os.environ.hasKey("FOO"));
+		#end
 
-		Assert.isFalse(Sys.environment().exists("foo"));
+		Assert.isTrue(try {
+			Sys.putEnv("NON_EXISTENT", null);
+			true;
+		} catch (e) {
+			trace(e);
+			false;
+		});
 	}
 	#end
 

tests/sys/src/UtilityProcess.hx

@@ -1,5 +1,5 @@
 /**
-	Used by TestUnicode.
+	Used by TestUnicode and TestSys.
 	Runs a given simple program based on the first argument.
  */
 
@@ -123,21 +123,67 @@ class UtilityProcess {
 		};
 	}
 
+	/** Runs the utility program via Sys.command rather than as a separate process,
+		for compatiblity with hxnodejs.
+
+		Returns the exit code of the command.
+	 **/
+	public static function runUtilityAsCommand(args:Array<String>, ?options:{?stdin:String, ?execPath:String, ?execName:String}):Int {
+		if (options == null) options = {};
+		if (options.execPath == null) options.execPath = BIN_PATH;
+		if (options.execName == null) options.execName = BIN_NAME;
+		final execFull = Path.join([options.execPath, options.execName]);
+		final exitCode =
+		#if (macro || interp)
+		Sys.command("haxe", ["compile-each.hxml", "-p", options.execPath, "--run", options.execName].concat(args));
+		#elseif cpp
+		Sys.command(execFull, args);
+		#elseif cs
+		(switch (Sys.systemName()) {
+			case "Windows":
+				Sys.command(execFull, args);
+			case _:
+				Sys.command("mono", [execFull].concat(args));
+		});
+		#elseif java
+		Sys.command(Path.join([java.lang.System.getProperty("java.home"), "bin", "java"]), ["-jar", execFull].concat(args));
+		#elseif python
+		Sys.command(python.lib.Sys.executable, [execFull].concat(args));
+		#elseif neko
+		Sys.command("neko", [execFull].concat(args));
+		#elseif hl
+		Sys.command("hl", [execFull].concat(args));
+		#elseif php
+		Sys.command(php.Global.defined('PHP_BINARY') ? php.Const.PHP_BINARY : 'php', [execFull].concat(args));
+		#elseif lua
+		Sys.command("lua", [execFull].concat(args));
+		#elseif js
+		Sys.command("node", [execFull].concat(args));
+		#else
+		1;
+		#end
+
+		return exitCode;
+	}
+
 	public static function main():Void {
 		var args = Sys.args();
-		function sequenceIndex(d:String, mode:String):String return (switch (UnicodeSequences.valid[Std.parseInt(d)]) {
+		function sequenceIndex(d:String, mode:String):String
+			return switch UnicodeSequences.valid[Std.parseInt(d)] {
 				case Only(ref): UnicodeSequences.codepointsToString(ref);
 				case Normal(nfc, nfd): UnicodeSequences.codepointsToString(mode == "nfc" ? nfc : nfd);
-			});
+			};
 		switch (args) {
 			case _.slice(0, 1) => ["putEnv"]:
-			// ["putEnv", var name, index, nfc mode, next args...]
-			Sys.putEnv(args[1], sequenceIndex(args[2], args[3]));
-			var out = runUtility(args.slice(4));
-			Sys.print(out.stdout);
-			Sys.exit(out.exitCode);
+				// ["putEnv", var name, index, nfc mode, next args...]
+				Sys.putEnv(args[1], sequenceIndex(args[2], args[3]));
+				var out = runUtility(args.slice(4));
+				Sys.print(out.stdout);
+				Sys.exit(out.exitCode);
 			case ["getCwd"]: Sys.println(Sys.getCwd());
 			case ["getEnv", name]: Sys.println(Sys.getEnv(name));
+			case ["checkEnv", name, value]:
+				Sys.exit(value == Sys.getEnv(name) ? 0 : 1);
 			case ["environment", name]: Sys.println(Sys.environment().get(name));
 			case ["exitCode", Std.parseInt(_) => code]: Sys.exit(code);
 			case ["args", data]: Sys.println(data);
@@ -148,9 +194,9 @@ class UtilityProcess {
 			case ["stdin.readString", Std.parseInt(_) => len]: Sys.println(Sys.stdin().readString(len, UTF8));
 			case ["stdin.readUntil", Std.parseInt(_) => end]: Sys.println(Sys.stdin().readUntil(end));
 			case ["stderr.writeString", d, mode]:
-			var stream = Sys.stderr(); stream.writeString(sequenceIndex(d, mode)); stream.flush();
+				var stream = Sys.stderr(); stream.writeString(sequenceIndex(d, mode)); stream.flush();
 			case ["stdout.writeString", d, mode]:
-			var stream = Sys.stdout(); stream.writeString(sequenceIndex(d, mode)); stream.flush();
+				var stream = Sys.stdout(); stream.writeString(sequenceIndex(d, mode)); stream.flush();
 			case ["programPath"]: Sys.println(Sys.programPath());
 			case _: // no-op
 		}