RFR 8199756 : Simplify language, country, script, and variant property initialization

[mandy chung]

Hi Roger,

Thanks for adding the test that should have created when JDK-4700857 was 
resolved.

Since this fix changes the setting of those i18n properties to later 
after JVM_InitProperties is called, it would be a good opportunity to 
set these properties in Java and remove the native fillI18nProps 
method.  What it needs from Java is to get the value of 
sprops->display_XXX, one option is to define a Java class holding these 
displayXXX fields that are set by initProperties directly.

What do you think?

Mandy

On 3/22/18 8:24 AM, Roger Riggs wrote:
> Hi,
>
> The webrev[1] has been updated with a new test added to verify that if 
> a user.* property
> is defined on the command line, then corresponding property has the 
> same value and
> correctly overrides any value from the platform.
>
> After verification, the comment (line 304) has been corrected to refer 
> to the platform native
> encoding of bytes -> strings instead of the user.* I18N properties.
>
> The tests for properties and locales have been run and passed in a 
> variety of locales.
>
> Thanks, Roger
>
> [1] http://cr.openjdk.java.net/~rriggs/webrev-prop-simplify-8199756/
>
>
> On 3/19/2018 6:38 PM, David Holmes wrote:
>> Hi Roger,
>>
>> On 19/03/2018 11:55 PM, Roger Riggs wrote:
>>> Hi Alan,
>>>
>>> The original changeset [1] is most easily viewed in the jdk-8u repo.
>>>
>>> The comment and placement of the removes of the properties and the 
>>> code to re-set them
>>> around the call to the JVM_InitProperties (which sets properties 
>>> from the
>>> -D arguments among others), seem to indicate that the previous property
>>> values are irrelevant.
>>
>> Not irrelevant: defaults.
>>
>>>
>>> The remaining question is whether there are any side effects or 
>>> usage of those
>>> four property values between the time they are initially set and 
>>> when they are removed.
>>> The code in System.initProperties is straightforward and the bulk of 
>>> the code
>>> is simply setting property values, with one interspersed call to 
>>> initialize the sun.jnu_encoding.
>>>  From all appearances there are no side effects or uses of the 
>>> initially set values of those four properties.
>>
>> I would have expected these values to possibly have an impact on 
>> InitalizeEncoding and subsequent use of PUTPROP_ForPlatformNString. 
>> Else why set them early then unset them again? Or maybe they impact 
>> any error messages that might arise in this stage of initialization?
>>
>> I expect any issues here to be extremely subtle - as Alan indicated. 
>> If there is any path to Java code that may try to read and use these 
>> properties then they need to be set.
>>
>> And if these are truly unnecessary then doesn't that imply that 
>> placing them in sprops via GetJavaProperties in the first place is 
>> also unnecessary?
>>
>> Has this been extensively tested on a non-English Windows system? And 
>> do we even have tests for various initialization failures?
>>
>> David
>> -----
>>
>>> Thanks, Roger
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-4700857
>>>      RFE: separating user locale and user interface locale
>>>
>>> On 3/17/18 5:46 AM, Alan Bateman wrote:
>>>> On 16/03/2018 20:32, Roger Riggs wrote:
>>>>> Please review a small simplification of the initialization of 
>>>>> System properties for
>>>>> language, country, script, and variant.  Some steps for 
>>>>> initializing them are unnecessary.
>>>>> The tests pass; a careful review would be appreciated so as to 
>>>>> avoid breakage.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~rriggs/webrev-prop-simplify-8199756/
>>>>>
>>>>> Issue:
>>>>>   https://bugs.openjdk.java.net/browse/JDK-8199756
>>>> There are subtle interactions with how system properties are set in 
>>>> the VM and on the command line which will take a bit effort to 
>>>> research in order to review this and be satisfied that the change 
>>>> is safe. Have you dug into the ancient history and bugs to see why 
>>>> it was done this way?
>>>>
>>>> -Alan
>>>
>