On Tue, 13 Apr 2021 19:30:53 GMT, Joe Wang <jo...@openjdk.org> wrote:

>> Although the code path is different, the logic to determine the encoding is 
>> not changed, as `sun.stdout/err.encoding` are only set if the VM is invoked 
>> from a terminal (in fact, there's a bug where they aren't set even in a 
>> terminal on unix env, which I fixed in this patch) which is the same 
>> condition in which `System.console()` returns the console. And for both 
>> cases, it will default to `Charset.defaultCharset()` if they are not 
>> available.
>> 
>> Having said that, one regression test started to fail with this 
>> implementation change as follows:
>> 
>> Exception in thread "main" java.util.ServiceConfigurationError: Locale 
>> provider adapter "CLDR"cannot be instantiated.
>>      at 
>> java.base/sun.util.locale.provider.LocaleProviderAdapter.forType(LocaleProviderAdapter.java:199)
>>      at 
>> java.base/sun.util.locale.provider.LocaleProviderAdapter.findAdapter(LocaleProviderAdapter.java:287)
>>      at 
>> java.base/sun.util.locale.provider.LocaleProviderAdapter.getAdapter(LocaleProviderAdapter.java:258)
>>      at java.base/java.text.NumberFormat.getInstance(NumberFormat.java:960)
>>      at 
>> java.base/java.text.NumberFormat.getNumberInstance(NumberFormat.java:518)
>>      at java.base/java.util.Scanner.useLocale(Scanner.java:1270)
>>      at java.base/java.util.Scanner.<init>(Scanner.java:543)
>>      at java.base/java.util.Scanner.<init>(Scanner.java:554)
>>      at TestConsole.main(TestConsole.java:54)
>> Caused by: java.lang.reflect.InvocationTargetException
>>      at 
>> java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>>  Method)
>>      at 
>> java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
>>      at 
>> java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
>>      at 
>> java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
>>      at 
>> java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
>>      at 
>> java.base/sun.util.locale.provider.LocaleProviderAdapter.forType(LocaleProviderAdapter.java:188)
>>      ... 8 more
>> 
>> I haven't looked at it in detail but it seems that calling 
>> `System.console()` in `System.initPhase1()` is not allowed, as the module 
>> system is not there yet. So I reverted the implementation to the original 
>> and simply retained the description in `System.out/err` with a change to 
>> `determined by` to `equivalent to`.
>
> Thanks for the explanation and updates. It's a worthwhile exercise to attempt 
> to align things around the new method. A short note above line 2023 to record 
> this might be useful as well (sth. like it's eq to console != null ? 
> console.charset() : Charset.defaultCharset();). No need to create another 
> update if you decide to add the note.

Thanks. Added some explanations as suggested.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3419

Reply via email to