Browse Source

Make Sys.environment() consistent between targets (#10460)

* Fix Sys.environment() on python, c#, and java

Previously, this would return the internal map which would update with
`Sys.putEnv()` calls. They now return copies so that they work as other
targets do.

Fixes #10401

* Update test for Sys.environment()

* Update documentation for `Sys.environment()`
tobil4sk 4 years ago
parent
commit
ed5785a754
5 changed files with 31 additions and 16 deletions
  1. 2 1
      std/Sys.hx
  2. 1 1
      std/cs/_std/Sys.hx
  3. 5 6
      std/java/_std/Sys.hx
  4. 1 1
      std/python/_std/Sys.hx
  5. 22 7
      tests/sys/src/TestSys.hx

+ 2 - 1
std/Sys.hx

@@ -64,7 +64,8 @@ extern class Sys {
 	static function putEnv(s:String, v:Null<String>):Void;
 
 	/**
-		Returns all environment variables.
+		Returns a map of the current environment variables and their values
+		as of the invocation of the function.
 	**/
 	static function environment():Map<String, String>;
 

+ 1 - 1
std/cs/_std/Sys.hx

@@ -70,7 +70,7 @@ class Sys {
 			}
 		}
 
-		return _env;
+		return _env.copy();
 	}
 
 	public static inline function sleep(seconds:Float):Void {

+ 5 - 6
std/java/_std/Sys.hx

@@ -54,14 +54,13 @@ using haxe.Int64;
 	}
 
 	public static function environment():Map<String, String> {
-		if (_env != null)
-			return _env;
-		var _env = _env = new haxe.ds.StringMap();
-		for (mv in java.lang.System.getenv().entrySet()) {
-			_env.set(mv.getKey(), mv.getValue());
+		if (_env == null) {
+			_env = new haxe.ds.StringMap();
+			for (mv in java.lang.System.getenv().entrySet())
+				_env.set(mv.getKey(), mv.getValue());
 		}
 
-		return _env;
+		return _env.copy();
 	}
 
 	public static function sleep(seconds:Float):Void {

+ 1 - 1
std/python/_std/Sys.hx

@@ -77,7 +77,7 @@ class Sys {
 	}
 
 	public static function environment():Map<String, String> {
-		return environ;
+		return environ.copy();
 	}
 
 	public static function sleep(seconds:Float):Void {

+ 22 - 7
tests/sys/src/TestSys.hx

@@ -10,6 +10,26 @@ class TestSys extends TestCommandBase {
 		// EXISTS should be set manually via the command line
 		Assert.notNull(env.get("EXISTS"));
 		Assert.isNull(env.get("doesn't exist"));
+
+		final nonExistent = "NON_EXISTENT";
+		env.set(nonExistent, "1");
+		// new copies should not be affected
+		Assert.isNull(Sys.environment()[nonExistent]);
+
+		#if !java
+		// env should not update when environment updates
+		final toUpdate = "TO_UPDATE";
+
+		Sys.putEnv(toUpdate, "1");
+		Assert.isNull(env.get(toUpdate));
+
+		// new copy should have the variable
+		Assert.equals("1", Sys.environment()[toUpdate]);
+
+		// environment should not update if env updates
+		env.set(toUpdate, "2");
+		Assert.equals("1", Sys.getEnv(toUpdate));
+		#end
 	}
 
 	function testGetEnv() {
@@ -23,18 +43,13 @@ class TestSys extends TestCommandBase {
 		Sys.putEnv("foo", "value");
 		Assert.equals("value", Sys.getEnv("foo"));
 
-		var env = Sys.environment();
-		Assert.equals("value", env.get("foo"));
+		Assert.equals("value", Sys.environment().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"));
+		Assert.isFalse(Sys.environment().exists("foo"));
 	}
 	#end