[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
HaxeFoundation · Oct 29, 2021 · e5c120c · e5c120c
1 parent 697abe9
commit e5c120c
Show file tree

Hide file tree

Showing 29 changed files with 168 additions and 79 deletions.
diff --git a/src/macro/eval/evalLuv.ml b/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 =

diff --git a/src/macro/eval/evalStdLib.ml b/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 ->

diff --git a/std/Sys.hx b/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.

diff --git a/std/cpp/NativeSys.hx b/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;

diff --git a/std/cpp/_std/Sys.hx b/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);
 	}
 

diff --git a/std/cs/_std/Sys.hx b/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);
 	}
 

diff --git a/std/eval/_std/Sys.hx b/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>;
 

diff --git a/std/eval/luv/Env.hx b/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.
 	**/

diff --git a/std/hl/_std/Sys.hx b/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";
 	}

diff --git a/std/java/_std/Sys.hx b/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");
 	}

diff --git a/std/lua/_std/Sys.hx b/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);
 	}
 

diff --git a/std/neko/_std/Sys.hx b/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);
 	}
 

diff --git a/std/php/_std/Sys.hx b/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 {

diff --git a/std/python/_std/Sys.hx b/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);
 	}

diff --git a/tests/runci/System.hx b/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();
+	}
+}
diff --git a/tests/runci/targets/Cpp.hx b/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"]);

diff --git a/tests/runci/targets/Cs.hx b/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");
 		}
 	}
-}
+}
diff --git a/tests/runci/targets/Hl.hx b/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"]);

diff --git a/tests/runci/targets/Java.hx b/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 {
 			}
 		}
 	}
-}
+}
diff --git a/tests/runci/targets/Js.hx b/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"]);

diff --git a/tests/runci/targets/Jvm.hx b/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"]);
 	}
-}
+}
diff --git a/tests/runci/targets/Lua.hx b/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"]);

diff --git a/tests/runci/targets/Macro.hx b/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"]);
 	}
-}
+}
diff --git a/tests/runci/targets/Neko.hx b/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"]);
 	}
-}
+}
diff --git a/tests/runci/targets/Php.hx b/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');
 		}
 	}
-}
+}