[sys] Fix `Sys.putEnv()` with null as variable value on all targets (#10402)

* [sys] Make value parameter for `Sys.putEnv()` nullable

* [sys] Add null case to `Sys.putEnv()` for various platforms

For cs, lua, php, and python

Make the v parameter nullable on neko, hl, cpp, and java

* [eval] Implement null case for `Sys.putEnv()` on Eval

Add `eval.luv.Env.unsetEnv()` as well

* [CI] Add test for null case in `Sys.putEnv()`

- Reorganise sys tests to allow for testing pre-defined environment
variables

src/macro/eval/evalLuv.ml

@@ -2141,6 +2141,10 @@ let env_fields = [
 		and value = decode_native_string v2 in
 		encode_unit_result (Env.setenv name ~value)
 	);
+	"unsetEnv", vfun1 (fun v ->
+		let name = decode_string v in
+		encode_unit_result (Env.unsetenv name)
+	);
 	"environ", vfun0 (fun() ->
 		let encode env =
 			let map =

src/macro/eval/evalStdLib.ml

@@ -2637,11 +2637,14 @@ module StdSys = struct
 			| _ -> vnull
 	)
 
-	let putEnv = vfun2 (fun s v ->
-		let s = decode_string s in
-		let v = decode_string v in
-		catch_unix_error Unix.putenv s v;
-		vnull
+	let putEnv = vfun2 (fun s -> function
+		| v when v = vnull ->
+			let _ = Luv.Env.unsetenv (decode_string s) in vnull
+		| v ->
+			let s = decode_string s in
+			let v = decode_string v in
+			catch_unix_error Unix.putenv s v;
+			vnull
 	)
 
 	let setCwd = vfun1 (fun s ->

std/Sys.hx

@@ -57,9 +57,11 @@ extern class Sys {
 	/**
 		Sets the value of the given environment variable.
 
+		If `v` is `null`, the environment variable is removed.
+
 		(java) This functionality is not available on Java; calling this function will throw.
 	**/
-	static function putEnv(s:String, v:String):Void;
+	static function putEnv(s:String, v:Null<String>):Void;
 
 	/**
 		Returns all environment variables.

std/cpp/NativeSys.hx

@@ -34,7 +34,7 @@ extern class NativeSys {
 	extern static function get_env(v:String):String;
 
 	@:native("_hx_std_put_env")
-	extern static function put_env(e:String, v:String):Void;
+	extern static function put_env(e:String, v:Null<String>):Void;
 
 	@:native("_hx_std_sys_sleep")
 	extern static function sys_sleep(f:Float):Void;

std/cpp/_std/Sys.hx

@@ -63,7 +63,7 @@ import haxe.SysTools;
 		return v;
 	}
 
-	public static function putEnv(s:String, v:String):Void {
+	public static function putEnv(s:String, v:Null<String>):Void {
 		NativeSys.put_env(s, v);
 	}
 

std/cs/_std/Sys.hx

@@ -50,9 +50,14 @@ class Sys {
 		return Environment.GetEnvironmentVariable(s);
 	}
 
-	public static function putEnv(s:String, v:String):Void {
+	public static function putEnv(s:String, v:Null<String>):Void {
 		Environment.SetEnvironmentVariable(s, v);
-		if (_env != null)
+		if (_env == null)
+			return;
+
+		if (v == null)
+			_env.remove(s);
+		else
 			_env.set(s, v);
 	}
 

std/eval/_std/Sys.hx

@@ -33,7 +33,7 @@ class Sys {
 
 	extern static public function getEnv(s:String):String;
 
-	extern static public function putEnv(s:String, v:String):Void;
+	extern static public function putEnv(s:String, v:Null<String>):Void;
 
 	extern static public function environment():Map<String, String>;
 

std/eval/luv/Env.hx

@@ -16,6 +16,11 @@ extern class Env {
 	**/
 	static function setEnv(name:String, value:NativeString):Result<Result.NoData>;
 
+	/**
+		Deletes an environment variable.
+	**/
+	static function unsetEnv(name:String):Result<Result.NoData>;
+
 	/**
 		Retrieves all environment variables.
 	**/

std/hl/_std/Sys.hx

@@ -86,7 +86,7 @@ class Sys {
 		return makePath(v);
 	}
 
-	public static function putEnv(s:String, v:String):Void {
+	public static function putEnv(s:String, v:Null<String>):Void {
 		if (!put_env(getPath(s), if (v == null) null else getPath(v)))
 			throw "putEnv() failure";
 	}

std/java/_std/Sys.hx

@@ -48,7 +48,7 @@ using haxe.Int64;
 		return java.lang.System.getenv(s);
 	}
 
-	public static function putEnv(s:String, v:String):Void {
+	public static function putEnv(s:String, v:Null<String>):Void {
 		// java offers no support for it (!)
 		throw new haxe.exceptions.NotImplementedException("Not implemented in this platform");
 	}

std/lua/_std/Sys.hx

@@ -98,7 +98,9 @@ class Sys {
 		return Os.getenv(s);
 	}
 
-	public inline static function putEnv(s:String, v:String):Void {
+	public inline static function putEnv(s:String, v:Null<String>):Void {
+		if (v == null)
+			return Os.unsetenv(s);
 		Os.setenv(s, v);
 	}
 

std/neko/_std/Sys.hx

@@ -69,7 +69,7 @@ import haxe.SysTools;
 		return new String(v);
 	}
 
-	public static function putEnv( s : String, v : String ) : Void {
+	public static function putEnv( s : String, v : Null<String> ) : Void {
 		untyped put_env(s.__s,if( v == null ) null else v.__s);
 	}
 

std/php/_std/Sys.hx

@@ -50,9 +50,14 @@ import haxe.SysTools;
 		return value == false ? null : value;
 	}
 
-	public static function putEnv(s:String, v:String):Void {
-		customEnvVars[s] = '$v'; // in case of `null` it should become `"null"`
-		Global.putenv('$s=$v');
+	public static function putEnv(s:String, v:Null<String>):Void {
+		if (v == null) {
+			Global.unset(customEnvVars[s]);
+			Global.putenv('$s');
+		} else {
+			customEnvVars[s] = '$v'; // in case of `null` it should become `"null"`
+			Global.putenv('$s=$v');
+		}
 	}
 
 	public static inline function sleep(seconds:Float):Void {

std/python/_std/Sys.hx

@@ -67,7 +67,11 @@ class Sys {
 		return environ.get(s);
 	}
 
-	public static function putEnv(s:String, v:String):Void {
+	public static function putEnv(s:String, v:Null<String>):Void {
+		if (v == null) {
+			environ.remove(s);
+			return;
+		}
 		python.lib.Os.putenv(s, v);
 		environ.set(s, v);
 	}

tests/runci/System.hx

@@ -83,6 +83,22 @@ class System {
 			fail();
 	}
 
+	static function showAndRunCommand(cmd:String, args:Null<Array<String>>, displayed:String):Int {
+		infoMsg('Command: $displayed');
+
+		final t = Timer.stamp();
+		final exitCode = Sys.command(cmd, args);
+		final dt = Math.round(Timer.stamp() - t);
+
+		final msg = 'Command exited with $exitCode in ${dt}s: $displayed';
+		if (exitCode != 0)
+			failMsg(msg);
+		else
+			successMsg(msg);
+
+		return exitCode;
+	}
+
 	/**
 	 * Recursively delete a directory.
 	 * @return Int Exit code of a system command executed to perform deletion.
@@ -172,4 +188,32 @@ class System {
 		Sys.println('Changing directory to $path');
 		Sys.setCwd(path);
 	}
-}
+
+	static function mergeArgs(cmd:String, args:Array<String>) {
+		return switch (Sys.systemName()) {
+			case "Windows":
+				[StringTools.replace(cmd, "/", "\\")].concat(args).map(haxe.SysTools.quoteWinArg.bind(_, true)).join(" ");
+			case _:
+				[cmd].concat(args).map(haxe.SysTools.quoteUnixArg).join(" ");
+		}
+	}
+
+	/* command for setting the environment variable to set before sys tests */
+	static final setCommand = if (Sys.systemName() == "Windows") "set"; else "export";
+	static final nameAndValue = "EXISTS=1";
+
+	/** Prepares environment for system tests and runs `cmd` with `args` **/
+	static public function runSysTest(cmd:String, ?args:Array<String>) {
+		final cmdStr = '$setCommand [$nameAndValue] && ' + cmd + (args == null ? '' : ' $args');
+
+		if (args != null)
+			cmd = mergeArgs(cmd, args);
+
+		final fullCmd = '$setCommand $nameAndValue && $cmd';
+
+		final exitCode = showAndRunCommand(fullCmd, null, cmdStr);
+
+		if (exitCode != 0)
+			fail();
+	}
+}

tests/runci/targets/Cpp.hx

@@ -60,7 +60,7 @@ class Cpp {
 
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-cpp.hxml"].concat(args));
-		runCpp("bin/cpp/Main-debug", []);
+		runSysTest(FileSystem.fullPath("bin/cpp/Main-debug"));
 
 		changeDirectory(threadsDir);
 		runCommand("haxe", ["build.hxml", "-cpp", "export/cpp"]);

tests/runci/targets/Cs.hx

@@ -33,7 +33,7 @@ class Cs {
 		if (args == null) args = [];
 		exe = FileSystem.fullPath(exe);
 		switch (systemName) {
-			case "Linux", "Mac":
+			case "Linux" | "Mac":
 				runCommand("mono", [exe].concat(args));
 			case "Windows":
 				runCommand(exe, args);
@@ -60,7 +60,13 @@ class Cs {
 
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-cs.hxml",'-D','fast_cast'].concat(args));
-		runCs("bin/cs/bin/Main-Debug.exe", []);
+		final exe = FileSystem.fullPath("bin/cs/bin/Main-Debug.exe");
+		switch (systemName) {
+			case "Linux" | "Mac":
+				runSysTest("mono", [exe]);
+			case "Windows":
+				runSysTest(exe);
+		}
 
 		changeDirectory(threadsDir);
 		runCommand("haxe", ["build.hxml", "-cs", "export/cs"]);
@@ -76,4 +82,4 @@ class Cs {
 			runCs("bin/main/bin/Main.exe");
 		}
 	}
-}
+}

tests/runci/targets/Hl.hx

@@ -83,7 +83,7 @@ class Hl {
 
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-hl.hxml"].concat(args));
-		runCommand(hlBinary, ["bin/hl/sys.hl"]);
+		runSysTest(hlBinary, ["bin/hl/sys.hl"]);
 
 		changeDirectory(miscHlDir);
 		runCommand("haxe", ["run.hxml"]);

tests/runci/targets/Java.hx

@@ -29,7 +29,7 @@ class Java {
 
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-java.hxml"].concat(args));
-		runCommand("java", ["-jar", "bin/java/Main-Debug.jar"]);
+		runSysTest("java", ["-jar", "bin/java/Main-Debug.jar"]);
 
 		changeDirectory(threadsDir);
 		runCommand("haxe", ["build.hxml", "-java", "export/java"].concat(args));
@@ -52,4 +52,4 @@ class Java {
 			}
 		}
 	}
-}
+}

tests/runci/targets/Js.hx

@@ -113,7 +113,7 @@ class Js {
 		changeDirectory(sysDir);
 		runCommand("npm", ["install", "deasync"], true);
 		runCommand("haxe", ["compile-js.hxml"].concat(args));
-		runCommand("node", ["bin/js/sys.js"]);
+		runSysTest("node", ["bin/js/sys.js"]);
 
 		changeDirectory(miscJsDir);
 		runCommand("haxe", ["run.hxml"]);

tests/runci/targets/Jvm.hx

@@ -16,10 +16,10 @@ class Jvm {
 
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-jvm.hxml"].concat(args));
-		runCommand("java", ["-jar", "bin/jvm/sys.jar"]);
+		runSysTest("java", ["-jar", "bin/jvm/sys.jar"]);
 
 		changeDirectory(threadsDir);
 		runCommand("haxe", ["build.hxml", "--jvm", "export/threads.jar"].concat(args));
 		runCommand("java", ["-jar", "export/threads.jar"]);
 	}
-}
+}

tests/runci/targets/Lua.hx

@@ -77,7 +77,7 @@ class Lua {
 
 			changeDirectory(sysDir);
 			runCommand("haxe", ["compile-lua.hxml"].concat(args));
-			runCommand("lua", ["bin/lua/sys.lua"]);
+			runSysTest("lua", ["bin/lua/sys.lua"]);
 
 			changeDirectory(miscDir + "luaDeadCode/stringReflection");
 			runCommand("haxe", ["compile.hxml"]);

tests/runci/targets/Macro.hx

@@ -28,7 +28,7 @@ class Macro {
 		runCommand("haxe", ["run.hxml"]);
 
 		changeDirectory(sysDir);
-		runCommand("haxe", ["compile-macro.hxml"].concat(args));
+		runSysTest("haxe", ["compile-macro.hxml"].concat(args));
 
 		switch Sys.systemName() {
 			case 'Linux':
@@ -40,4 +40,4 @@ class Macro {
 		changeDirectory(threadsDir);
 		runCommand("haxe", ["build.hxml", "--interp"]);
 	}
-}
+}

tests/runci/targets/Neko.hx

@@ -11,10 +11,10 @@ class Neko {
 
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-neko.hxml"].concat(args));
-		runCommand("neko", ["bin/neko/sys.n"]);
+		runSysTest("neko", ["bin/neko/sys.n"]);
 
 		changeDirectory(threadsDir);
 		runCommand("haxe", ["build.hxml", "--neko", "export/threads.n"]);
 		runCommand("neko", ["export/threads.n"]);
 	}
-}
+}

tests/runci/targets/Php.hx

@@ -25,7 +25,7 @@ class Php {
 					}
 				case _:
 			}
-			infoMsg('php ${phpVer} has already been installed.');
+			infoMsg('php $phpVer has already been installed.');
 			return;
 		}
 		switch systemName {
@@ -57,19 +57,18 @@ class Php {
 
 		for(prefix in prefixes) {
 			changeDirectory(unitDir);
-			if(isCi()) {
+			if(isCi())
 				deleteDirectoryRecursively(binDir);
-			}
 
 			runCommand("haxe", ["compile-php.hxml"].concat(prefix).concat(args));
 			runThroughPhpVersions(runCommand.bind(_, [binDir + "/index.php"]));
 
 			changeDirectory(sysDir);
-			if(isCi()) {
+			if(isCi())
 				deleteDirectoryRecursively(binDir);
-			}
+
 			runCommand("haxe", ["compile-php.hxml"].concat(prefix).concat(args));
-			runThroughPhpVersions(runCommand.bind(_, ["bin/php/Main/index.php"]));
+			runThroughPhpVersions(runSysTest.bind(_, ["bin/php/Main/index.php"]));
 		}
 	}
 
@@ -83,4 +82,4 @@ class Php {
 				fn('php');
 		}
 	}
-}
+}

tests/runci/targets/Python.hx

@@ -68,7 +68,7 @@ class Python {
 		changeDirectory(sysDir);
 		runCommand("haxe", ["compile-python.hxml"].concat(args));
 		for (py in pys) {
-			runCommand(py, ["bin/python/sys.py"]);
+			runSysTest(py, ["bin/python/sys.py"]);
 		}
 
 		changeDirectory(miscPythonDir);
@@ -86,4 +86,4 @@ class Python {
 			runCommand(py, ["export/threads.py"]);
 		}
 	}
-}
+}

tests/sys/run.hxml

@@ -10,28 +10,24 @@ compile.hxml
 
 # Mac/Linux
 --next
---cmd echo Neko   && neko bin/neko/sys.n
---cmd echo Python && python3 bin/python/sys.py
---cmd echo Cpp    && bin/cpp/Main-debug
---cmd echo CS     && mono bin/cs/bin/Main-Debug.exe
---cmd echo Java   && java -jar bin/java/Main-Debug.jar
---cmd echo Php    && php bin/php/Main/index.php
---cmd echo Hl     && hl bin/hl/sys.hl
---cmd echo Js     && node bin/js/sys.js
+--cmd echo Neko   && export EXISTS=1 && neko bin/neko/sys.n
+--cmd echo Python && export EXISTS=1 && python3 bin/python/sys.py
+--cmd echo Cpp    && export EXISTS=1 && bin/cpp/Main-debug
+--cmd echo CS     && export EXISTS=1 && mono bin/cs/bin/Main-Debug.exe
+--cmd echo Java   && export EXISTS=1 && java -jar bin/java/Main-Debug.jar
+--cmd echo Php    && export EXISTS=1 && php bin/php/Main/index.php
+--cmd echo Hl     && export EXISTS=1 && hl bin/hl/sys.hl
+--cmd echo Js     && export EXISTS=1 && node bin/js/sys.js
+--cmd echo Macro  && export EXISTS=1 && haxe compile-macro.hxml
 
 # Windows
 # --next
-# --cmd echo Neko   && neko bin\neko\sys.n
-# --cmd echo Python && python3 bin\python\sys.py
-# --cmd echo Cpp    && bin\cpp\Main-debug.exe
-# --cmd echo CS     && bin\cs\bin\Main-Debug.exe
-# --cmd echo Java   && java -jar bin\java\Main-Debug.jar
-# --cmd echo Php    && php bin\php\Main\index.php
-# --cmd echo Hl     && hl bin/hl/sys.hl
-# --cmd echo Js     && node bin/js/sys.js
-
-# Macro has to be placed at the end since it would exit the compilation process.
---next
---cmd echo Macro
---next
-compile-macro.hxml
+# --cmd echo Neko   && set EXISTS=1 && neko bin\neko\sys.n
+# --cmd echo Python && set EXISTS=1 && python3 bin\python\sys.py
+# --cmd echo Cpp    && set EXISTS=1 && bin\cpp\Main-debug.exe
+# --cmd echo CS     && set EXISTS=1 && bin\cs\bin\Main-Debug.exe
+# --cmd echo Java   && set EXISTS=1 && java -jar bin\java\Main-Debug.jar
+# --cmd echo Php    && set EXISTS=1 && php bin\php\Main\index.php
+# --cmd echo Hl     && set EXISTS=1 && hl bin/hl/sys.hl
+# --cmd echo Js     && set EXISTS=1 && node bin/js/sys.js
+# --cmd echo Macro  && set EXISTS=1 && haxe compile-macro.hxml

tests/sys/src/Main.hx

@@ -23,15 +23,9 @@ class Main {
 		#end
 		#end
 		#if php
-		switch (Sys.systemName()) {
-			case "Windows":
-				// pass
-			case _:
-				runner.addCase(new net.TestSocket());
-		}
-		#else
-			runner.addCase(new net.TestSocket());
+		if (Sys.systemName() != "Windows")
 		#end
+			runner.addCase(new net.TestSocket());
 		var report = Report.create(runner);
 		report.displayHeader = AlwaysShowHeader;
 		report.displaySuccessResults = NeverShowSuccessResults;

tests/sys/src/TestSys.hx

@@ -5,18 +5,38 @@ class TestSys extends TestCommandBase {
 		return Sys.command(cmd, args);
 	}
 
-	function testEnv() {
-		#if !(java)
+	function testEnvironment() {
+		var env = Sys.environment();
+		// EXISTS should be set manually via the command line
+		Assert.notNull(env.get("EXISTS"));
+		Assert.isNull(env.get("doesn't exist"));
+	}
+
+	function testGetEnv() {
+		// EXISTS should be set manually via the command line
+		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"));
-		#end
-		Assert.equals(null, Sys.getEnv("doesn't exist"));
 
-		#if !(java)
 		var env = Sys.environment();
 		Assert.equals("value", env.get("foo"));
+
+		// null
+		Sys.putEnv("foo", null);
+		Assert.isNull(Sys.getEnv("foo"));
+
+		#if !(python || cs) // #10401
+		env = Sys.environment();
 		#end
+
+		Assert.isFalse(env.exists("foo"));
 	}
+	#end
 
 	function testProgramPath() {
 		var p = Sys.programPath();