RFR: JDK-8193055 ADD_JVM_ARG_IF_OK always fails

[Magnus Ihse Bursie]

On 2017-12-05 13:03, David Holmes wrote:
> Hi Magnus,
>
> On 5/12/2017 8:49 PM, Magnus Ihse Bursie wrote:
>> We mistakenly use -XX:-UnlockDiagnosticVMOptions instead of 
>> -XX:+UnlockDiagnosticVMOptions in a ADD_JVM_ARG_IF_OK call. This 
>> means that the test will always fail and the arguments will never be 
>> added to the command line.
>>
>> Since this has been the case all time along, it's probably not that 
>> important, but it was added to keep down the logging when using a 
>> debug build as boot jdk, which has probably not been tested that much 
>> either.
>
> LogVMOutput will only be turned on in a debug build if you also enable 
> specific (mostly debug only) log/print/trace options.
I thought that these extra debug options were enabled by default in 
debug build, and that required this action to disable them. Otherwise it 
doesn't really make sense.

>
>> We should either fix this or remove the arguments completely.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8193055
>> Patch inline:
>> diff --git a/make/autoconf/boot-jdk.m4 b/make/autoconf/boot-jdk.m4
>> --- a/make/autoconf/boot-jdk.m4
>> +++ b/make/autoconf/boot-jdk.m4
>> @@ -354,7 +354,7 @@
>>     AC_MSG_CHECKING([flags for boot jdk java command] )
>>
>>     # Disable special log output when a debug build is used as Boot 
>> JDK...
>> -  ADD_JVM_ARG_IF_OK([-XX:-PrintVMOptions 
>> -XX:-UnlockDiagnosticVMOptions 
>> -XX:-LogVMOutput],boot_jdk_jvmargs,[$JAVA])
>> +  ADD_JVM_ARG_IF_OK([-XX:-PrintVMOptions 
>> -XX:+UnlockDiagnosticVMOptions 
>> -XX:-LogVMOutput],boot_jdk_jvmargs,[$JAVA])
>
> Fix is fine. But you could probably remove them too. Do you recall why 
> this was added? It may relate to something now migrated to Unified 
> Logging.
At first I thought that it was introduced as part of JDK-8010767, but 
that only shuffled it around a bit. Going full Indiana Jones I've dug 
all the way to the bottom of the repo. It was introduced by the very 
first build-infra incarnation. :-)

So, I'd say, we should probably remove it instead. Updating my webrev:

diff --git a/make/autoconf/boot-jdk.m4 b/make/autoconf/boot-jdk.m4
--- a/make/autoconf/boot-jdk.m4
+++ b/make/autoconf/boot-jdk.m4
@@ -353,9 +353,6 @@

    AC_MSG_CHECKING([flags for boot jdk java command] )

-  # Disable special log output when a debug build is used as Boot JDK...
-  ADD_JVM_ARG_IF_OK([-XX:-PrintVMOptions -XX:-UnlockDiagnosticVMOptions 
-XX:-LogVMOutput],boot_jdk_jvmargs,[$JAVA])
-
    # Force en-US environment
    ADD_JVM_ARG_IF_OK([-Duser.language=en 
-Duser.country=US],boot_jdk_jvmargs,[$JAVA])



/Magnus


>
> David
> -----
>
>>     # Force en-US environment
>>     ADD_JVM_ARG_IF_OK([-Duser.language=en 
>> -Duser.country=US],boot_jdk_jvmargs,[$JAVA])
>>
>> /Magnus