On Tue, 13 Apr 2021 18:24:55 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/System.java line 2020: >> >>> 2018: setIn0(new BufferedInputStream(fdIn)); >>> 2019: setOut0(newPrintStream(fdOut, cs)); >>> 2020: setErr0(newPrintStream(fdErr, cs)); >> >> It was getting from sun.stdout.encoding or sun.stderr.encoding. After the >> change, when there is no console, it would be set with >> Charset.defaultCharset(). Would that be an incompatible change? > > 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/3419
