Make Sys.environment() consistent between targets #10460
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 HaxeFoundation#10401
|
CI failure is a one-off error, unrelated to the changes. |
tobil4sk
added a commit
to tobil4sk/haxec
that referenced
this pull request
Nov 6, 2021
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 HaxeFoundation#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.
tobil4sk
added a commit
to tobil4sk/haxec
that referenced
this pull request
Nov 7, 2021
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 HaxeFoundation#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.
tobil4sk
added a commit
to tobil4sk/haxec
that referenced
this pull request
Nov 8, 2021
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 HaxeFoundation#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.
tobil4sk
added a commit
to tobil4sk/haxec
that referenced
this pull request
Nov 8, 2021
Fixes HaxeFoundation#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 HaxeFoundation#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.
Simn
pushed a commit
that referenced
this pull request
Nov 16, 2021
* [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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #10401
On python, c# and java,
Sys.environment()now returns a copy of the map of environment variables every time instead of the internal maps used to keep track of them. This prevents modifications of the map resulting in future calls toSys.environment()returning the user modified map.This now means that
Sys.putEnv()doesn't update existing copies of the map, and modifying the map on python does not change the result ofSys.getEnv().