RFR: JDK-8193055 ADD_JVM_ARG_IF_OK always fails

[Erik Joelsson]

Looks good.

/Erik


On 2017-12-05 04:27, Magnus Ihse Bursie wrote:
> 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
>