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:
us...@infra.apache.org
