-
-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can not unset or set empty environment variables. #11
Comments
@parched I should be able to fix this quickly if you provide a stack-trace. |
@cowwoc Like this?
|
I am having a hard time reproducing this problem. I tried this configuration:
and it worked fine. I suspect this was fixed in a newer dependency than you are running. What configuration, platform, JDK, and version of CMake plugin are you using? Can you try reproducing the problem with the 3.7.0-b1-SNAPSHOT version of the plugin? |
Ok yes, your I will check everything on Monday, but from memory and looking at that error message, Linux with JDK 8, 2.8.11-b4. |
Okay, so I think this is fixed. For reference, I am using JDK8 update 112 under Windows. If you still see the problem, try upgrading to update 112. We'll wait for your report on Monday :) |
I've released 3.7.0-b1 (non-SNAPSHOT) so you can test against that instead. |
Hi @cowwoc, I have tried 3.7.0-b1 with java 1.8.0_112 and I still get the exception. Maybe the difference is that I'm using Linux. |
@parched Thanks for the report. I'll take a look (on Linux) later on this week. Please send me a pom.xml that triggers the exception if you have one; otherwise, I'll work without it. |
|
…se its behavior is OS-specific. Attempting to fix NullPointerException on Linux when passing null values: #11
Thanks for the testcase. I reproduced the problem and I believe it is fixed in 3.7.0-b2-SNAPSHOT. If you confirm I will make an official release. |
Hi @cowwoc, I will try and test if I get a chance. Looking at the code changes though, it looks like the nullptr exception will be stopped by just ignoring the variable it rather than setting it to empty which is the goal. |
@parched My mistake. I misunderstood the goal. Please try the updated SNAPSHOT which sets an empty value instead. |
@cowwoc that looks better. Unfortunately I'm on holiday for a month so won't be able to test it soon. If that example I posted works for you though I'm happy to close this. |
Sounds good. I'll make a release and you can confirm that this is closed when you get back. (Have a great time!) |
Hey @parched, Can you please try version 3.7.2-b1? I fixed this issue a while back but forgot to have you verify the fix. |
Sorry for the long delay, we finally got around to upgrading to 3.7.2-b1 and the issue is indeed fixed. Thank you! |
Excellent, thanks for letting us know. |
If you try to set an environment variable to empty it throws a null pointer exception.
I think this is because maven treats empty as
null
andProcessBuilder.environment()
doesn't handlenull
values.A solution would be to check for any keys with
null
values inenvironmentVariables
and then remove them fromProcessBuilder.environment()
The text was updated successfully, but these errors were encountered: