[GitHub] [maven-shared-utils] mkarg commented on a change in pull request #68: [MSHARED-969] Environment variable with null value

[GitBox <gi...@apache.org>]

mkarg commented on a change in pull request #68:
URL: https://github.com/apache/maven-shared-utils/pull/68#discussion_r549710348



##########
File path: src/main/java/org/apache/maven/shared/utils/cli/Commandline.java
##########
@@ -241,15 +241,16 @@ public void addSystemEnvironment()
     public String[] getEnvironmentVariables()
     {
         addSystemEnvironment();
-        String[] environmentVars = new String[envVars.size()];
-        int i = 0;
+        List<String> environmentVars = new ArrayList<>();
         for ( String name : envVars.keySet() )

Review comment:
       While I think the solution is working, I wonder whether it solves the actual cause: At least on Windows, it is impossible to stored environment variables with a `NULL` value. Try this effectly removes it from the environment, according to the Windows API docs. So at least on Windows the correct solution would be that this variable name-value-pair must not be put into `envVars` right from the start. In turn, *here* is no change to be made.
   
   Can someone conform that it is valid on other OS to actually have an environment variable in the environment that stores `NULL` (not the String but `null` in the Java sense)?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org