RFR: 8212794 IBM-964 is required for AIX default charset

[Roger Riggs]

Hi Ichiroh,

src/jdk.charsets/share/classes/sun/nio/cs/ext/IBM33722.java:

     I think this is no longer needed since it only has imports.
     By the way, the style is to import each specific class and avoid 
wild card imports.

TestIBMBugs:
   - Please use a style consistent with other methods in the class.
     In this case spaces are needed around "{" and "}, and a space after 
"," comma.

   - The new method bug8212794, includes a test for x-IBM33722.
     Is that needed since there does not appear to be a change for 33722?

Regards, Roger


On 11/30/2018 09:58 AM, Magnus Ihse Bursie wrote:
>
>
> On 2018-11-30 10:49, Ichiroh Takiguchi wrote:
>> Hello.
>>
>> Could you review the fix again ?
>>
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8212794
>> Change: https://cr.openjdk.java.net/~itakiguchi/8212794/webrev.02/
> I think it looks good but please let someone from core-libs review it 
> too.
>
> /Magnus
>>
>> I just fixed only IBM964 for JDK-8212794.
>> (IBM29626C fix is not included)
>>
>> On non AIX platform (Linux),
>> ibm-euctw alias is added for IBM964.
>>
>> Without fix
>> $ jshell
>> |  Welcome to JShell -- Version 12-ea
>> |  For an introduction type: /help intro
>>
>> jshell> var cs = java.nio.charset.Charset.forName("IBM964")
>> cs ==> x-IBM964
>>
>> jshell> cs.getClass().getName()
>> $2 ==> "sun.nio.cs.ext.IBM964"
>>
>> jshell> System.out.println(String.join("\n", cs.aliases()))
>> ibm-964
>> cp964
>> ibm964
>> 964
>>
>> jshell> /exit
>> |  Goodbye
>> $
>> ======
>>
>> With fix
>> ======
>> $ jshell
>> |  Welcome to JShell -- Version 12-internal
>> |  For an introduction type: /help intro
>>
>> jshell> var cs = java.nio.charset.Charset.forName("IBM964")
>> cs ==> x-IBM964
>>
>> jshell> cs.getClass().getName()
>> $2 ==> "sun.nio.cs.ext.IBM964"
>>
>> jshell> System.out.println(String.join("\n", cs.aliases()))
>> ibm-964
>> cp964
>> ibm-euctw
>> ibm964
>> 964
>>
>> jshell> /exit
>> |  Goodbye
>> $
>> ======
>>
>> On AIX platform
>> IBM964 is moved to java.base module from jdk.charset module.
>>
>> ======
>> $ LANG=zh_TW jshell
>> |  Welcome to JShell -- Version 12-internal
>> |  For an introduction type: /help intro
>>
>> jshell> var cs = java.nio.charset.Charset.defaultCharset()
>> cs ==> x-IBM964
>>
>> jshell> cs.getClass().getName()
>> $2 ==> "sun.nio.cs.IBM964"
>>
>> jshell> System.out.println(String.join("\n", cs.aliases()))
>> ibm-964
>> cp964
>> ibm-euctw
>> ibm964
>> 964
>>
>> jshell> /exit
>> |  Goodbye
>> $
>> ======
>>
>> I'd like to obtain a sponsor for this issue.
>>
>> Thanks,
>> Ichiroh Takiguchi
>> IBM Japan, Ltd.
>>
>> On 2018-11-29 22:39, Ichiroh Takiguchi wrote:
>>> Hello Alan & Magnus.
>>>
>>> Sorry for you confusion.
>>> I did many copy actions and rename actions.
>>> So you may see, I added unexpected code into non AIX platform.
>>>
>>> I think I should not put 2 kind of modification.
>>>
>>> For this bug id, I'll handle IBM964 and IBM33722.
>>> (also SimpleEUCEncoder.java is required)
>>>
>>> I'll submit code review again.
>>>
>>> Additionally, I'll touch
>>> make/data/charsetmapping/charsets
>>> make/data/charsetmapping/stdcs-aix
>>>
>>> I'll not touch
>>> make/jdk/src/classes/build/tools/charsetmapping/Main.java
>>> make/jdk/src/classes/build/tools/charsetmapping/SRC.java
>>>
>>> My build machine is not so fast, after test is done.
>>> I'll post new code.
>>>
>>> Thanks,
>>> Ichiroh Takiguchi
>>>
>>> On 2018-11-28 19:10, Magnus Ihse Bursie wrote:
>>>> On 2018-11-28 10:36, Alan Bateman wrote:
>>>>> On 28/11/2018 09:28, Magnus Ihse Bursie wrote:
>>>>>> I'm quite unsatisfied with the current handling of character sets 
>>>>>> in the build in general. :-( I'd really like to modernize it. I 
>>>>>> have a, slightly fuzzy, laundry list of things I want to fix from 
>>>>>> a build perspective, but I'm not sure of what "external" 
>>>>>> requirements are coming from AIX and the general core-libs agenda 
>>>>>> regarding character sets in general.
>>>>>>
>>>>>> I think there is a good opportunity to solve many problems at the 
>>>>>> same time here, as long as everyone agrees on what is the 
>>>>>> preferred outcome.
>>>>> The support in the build to configure the charsets to include in 
>>>>> java.base on each platform has been working well. Charsets that 
>>>>> aren't in java.base go into the jdk.charsets service provider 
>>>>> module and that has been working well too. From the result point 
>>>>> of view, perhaps, but definitely not from the build perspective. 
>>>>> ;-) But yes, I understand this is functionality that should be kept.
>>>>> One thing that we lack is some way to add charsets for specific 
>>>>> platforms and this comes up with the IBM patches where they are 
>>>>> looking to adding several additional IBM charsets. One starting 
>>>>> point that we've touched on in several threads here is dropping 
>>>>> the EBCDIC charsets from the main stream builds. Going there will 
>>>>> need build support.
>>>> So build support for trivially adding specific charsets to specific
>>>> platforms? Both to java.base (for AIX) and jdk.charsets, I presume,
>>>> then?
>>>>
>>>> Can you expand on the issue of dropping ebcdic? What's the problem
>>>> that needs build support?
>>>>
>>>> /Magnus
>>>>>
>>>>>
>>>>> -Alan
>>
>