It's nice to use environment().clear() in tests,
for testability, but environment().clear() has the problem that
runtime environments on both Unix and Windows implicitly
depend on certain environment variables to be able to do anything.
E.g. you may someday end up with tests failing on unix due to
KRB5CCNAME undefined.
So environment().clear() is a bad idea for reliable code
(and before ProcessBuilder, you had no choice; Runtime.exec API
did it implicitly)
Still, we would like to have tests for it.
We could make tests more robust by whitelisting a set of
environment variables added back in after clear(),
which would include SystemRoot on Windows, and perhaps
likely suspects like PATH and KRB5CCNAME.
Heiko, that would be a difficult change to get right,
and perhaps impossible to get 100% right if you take
into consideration environment variable values that
don't survive round-trip decoding-encoding intact or
that contain newlines. So this would not be necessary
to get my approval (it's a pre-existing test failure).
Also, FYI, it is my understanding that pre-2008
versions of the Microsoft compiler are still supported.
It would be my hope that as many versions as practical
would be supported, since these products cost real money.
Martin
On Mon, Mar 23, 2009 at 05:56, Alan Bateman <alan.bate...@sun.com> wrote:
> Heiko Wagner wrote:
>>
>> Thanks for the description of the comment writing style. I have
>> investigated, why I get a return code of 6. This return code indicates
>> that
>> java.exe launcher fails to start the Java VM. I will try to explain step
>> by
>> step. ;-)
>>
>> This is the test code that fails:
>>
>> try {
>> ProcessBuilder pb = new ProcessBuilder();
>> pb.environment().clear();
>> equal(getenvInChild(pb), "");
>> } catch (Throwable t) { unexpected(t); }
>>
>> 1) this code does start the java.exe
>> 2) java.exe tries to start the Java VM in function LoadJavaVM
>> 3) LoadJavaVM tries to load the jvm.dll via LoadLibrary
>> 4) LoadLibrary loads the jvm.dll and it's dependent libraries, in
>> particular
>> the C runtime lib
>> 5) loading of the MSCV90.DLL fails with error message:
>> "Unable to resolve storage root for assembly directory
>> x86_Microsoft.VC90.CRT"
>> This is caused by the "pb.environment().clear()", because it clears the
>> env variable %SystemRoot%.
>> 6) LoadLibrary fails due to missing dependencies, thus causing java.exe
>> fail
>> with exit code 6
>>
>> You might not experience this problem, when you compile the JDK with an
>> oder
>> MS VC++ version. According to the build readme in the JDK VS2008 is
>> required
>> ("...The 32-bit OpenJDK Windows build requires Microsoft Visual Studio C++
>> 2008 (VS2008) Standard Edition compiler..."), but it seems the binary
>> build
>> downloads on the openJDK site use an older version of Visual Studio.
>>
>
> This does seems to be an environment issue. I ran this test with b51 on
> Windows XP and it fails for each case where the child is created without
> SystemRoot in the environment. This build was done with VC7/.NET 2003. I
> hacked the test to ensure that this variable is inherited from the parent
> and the modified tests works as expected. I also grabbed your changes and
> patched my src/windows/native/java/lang/ProcessImpl.c and all Runtime and
> ProcessBuilder tests are passing.
>
> -Alan.
>
