Integrated: 8204541: Correctly support AIX xlC 16.1 symbol visibility flags

[Ichiroh Takiguchi]

On Fri, 18 Mar 2022 01:50:56 GMT, Ichiroh Takiguchi  
wrote:

> This change is just for AIX platform with IBM XL C/C++ 16.1.
> By this change, lib's entry points are reduced by compilation and linkage 
> option changes.
> 
> Without changes:
> 
> $ dump -X32_64 -Tv lib/libawt.so | grep EXP | wc
> 914
> 
> With changes
> 
> $ dump -X32_64 -Tv lib/libawt.so | grep EXP | wc
> 142

This pull request has now been integrated.

Changeset: 0c3094c8
Author:Ichiroh Takiguchi 
URL:   
https://git.openjdk.java.net/jdk/commit/0c3094c8186b4d53e8bad80e2369fc7b9ae9e201
Stats: 11 lines in 3 files changed: 6 ins; 2 del; 3 mod

8204541: Correctly support AIX xlC 16.1 symbol visibility flags

Reviewed-by: ihse, stuefe

-

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

Re: RFR: 8204541: Correctly support AIX xlC 16.1 symbol visibility flags [v2]

[Ichiroh Takiguchi]

On Fri, 18 Mar 2022 08:07:53 GMT, Thomas Stuefe  wrote:

>> Ichiroh Takiguchi has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Remove unexpected comment
>
> So, does this mean we cannot build with older xlC versions anymore? Just 
> curious.
> 
> Pinging @backwaterred .

Thanks, @tstuefe .

I saw following link, I 'm using AIX XL C++ 16.1.
https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms

And following change was already applied. 
[JDK-8224214](https://bugs.openjdk.java.net/browse/JDK-8224214) [AIX] Remove 
support for legacy xlc compiler

@backwaterred , could you check older compiler should be supported ?

-

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

Re: RFR: 8204541: Correctly support AIX xlC 16.1 symbol visibility flags [v2]

[Ichiroh Takiguchi]

> This change is just for AIX platform with IBM XL C/C++ 16.1.
> By this change, lib's entry points are reduced by compilation and linkage 
> option changes.
> 
> Without changes:
> 
> $ dump -X32_64 -Tv lib/libawt.so | grep EXP | wc
> 914
> 
> With changes
> 
> $ dump -X32_64 -Tv lib/libawt.so | grep EXP | wc
> 142

Ichiroh Takiguchi has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove unexpected comment

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7862/files
  - new: https://git.openjdk.java.net/jdk/pull/7862/files/a4c73abb..4b383aeb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7862&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7862&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7862.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7862/head:pull/7862

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

Re: RFR: 8204541: Correctly support AIX xlC 16.1 symbol visibility flags [v2]

[Ichiroh Takiguchi]

On Fri, 18 Mar 2022 07:41:28 GMT, Magnus Ihse Bursie  wrote:

>> Ichiroh Takiguchi has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Remove unexpected comment
>
> make/autoconf/flags-ldflags.m4 line 80:
> 
>> 78: 
>> 79:   elif test "x$TOOLCHAIN_TYPE" = xxlc; then
>> 80: # -bexpall should be used instead of -bnoexpall if -qvisibility 
>> option is not defined
> 
> If you hard-code for the assumption that xlc v16 is being used, then this 
> comment does not make any sense..?

Thanks, @magicus .
I removed the comment.

-

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

RFR: 8204541: Correctly support AIX xlC 16.1 symbol visibility flags

[Ichiroh Takiguchi]

This change is just for AIX platform with IBM XL C/C++ 16.1.
By this change, lib's entry points are reduced by compilation and linkage 
option changes.

Without changes:

$ dump -X32_64 -Tv lib/libawt.so | grep EXP | wc
914

With changes

$ dump -X32_64 -Tv lib/libawt.so | grep EXP | wc
142

-

Commit messages:
 - 8204541: Correctly support AIX xlC 16.1 symbol visibility flags

Changes: https://git.openjdk.java.net/jdk/pull/7862/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7862&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8204541
  Stats: 13 lines in 3 files changed: 7 ins; 2 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7862.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7862/head:pull/7862

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

Re: RFR: 8201429: Support AIX Input Method Editor (IME) for AWT Input Method Framework (IMF)

[Ichiroh Takiguchi]

Hello Sergey.

I'm not sure if I understand what you want to change...

XCreateGC:
The colors are created upper code, they will be overwritten.

XSetBackground:
I'm sorry, I have no idea about XSetBackground(),
I thought background might have default value, But I could not find out 
the doc.


Ichiroh Takiguchi
IBM Japan, Ltd.

On 2020-02-20 14:10, Sergey Bylokhov wrote:

Hello, Christoph.

Could you shed some light on the changes below:

-statusWindow->fgGC = XCreateGC(dpy, status, valuemask, &values);
+statusWindow->fgGC = create_gc(status, FALSE);
 XSetForeground(dpy, statusWindow->fgGC, fg);
-statusWindow->bgGC = XCreateGC(dpy, status, valuemask, &values);
+statusWindow->bgGC = create_gc(status, TRUE);
 XSetForeground(dpy, statusWindow->bgGC, bg);

The new method create_gc() is used to set the FG and BG color of the 
GC.

But foreground color immediately replaced by the XSetForeground.
I am asking because the create_gc() uses 
AwtScreenDataPtr.whitepixel/blackpixel

which I would like to delete. I guess it should be possible rewrite
this code like this:

statusWindow->fgGC = XCreateGC(dpy, status, valuemask, &values);
XSetForeground(dpy, statusWindow->fgGC, fg);
XSetBackground(dpy, statusWindow->fgGC, bg);
statusWindow->bgGC = XCreateGC(dpy, status, valuemask, &values);
XSetForeground(dpy, statusWindow->bgGC, bg);
XSetBackground(dpy, statusWindow->bgGC, fg);

What do you think?

On 5/28/18 5:38 am, Langer, Christoph wrote:

Hi Phil,

thanks for your review. I have incorporated your suggestions: 
http://cr.openjdk.java.net/~clanger/webrevs/8201429.3/


I'll run it through our internal testing and run a jdk-submit job with 
it. When all is green, I'll push it to jdk-client tomorrow.


Best regards
Christoph


-Original Message-
From: Philip Race [mailto:philip.r...@oracle.com]
Sent: Sonntag, 20. Mai 2018 01:53
To: Langer, Christoph 
Cc: awt-...@openjdk.java.net; Ichiroh Takiguchi
; build-dev@openjdk.java.net; 
ppc-aix-port-

d...@openjdk.java.net
Subject: Re:  RFR: 8201429: Support AIX Input Method Editor
(IME) for AWT Input Method Framework (IMF)

I think I am 99% OK with this.

In general I see what you are doing here and how you've presented the
webrev.
Treating even the new files as moved helps see the differences but it 
is

still
a challenge to follow all the moving pieces.

So before we had just

  abstract class unix/X11InputMethod <-class 
unix/XInputMethod


Now we have

 abstract class unix/X11InputMethodBase

  /\

 /   \

/  \

   /\

abstract class unix/X11InputMethodabstract class
aix/X11InputMethod

\/
 \  /
  \/
class unix/XInputMethod



I have submitted a build job with this patch to make sure it all 
builds

on Linux & Solaris ..
and it was all fine.

But testing for this would have to be manual, and I don't have cycles
for that.
So I'll have to accept that the testing done by IBM was enough

So only minor comments ...
http://cr.openjdk.java.net/~clanger/webrevs/8201429.2/src/java.desktop/u
nix/classes/sun/awt/X11InputMethodBase.java.sdiff.html

   730 case 0: //None of the value is set by Wnn

"value is " ->  "values are " ?


http://cr.openjdk.java.net/~clanger/webrevs/8201429.2/src/java.desktop/u
nix/native/libawt_xawt/awt/awt_InputMethod.c.sdiff.html

why did you move

26 #ifdef HEADLESS
27 #error This file should not be included in headless 
library

28 #endif


I think it should be first. It is also missing from the equivalent 
AIX file but that

is
up to you .. I really didn't look any further at the AIX files.

.. and there seems no point to moving around some of the other 
includes

except to make the webrev harder to read :-)

But good job cleaning up a lot of the formatting of the native code.

-phil.





On 5/18/18, 4:59 AM, Langer, Christoph wrote:

Hi all,

Here is an updated webrev:
http://cr.openjdk.java.net/~clanger/webrevs/8201429.2/
Can someone from the graphics/awt team please have a look at that
change? Especially checking that we don't break non-AIX platforms? 
Thanks

in advance.


@Ichiroh: Thanks for your review and tests. Adressing your points:


resetCompositionState() was missing in
src/java.desktop/aix/classes/sun/awt/X11InputMethod.java


==

==
--- a/src/java.desktop/aix/classes/sun/awt/X11InputMethod.java  Wed

May

09 09:05:32 2018 +0900
+++ b/src/java.desktop/

RE: JDK-8217880 AIX build issue about JDK-8214533

[Ichiroh Takiguchi]

To reviewers,
Thank you for reviewing in a short time.

Best regards,
Ichiroh Takiguchi

On 2019-01-29 20:20, Lindenmaier, Goetz wrote:

Pushed.

Best regards,
  Goetz


-Original Message-
From: Ichiroh Takiguchi 
Sent: Dienstag, 29. Januar 2019 11:30
To: Lindenmaier, Goetz 
Cc: build-dev ; ppc-aix-port-dev 
d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Baesken, 
Matthias


Subject: RE: JDK-8217880 AIX build issue about JDK-8214533

Hello Goetz.

Thank you for build testing.

Yes, I need a sponsor.
If possible, could you handle it ?

Thanks,
Ichiroh Takiguchi

On 2019-01-29 19:17, Lindenmaier, Goetz wrote:
> Hi,
>
> this looks good, the build works with this patch.
> Do you need a sponsor?
>
> Best regards,
>   Goetz.
>
>> -----Original Message-
>> From: Ichiroh Takiguchi 
>> Sent: Dienstag, 29. Januar 2019 02:21
>> To: Lindenmaier, Goetz 
>> Cc: build-dev ; ppc-aix-port-dev
>> > d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Baesken,
>> Matthias
>> 
>> Subject: RFR: JDK-8217880 AIX build issue about JDK-8214533
>>
>> Hello.
>>
>> Sorry about build issue for JDK-8214533.
>>
>> EUC_JP was extra entry on make/data/charsetmapping/stdcs-aix.
>>
>> Bug:https://bugs.openjdk.java.net/browse/JDK-8217880
>> Change: https://cr.openjdk.java.net/~itakiguchi/8217880/webrev.00/
>>
>> Could you review the fix ?
>>
>> Thanks,
>> Ichiroh Takiguchi
>> IBM Japan, Ltd.
>>
>> On 2019-01-28 22:49, Ichiroh Takiguchi wrote:
>> > Hello Goetz.
>> >
>> > Thank you for your suggestion.
>> > I just open JDK-8217880 [1].
>> >
>> > I just restart clean build.
>> >
>> > I'll post new fix including testcase for EUC_JP_LINUX and EUC_JP_Open.
>> >
>> > [1] https://bugs.openjdk.java.net/browse/JDK-8217880
>> >
>> > Thanks,
>> > Ichiroh Takiguchi
>> >
>> > On 2019-01-28 22:11, Lindenmaier, Goetz wrote:
>> >> Hi Ichiroh,
>> >>
>> >> just open a bug, like "Fix aix build after 8214533" and post a RFR for
>> >> it.
>> >> I assume the fix is quite trivial so we can review it quick.
>> >>
>> >> Best regards,
>> >>   Goetz.
>> >>
>> >>> -Original Message-
>> >>> From: ppc-aix-port-dev 
On
>> >>> Behalf Of Ichiroh Takiguchi
>> >>> Sent: Montag, 28. Januar 2019 14:13
>> >>> To: Baesken, Matthias 
>> >>> Cc: build-dev ; ppc-aix-port-dev
>> >>> > >>> d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Alan Bateman
>> >>> 
>> >>> Subject: RE: RFR: 8214533 IBM-29626C is required for AIX default
>> >>> charset
>> >>>
>> >>> Hello.
>> >>>
>> >>> I'm very sorry. It's my fault.
>> >>> EUC_JP class was moved to java.base module.
>> >>> (sun.nio.cs.EUC_JP).
>> >>>
>> >>> make/data/charsetmapping/stdcs-aix should have EUC_JP_LINUX and
>> >>> EUC_JP_Open.
>> >>>
>> >>> Could you suggest me how I should provide new webrev files ?
>> >>>
>> >>> Thanks,
>> >>> Ichiroh Takiguchi
>> >>>
>> >>>
>> >>> On 2019-01-28 17:03, Baesken, Matthias wrote:
>> >>> > Hello,   seems  8214533   got pushed  recently  into  jdk/jdk.   Now
>> >>> > we see build errors on AIX  ,  are they related to  this change ?
>> >>> >
>> >>> >
>> >>> > /nb/rs6000_64/nightly/output-jdk-
>> >>>
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:63:
>> >>> > error: Decoder is not public in EUC_JP; cannot be accessed from
>> >>> > outside package
>> >>> > private static class Decoder extends EUC_JP.Decoder {
>> >>> >^
>> >>> > /nb/rs6000_64/nightly/output-jdk-
>> >>>
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:69:
>> >>> > error: Encoder is not public in EUC_JP; cannot be accessed from
>> >>> > outside package
>> >>> > private static class Encoder extends EUC_JP.Encoder {
>> >>> >^
>> >>> > /nb/rs6000_64/nightly/output-jdk-
>> >>> test/sup

RE: JDK-8217880 AIX build issue about JDK-8214533

[Ichiroh Takiguchi]

Hello Goetz.

Thank you for build testing.

Yes, I need a sponsor.
If possible, could you handle it ?

Thanks,
Ichiroh Takiguchi

On 2019-01-29 19:17, Lindenmaier, Goetz wrote:

Hi,

this looks good, the build works with this patch.
Do you need a sponsor?

Best regards,
  Goetz.


-Original Message-
From: Ichiroh Takiguchi 
Sent: Dienstag, 29. Januar 2019 02:21
To: Lindenmaier, Goetz 
Cc: build-dev ; ppc-aix-port-dev 
d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Baesken, 
Matthias


Subject: RFR: JDK-8217880 AIX build issue about JDK-8214533

Hello.

Sorry about build issue for JDK-8214533.

EUC_JP was extra entry on make/data/charsetmapping/stdcs-aix.

Bug:https://bugs.openjdk.java.net/browse/JDK-8217880
Change: https://cr.openjdk.java.net/~itakiguchi/8217880/webrev.00/

Could you review the fix ?

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

On 2019-01-28 22:49, Ichiroh Takiguchi wrote:
> Hello Goetz.
>
> Thank you for your suggestion.
> I just open JDK-8217880 [1].
>
> I just restart clean build.
>
> I'll post new fix including testcase for EUC_JP_LINUX and EUC_JP_Open.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8217880
>
> Thanks,
> Ichiroh Takiguchi
>
> On 2019-01-28 22:11, Lindenmaier, Goetz wrote:
>> Hi Ichiroh,
>>
>> just open a bug, like "Fix aix build after 8214533" and post a RFR for
>> it.
>> I assume the fix is quite trivial so we can review it quick.
>>
>> Best regards,
>>   Goetz.
>>
>>> -Original Message-
>>> From: ppc-aix-port-dev  On
>>> Behalf Of Ichiroh Takiguchi
>>> Sent: Montag, 28. Januar 2019 14:13
>>> To: Baesken, Matthias 
>>> Cc: build-dev ; ppc-aix-port-dev
>>> >> d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Alan Bateman
>>> 
>>> Subject: RE: RFR: 8214533 IBM-29626C is required for AIX default
>>> charset
>>>
>>> Hello.
>>>
>>> I'm very sorry. It's my fault.
>>> EUC_JP class was moved to java.base module.
>>> (sun.nio.cs.EUC_JP).
>>>
>>> make/data/charsetmapping/stdcs-aix should have EUC_JP_LINUX and
>>> EUC_JP_Open.
>>>
>>> Could you suggest me how I should provide new webrev files ?
>>>
>>> Thanks,
>>> Ichiroh Takiguchi
>>>
>>>
>>> On 2019-01-28 17:03, Baesken, Matthias wrote:
>>> > Hello,   seems  8214533   got pushed  recently  into  jdk/jdk.   Now
>>> > we see build errors on AIX  ,  are they related to  this change ?
>>> >
>>> >
>>> > /nb/rs6000_64/nightly/output-jdk-
>>> test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:63:
>>> > error: Decoder is not public in EUC_JP; cannot be accessed from
>>> > outside package
>>> > private static class Decoder extends EUC_JP.Decoder {
>>> >^
>>> > /nb/rs6000_64/nightly/output-jdk-
>>> test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:69:
>>> > error: Encoder is not public in EUC_JP; cannot be accessed from
>>> > outside package
>>> > private static class Encoder extends EUC_JP.Encoder {
>>> >^
>>> > /nb/rs6000_64/nightly/output-jdk-
>>> test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:65:
>>> > error: Decoder is not public in EUC_JP; cannot be accessed from
>>> > outside package
>>> > private static class Decoder extends EUC_JP.Decoder {
>>> >^
>>> > /nb/rs6000_64/nightly/output-jdk-
>>> test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:85:
>>> > error: Encoder is not public in EUC_JP; cannot be accessed from
>>> > outside package
>>> > private static class Encoder extends EUC_JP.Encoder {
>>> >
>>> > Best regards, Matthias
>>> >
>>> >
>>> >
>>> >> -Original Message-
>>> >> From: ppc-aix-port-dev 
On
>>> >> Behalf Of Ichiroh Takiguchi
>>> >> Sent: Dienstag, 15. Januar 2019 01:51
>>> >> To: Alan Bateman 
>>> >> Cc: build-dev ; ppc-aix-port-dev >> >> port-...@openjdk.java.net>; core-libs-...@openjdk.java.net
>>> >> Subject: Re: RFR: 8214533 IBM-29626C is required for AIX default
>>> >> charset
>>> >>
>>> >> Hello Alan.
>>> >>
>>> >

RFR: JDK-8217880 AIX build issue about JDK-8214533

[Ichiroh Takiguchi]

Hello.

Sorry about build issue for JDK-8214533.

EUC_JP was extra entry on make/data/charsetmapping/stdcs-aix.

Bug:https://bugs.openjdk.java.net/browse/JDK-8217880
Change: https://cr.openjdk.java.net/~itakiguchi/8217880/webrev.00/

Could you review the fix ?

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

On 2019-01-28 22:49, Ichiroh Takiguchi wrote:

Hello Goetz.

Thank you for your suggestion.
I just open JDK-8217880 [1].

I just restart clean build.

I'll post new fix including testcase for EUC_JP_LINUX and EUC_JP_Open.

[1] https://bugs.openjdk.java.net/browse/JDK-8217880

Thanks,
Ichiroh Takiguchi

On 2019-01-28 22:11, Lindenmaier, Goetz wrote:

Hi Ichiroh,

just open a bug, like "Fix aix build after 8214533" and post a RFR for 
it.

I assume the fix is quite trivial so we can review it quick.

Best regards,
  Goetz.


-Original Message-
From: ppc-aix-port-dev  On
Behalf Of Ichiroh Takiguchi
Sent: Montag, 28. Januar 2019 14:13
To: Baesken, Matthias 
Cc: build-dev ; ppc-aix-port-dev 

d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Alan Bateman

Subject: RE: RFR: 8214533 IBM-29626C is required for AIX default 
charset


Hello.

I'm very sorry. It's my fault.
EUC_JP class was moved to java.base module.
(sun.nio.cs.EUC_JP).

make/data/charsetmapping/stdcs-aix should have EUC_JP_LINUX and
EUC_JP_Open.

Could you suggest me how I should provide new webrev files ?

Thanks,
Ichiroh Takiguchi


On 2019-01-28 17:03, Baesken, Matthias wrote:
> Hello,   seems  8214533   got pushed  recently  into  jdk/jdk.   Now
> we see build errors on AIX  ,  are they related to  this change ?
>
>
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:63:
> error: Decoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Decoder extends EUC_JP.Decoder {
>^
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:69:
> error: Encoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Encoder extends EUC_JP.Encoder {
>^
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:65:
> error: Decoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Decoder extends EUC_JP.Decoder {
>^
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:85:
> error: Encoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Encoder extends EUC_JP.Encoder {
>
> Best regards, Matthias
>
>
>
>> -Original Message-
>> From: ppc-aix-port-dev  On
>> Behalf Of Ichiroh Takiguchi
>> Sent: Dienstag, 15. Januar 2019 01:51
>> To: Alan Bateman 
>> Cc: build-dev ; ppc-aix-port-dev > port-...@openjdk.java.net>; core-libs-...@openjdk.java.net
>> Subject: Re: RFR: 8214533 IBM-29626C is required for AIX default
>> charset
>>
>> Hello Alan.
>>
>> Could you review the fix again ?
>>
>> Bug:https://bugs.openjdk.java.net/browse/JDK-8214533
>> Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.01/
>>
>> I added IBM29626C charset as standard way.
>> Please give any suggestion and question.
>>
>> Thanks,
>> Ichiroh Takiguchi
>> IBM Japan, Ltd.
>>
>> On 2018-12-14 18:58, Ichiroh Takiguchi wrote:
>> > Hello Alan.
>> >
>> > I opened JDK-8215333 for Charset filtering issue [1].
>> > I cannot wait until JDK-8215333 is closed.
>> > Is it possible to put IBM-29626C charset with standard way ?
>> >
>> > [1] https://bugs.openjdk.java.net/browse/JDK-8215333
>> >
>> > Thanks,
>> > Ichiroh Takiguchi
>> >
>> > On 2018-12-10 21:21, Ichiroh Takiguchi wrote:
>> >> Hello Roger, Magnus and Alan.
>> >> I may need to put alias information into charsets file.
>> >> stdcs-xxx cannot handle this information...
>> >>
>> >> Still AIX needs IBM-29626C charset for default encoding...
>> >>
>> >> I appreciate if you give me further suggestions.
>> >>
>> >> Thanks,
>> >> Ichiroh Takiguchi
>> >>
>> >> On 2018-12-10 20:50, Alan Bateman wrote:
>> >>> On 10/12/2018 11:01, Magnus Ihse Bursie wrote:
>> >>>> On 2018-12-07 21:20, Roger Riggs wrote:
>> >>>>> Hi,
>> >>>>>
>> >>>&

RE: RFR: 8214533 IBM-29626C is required for AIX default charset

[Ichiroh Takiguchi]

Hello Goetz.

Thank you for your suggestion.
I just open JDK-8217880 [1].

I just restart clean build.

I'll post new fix including testcase for EUC_JP_LINUX and EUC_JP_Open.

[1] https://bugs.openjdk.java.net/browse/JDK-8217880

Thanks,
Ichiroh Takiguchi

On 2019-01-28 22:11, Lindenmaier, Goetz wrote:

Hi Ichiroh,

just open a bug, like "Fix aix build after 8214533" and post a RFR for 
it.

I assume the fix is quite trivial so we can review it quick.

Best regards,
  Goetz.


-Original Message-
From: ppc-aix-port-dev  On
Behalf Of Ichiroh Takiguchi
Sent: Montag, 28. Januar 2019 14:13
To: Baesken, Matthias 
Cc: build-dev ; ppc-aix-port-dev 

d...@openjdk.java.net>; core-libs-...@openjdk.java.net; Alan Bateman

Subject: RE: RFR: 8214533 IBM-29626C is required for AIX default 
charset


Hello.

I'm very sorry. It's my fault.
EUC_JP class was moved to java.base module.
(sun.nio.cs.EUC_JP).

make/data/charsetmapping/stdcs-aix should have EUC_JP_LINUX and
EUC_JP_Open.

Could you suggest me how I should provide new webrev files ?

Thanks,
Ichiroh Takiguchi


On 2019-01-28 17:03, Baesken, Matthias wrote:
> Hello,   seems  8214533   got pushed  recently  into  jdk/jdk.   Now
> we see build errors on AIX  ,  are they related to  this change ?
>
>
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:63:
> error: Decoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Decoder extends EUC_JP.Decoder {
>^
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:69:
> error: Encoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Encoder extends EUC_JP.Encoder {
>^
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:65:
> error: Decoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Decoder extends EUC_JP.Decoder {
>^
> /nb/rs6000_64/nightly/output-jdk-
test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:85:
> error: Encoder is not public in EUC_JP; cannot be accessed from
> outside package
> private static class Encoder extends EUC_JP.Encoder {
>
> Best regards, Matthias
>
>
>
>> -Original Message-
>> From: ppc-aix-port-dev  On
>> Behalf Of Ichiroh Takiguchi
>> Sent: Dienstag, 15. Januar 2019 01:51
>> To: Alan Bateman 
>> Cc: build-dev ; ppc-aix-port-dev > port-...@openjdk.java.net>; core-libs-...@openjdk.java.net
>> Subject: Re: RFR: 8214533 IBM-29626C is required for AIX default
>> charset
>>
>> Hello Alan.
>>
>> Could you review the fix again ?
>>
>> Bug:https://bugs.openjdk.java.net/browse/JDK-8214533
>> Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.01/
>>
>> I added IBM29626C charset as standard way.
>> Please give any suggestion and question.
>>
>> Thanks,
>> Ichiroh Takiguchi
>> IBM Japan, Ltd.
>>
>> On 2018-12-14 18:58, Ichiroh Takiguchi wrote:
>> > Hello Alan.
>> >
>> > I opened JDK-8215333 for Charset filtering issue [1].
>> > I cannot wait until JDK-8215333 is closed.
>> > Is it possible to put IBM-29626C charset with standard way ?
>> >
>> > [1] https://bugs.openjdk.java.net/browse/JDK-8215333
>> >
>> > Thanks,
>> > Ichiroh Takiguchi
>> >
>> > On 2018-12-10 21:21, Ichiroh Takiguchi wrote:
>> >> Hello Roger, Magnus and Alan.
>> >> I may need to put alias information into charsets file.
>> >> stdcs-xxx cannot handle this information...
>> >>
>> >> Still AIX needs IBM-29626C charset for default encoding...
>> >>
>> >> I appreciate if you give me further suggestions.
>> >>
>> >> Thanks,
>> >> Ichiroh Takiguchi
>> >>
>> >> On 2018-12-10 20:50, Alan Bateman wrote:
>> >>> On 10/12/2018 11:01, Magnus Ihse Bursie wrote:
>> >>>> On 2018-12-07 21:20, Roger Riggs wrote:
>> >>>>> Hi,
>> >>>>>
>> >>>>> It is a nice feature that charsets are selected at build time using
>> >>>>> the stdcs-xxx files.
>> >>>>> This change breaks that pattern and embeds os specific information
>> >>>>> in more than one place.
>> >>>>> That does not seem like an improvement.  Is there any alternative?
>> >>>> I agree. Why is it not enough just to add it to stdcs-aix?
>> >>> My reading of the patch is that the "os" key is to avoid generating
>> >>> it
>> >>> on non-AIX platforms, it will otherwise end up in jdk.charsets on
>> >>> non-AIX platforms. The general direction is welcome but I think
>> >>> further work and discussion will be needed to get the right set of
>> >>> changes to support filtering in the build. It can probably be
>> >>> separated from the changes to add IBM-29626C to AIX's java.base.
>> >>>
>> >>> -Alan

RE: RFR: 8214533 IBM-29626C is required for AIX default charset

[Ichiroh Takiguchi]

Hello.

I'm very sorry. It's my fault.
EUC_JP class was moved to java.base module.
(sun.nio.cs.EUC_JP).

make/data/charsetmapping/stdcs-aix should have EUC_JP_LINUX and 
EUC_JP_Open.


Could you suggest me how I should provide new webrev files ?

Thanks,
Ichiroh Takiguchi


On 2019-01-28 17:03, Baesken, Matthias wrote:

Hello,   seems  8214533   got pushed  recently  into  jdk/jdk.   Now
we see build errors on AIX  ,  are they related to  this change ?


/nb/rs6000_64/nightly/output-jdk-test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:63:
error: Decoder is not public in EUC_JP; cannot be accessed from
outside package
private static class Decoder extends EUC_JP.Decoder {
   ^
/nb/rs6000_64/nightly/output-jdk-test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_LINUX.java:69:
error: Encoder is not public in EUC_JP; cannot be accessed from
outside package
private static class Encoder extends EUC_JP.Encoder {
   ^
/nb/rs6000_64/nightly/output-jdk-test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:65:
error: Decoder is not public in EUC_JP; cannot be accessed from
outside package
private static class Decoder extends EUC_JP.Decoder {
   ^
/nb/rs6000_64/nightly/output-jdk-test/support/gensrc/jdk.charsets/sun/nio/cs/ext/EUC_JP_Open.java:85:
error: Encoder is not public in EUC_JP; cannot be accessed from
outside package
private static class Encoder extends EUC_JP.Encoder {

Best regards, Matthias




-Original Message-
From: ppc-aix-port-dev  On
Behalf Of Ichiroh Takiguchi
Sent: Dienstag, 15. Januar 2019 01:51
To: Alan Bateman 
Cc: build-dev ; ppc-aix-port-dev ; core-libs-...@openjdk.java.net
Subject: Re: RFR: 8214533 IBM-29626C is required for AIX default 
charset


Hello Alan.

Could you review the fix again ?

Bug:https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.01/

I added IBM29626C charset as standard way.
Please give any suggestion and question.

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

On 2018-12-14 18:58, Ichiroh Takiguchi wrote:
> Hello Alan.
>
> I opened JDK-8215333 for Charset filtering issue [1].
> I cannot wait until JDK-8215333 is closed.
> Is it possible to put IBM-29626C charset with standard way ?
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8215333
>
> Thanks,
> Ichiroh Takiguchi
>
> On 2018-12-10 21:21, Ichiroh Takiguchi wrote:
>> Hello Roger, Magnus and Alan.
>> I may need to put alias information into charsets file.
>> stdcs-xxx cannot handle this information...
>>
>> Still AIX needs IBM-29626C charset for default encoding...
>>
>> I appreciate if you give me further suggestions.
>>
>> Thanks,
>> Ichiroh Takiguchi
>>
>> On 2018-12-10 20:50, Alan Bateman wrote:
>>> On 10/12/2018 11:01, Magnus Ihse Bursie wrote:
>>>> On 2018-12-07 21:20, Roger Riggs wrote:
>>>>> Hi,
>>>>>
>>>>> It is a nice feature that charsets are selected at build time using
>>>>> the stdcs-xxx files.
>>>>> This change breaks that pattern and embeds os specific information
>>>>> in more than one place.
>>>>> That does not seem like an improvement.  Is there any alternative?
>>>> I agree. Why is it not enough just to add it to stdcs-aix?
>>> My reading of the patch is that the "os" key is to avoid generating
>>> it
>>> on non-AIX platforms, it will otherwise end up in jdk.charsets on
>>> non-AIX platforms. The general direction is welcome but I think
>>> further work and discussion will be needed to get the right set of
>>> changes to support filtering in the build. It can probably be
>>> separated from the changes to add IBM-29626C to AIX's java.base.
>>>
>>> -Alan

Re: RFR: 8214533 IBM-29626C is required for AIX default charset

[Ichiroh Takiguchi]

Hello Alan.

Could you review the fix again ?

Bug:https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.01/

I added IBM29626C charset as standard way.
Please give any suggestion and question.

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

On 2018-12-14 18:58, Ichiroh Takiguchi wrote:

Hello Alan.

I opened JDK-8215333 for Charset filtering issue [1].
I cannot wait until JDK-8215333 is closed.
Is it possible to put IBM-29626C charset with standard way ?

[1] https://bugs.openjdk.java.net/browse/JDK-8215333

Thanks,
Ichiroh Takiguchi

On 2018-12-10 21:21, Ichiroh Takiguchi wrote:

Hello Roger, Magnus and Alan.
I may need to put alias information into charsets file.
stdcs-xxx cannot handle this information...

Still AIX needs IBM-29626C charset for default encoding...

I appreciate if you give me further suggestions.

Thanks,
Ichiroh Takiguchi

On 2018-12-10 20:50, Alan Bateman wrote:

On 10/12/2018 11:01, Magnus Ihse Bursie wrote:

On 2018-12-07 21:20, Roger Riggs wrote:

Hi,

It is a nice feature that charsets are selected at build time using 
the stdcs-xxx files.
This change breaks that pattern and embeds os specific information 
in more than one place.

That does not seem like an improvement.  Is there any alternative?

I agree. Why is it not enough just to add it to stdcs-aix?
My reading of the patch is that the "os" key is to avoid generating 
it

on non-AIX platforms, it will otherwise end up in jdk.charsets on
non-AIX platforms. The general direction is welcome but I think
further work and discussion will be needed to get the right set of
changes to support filtering in the build. It can probably be
separated from the changes to add IBM-29626C to AIX's java.base.

-Alan

Re: RFR: 8214533 IBM-29626C is required for AIX default charset

[Ichiroh Takiguchi]

Hello Alan.

I opened JDK-8215333 for Charset filtering issue [1].
I cannot wait until JDK-8215333 is closed.
Is it possible to put IBM-29626C charset with standard way ?

[1] https://bugs.openjdk.java.net/browse/JDK-8215333

Thanks,
Ichiroh Takiguchi

On 2018-12-10 21:21, Ichiroh Takiguchi wrote:

Hello Roger, Magnus and Alan.
I may need to put alias information into charsets file.
stdcs-xxx cannot handle this information...

Still AIX needs IBM-29626C charset for default encoding...

I appreciate if you give me further suggestions.

Thanks,
Ichiroh Takiguchi

On 2018-12-10 20:50, Alan Bateman wrote:

On 10/12/2018 11:01, Magnus Ihse Bursie wrote:

On 2018-12-07 21:20, Roger Riggs wrote:

Hi,

It is a nice feature that charsets are selected at build time using 
the stdcs-xxx files.
This change breaks that pattern and embeds os specific information 
in more than one place.

That does not seem like an improvement.  Is there any alternative?

I agree. Why is it not enough just to add it to stdcs-aix?

My reading of the patch is that the "os" key is to avoid generating it
on non-AIX platforms, it will otherwise end up in jdk.charsets on
non-AIX platforms. The general direction is welcome but I think
further work and discussion will be needed to get the right set of
changes to support filtering in the build. It can probably be
separated from the changes to add IBM-29626C to AIX's java.base.

-Alan

Re: RFR: 8214533 IBM-29626C is required for AIX default charset

[Ichiroh Takiguchi]

Hello Roger, Magnus and Alan.
I may need to put alias information into charsets file.
stdcs-xxx cannot handle this information...

Still AIX needs IBM-29626C charset for default encoding...

I appreciate if you give me further suggestions.

Thanks,
Ichiroh Takiguchi

On 2018-12-10 20:50, Alan Bateman wrote:

On 10/12/2018 11:01, Magnus Ihse Bursie wrote:

On 2018-12-07 21:20, Roger Riggs wrote:

Hi,

It is a nice feature that charsets are selected at build time using 
the stdcs-xxx files.
This change breaks that pattern and embeds os specific information in 
more than one place.

That does not seem like an improvement.  Is there any alternative?

I agree. Why is it not enough just to add it to stdcs-aix?

My reading of the patch is that the "os" key is to avoid generating it
on non-AIX platforms, it will otherwise end up in jdk.charsets on
non-AIX platforms. The general direction is welcome but I think
further work and discussion will be needed to get the right set of
changes to support filtering in the build. It can probably be
separated from the changes to add IBM-29626C to AIX's java.base.

-Alan

Re: RFR: 8212794 IBM-964 and IBM-29626C are required for AIX default charset

[Ichiroh Takiguchi]

Hello.

Could you review the fix ?

Bug:https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.00/

IBM29626C charset is required for AIX default charset.
Java cannot start because of java/lang/ExceptionInInitializerError on 
AIX ja_JP locale.


To build team,
I'd like to change following charsetmapping tool.
* make/jdk/src/classes/build/tools/charsetmapping/Main.java
* make/jdk/src/classes/build/tools/charsetmapping/SPI.java
* make/jdk/src/classes/build/tools/charsetmapping/SRC.java

build.tools.charsetmapping,Main supports "os" tag, but it seems it's not 
used.

Currently, "os" supports "windows" or "unix".
I extended "os" tag's feature.
If "os aix" is there, this charset is only added into AIX platform.
(I assume "type template" should be used)
"aix" comes from "stdcs-aix" file name.
==
charset x-IBM29626C IBM29626C
package sun.nio.cs.ext
typetemplate
os  aix   <=
alias   cp29626C   # JDK historical
alias   ibm29626C
alias   ibm-29626C
alias   29626C
alias   ibm-eucjp
==

If cs.os is null,
this charset is stored into gensrc directory.
Charset name is added into StandardCharsets.java or 
ExtendedCharsets.java

If cs.os is "false",
this charset is NOT stored into gensrc directory.
Charset name is NOT added into StandardCharsets.java or 
ExtendedCharsets.java


"os" tag supports multiple entries by using ",", like "aix,linux"
Then we can store new charset into 
src/jdk.charsets/share/classes/sun/nio/cs/ext/ directory



$ locale charmap
IBM-eucJP
$ jshell
|  JShell  --  12-internal
|: /help intro

jshell> var cs = java.nio.charset.Charset.defaultCharset()
cs ==> x-IBM29626C

jshell> cs.getClass().getName()
$2 ==> "sun.nio.cs.IBM29626C"

jshell> System.out.println(String.join("\n", cs.aliases()))
cp29626C
ibm-29626C
ibm-eucjp
ibm29626C
29626C

jshell> /exit
|
$

I tested Linux and Windows build.
==
$ grep 29626 build.log
IBM29626C, x-IBM29626C, null, sun.nio.cs.ext, template  false

$ find support/gensrc/  | grep 29626

$ find support/gensrc/  | grep Charsets
support/gensrc/java.base/sun/nio/cs/StandardCharsets.java
support/gensrc/jdk.charsets/sun/nio/cs/ext/ExtendedCharsets.java

$ find support/gensrc/  | grep Charsets | xargs grep 29626

$
==

I'd like to obtain a sponsor for this issue.

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

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

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

[Ichiroh Takiguchi]

Hello Roger.

Thank you for your suggestion.

Could you review the fix again ?

Bug:https://bugs.openjdk.java.net/browse/JDK-8212794
Change: https://cr.openjdk.java.net/~itakiguchi/8212794/webrev.04/

Thanks,
Ichiroh Takiguchi

On 2018-12-04 23:36, Roger Riggs wrote:

Hi Ichiroh,

On 12/03/2018 10:30 PM, Ichiroh Takiguchi wrote:

Hello Roger.

Thank you for your suggestion.


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.
"import sun.nio.cs.*;" is required because of 
SimpleEUCEncoder.java.template.
SimpleEUCEncoder.java.template has conversion loop and IBM964 refers 
it.

It means that,
* On AIX platform, SimpleEUCEncoder should be in sun.nio.cs package
* On non-AIX platform, SimpleEUCEncoder should be in sun.nio.cs.ext 
package

I could not determine which package has SimpleEUCEncoder.
And same kind code is available on ISO2022_JP.java.
Please let me know if you know another way.

I understand, it is because IBM33722 may or may not be in the same
package as SimpleEUCEncoder.
It is SimpleEUCEncoder that may be in a different package, not 
IBM33722.



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.

I'll changed.

226-227:  add a space before "{" to have the same style as line 210.



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

Yes, it's no need.

Could you review the fix again ?
Bug:    https://bugs.openjdk.java.net/browse/JDK-8212794
Change: https://cr.openjdk.java.net/~itakiguchi/8212794/webrev.03/


Thanks, looks fine.

Regards, Roger



Thanks,
Ichiroh Takiguchi

On 2018-12-04 05:50, Roger Riggs wrote:

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/

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

[Ichiroh Takiguchi]

Hello Magnus.

IBM33722 should be in sun.nio.cs.ext package on jdk.charset module
Because it's not used for default encoding on AIX platform.

In case of template file, build tool checks 
make/data/charsetmapping/stdcs-aix file for this case.

If class name is there, it will be migrated to sun.nio.cs package.
About IBM33722,
If IBM33722 is moved to sun.nio.cs package also,
"import sun.nio.cs.*;" is no need on IBM33722.java
If IBM33722 is not in stdcs-aix,
"import sun.nio.cs.*;" is still required on IBM33722.java

Thanks,
Ichiroh Takiguchi

On 2018-12-04 19:42, Magnus Ihse Bursie wrote:

On 2018-12-04 04:30, Ichiroh Takiguchi wrote:

Hello Roger.

Thank you for your suggestion.


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.
"import sun.nio.cs.*;" is required because of 
SimpleEUCEncoder.java.template.
SimpleEUCEncoder.java.template has conversion loop and IBM964 refers 
it.

It means that,
* On AIX platform, SimpleEUCEncoder should be in sun.nio.cs package
* On non-AIX platform, SimpleEUCEncoder should be in sun.nio.cs.ext 
package

I could not determine which package has SimpleEUCEncoder.
And same kind code is available on ISO2022_JP.java.
Please let me know if you know another way.

I'm not sure I'm fully following the template intricacies here, but
would this not be solved if IBM33722.java was made a template as well?
Then you could do
import $PACKAGE$. SimpleEUCEncoder
Or so I'd think.

/Magnus



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.

I'll changed.


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

Yes, it's no need.

Could you review the fix again ?
Bug:https://bugs.openjdk.java.net/browse/JDK-8212794
Change: https://cr.openjdk.java.net/~itakiguchi/8212794/webrev.03/

Thanks,
Ichiroh Takiguchi

On 2018-12-04 05:50, Roger Riggs wrote:

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 modifica

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

[Ichiroh Takiguchi]

Hello Roger.

Thank you for your suggestion.


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.
"import sun.nio.cs.*;" is required because of 
SimpleEUCEncoder.java.template.

SimpleEUCEncoder.java.template has conversion loop and IBM964 refers it.
It means that,
* On AIX platform, SimpleEUCEncoder should be in sun.nio.cs package
* On non-AIX platform, SimpleEUCEncoder should be in sun.nio.cs.ext 
package

I could not determine which package has SimpleEUCEncoder.
And same kind code is available on ISO2022_JP.java.
Please let me know if you know another way.


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.

I'll changed.


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

Yes, it's no need.

Could you review the fix again ?
Bug:https://bugs.openjdk.java.net/browse/JDK-8212794
Change: https://cr.openjdk.java.net/~itakiguchi/8212794/webrev.03/

Thanks,
Ichiroh Takiguchi

On 2018-12-04 05:50, Roger Riggs wrote:

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" 
re

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

[Ichiroh Takiguchi]

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 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

Re: RFR: 8212794 IBM-964 and IBM-29626C are required for AIX default charset

[Ichiroh Takiguchi]

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

RE: RFR : 8204541 Correctly support AIX xlC 13.1 symbol visibility flags

[Ichiroh Takiguchi]

Hello Christoph

According to build log, if <#include "osSupport.hpp"> was there:
"/home/jdktest/sandbox/jdk/build/aix-ppc64-normal-server-release/support/headers/java.base/jdk_internal_jimage_NativeImageBuffer.h", 
line 15.27: 1540-0040 (S) The text
 "Java_jdk_internal_jimage_NativeImageBuffer_getNativeMap" is 
unexpected.  "visibility" may be undeclared or ambiguous.
make[3]: *** 
[/home/jdktest/sandbox/jdk/build/aix-ppc64-normal-server-release/support/native/java.base/libjimage/NativeImageBuffer.o] 
Error 1

make[3]: Leaving directory `/home/jdktest/sandbox/jdk/make'
make[2]: *** [java.base-libs] Error 2
make[2]: *** Waiting for unfinished jobs

On 2018-06-07 22:06, Langer, Christoph wrote:

Hi Ichiroh,

what's the exact error message if you  #include "osSupport.hpp"? (I
have no xlC 13 at hand to try myself...)

Best regards
Christoph


-Original Message-
From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
Sent: Donnerstag, 7. Juni 2018 14:53
To: build-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net; 
core-

libs-...@openjdk.java.net
Cc: Lindenmaier, Goetz ; Baesken, Matthias
; Langer, Christoph

Subject: RFR : 8204541 Correctly support AIX xlC 13.1 symbol 
visibility flags


Hello.

Could you review it ?
Bug:https://bugs.openjdk.java.net/browse/JDK-8204541
Change: http://cr.openjdk.java.net/~clanger/webrevs/8204541.0/

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

 Original Message 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support 
on

xlc 12.1
Date: 2018-06-07 20:43
 From: "Langer, Christoph" 
To: Ichiroh Takiguchi 
Cc: "'build-dev@openjdk.java.net'" ,
"ppc-aix-port-...@openjdk.java.net" ,
"core-libs-...@openjdk.java.net" ,
"Lindenmaier, Goetz" , "Baesken, Matthias"


Hi Ichiroh,

your proposal seems to make sense. I have created a bug for this:
https://bugs.openjdk.java.net/browse/JDK-8204541

Can you please generate a webrev (referencing this bug, -c option of
webrev.ksh) and mail it over to me. Then I'll upload it and you can 
post

an official RFR mail.

Best regards
Christoph

> -Original Message-
> From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
> Sent: Dienstag, 5. Juni 2018 08:59
> To: Baesken, Matthias 
> Cc: Langer, Christoph ; 'build-
> d...@openjdk.java.net' ; ppc-aix-port-
> d...@openjdk.java.net; core-libs-...@openjdk.java.net; Lindenmaier,
> Goetz 
> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on
> xlc 12.1
>
> Hello Matthias and Christoph.
> Thank you for your explanations.
>
> I did not have enough knowledge about "visibility".
>
> I created following patches.
>
> 
> diff -r 02934b0d661b
> src/java.base/share/native/libjimage/NativeImageBuffer.cpp
> --- a/src/java.base/share/native/libjimage/NativeImageBuffer.cpp   Wed
> May
> 30 14:46:28 2018 +0200
> +++ b/src/java.base/share/native/libjimage/NativeImageBuffer.cpp
Tue
> Jun
> 05 12:10:41 2018 +0900
> @@ -39,7 +39,9 @@
>   #include "imageFile.hpp"
>   #include "inttypes.hpp"
>   #include "jimage.hpp"
> +#if !defined(_AIX)
>   #include "osSupport.hpp"
> +#endif
>
>   #include "jdk_internal_jimage_NativeImageBuffer.h"
>
> diff -r 02934b0d661b src/java.base/unix/native/include/jni_md.h
> --- a/src/java.base/unix/native/include/jni_md.h   Wed May 30 14:46:28
> 2018 +0200
> +++ b/src/java.base/unix/native/include/jni_md.h   Tue Jun 05 12:10:41
> 2018 +0900
> @@ -29,7 +29,8 @@
>   #ifndef __has_attribute
> #define __has_attribute(x) 0
>   #endif
> -#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
> (__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
> +#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
> (__GNUC_MINOR__ > 2))) || __has_attribute(visibility) \
> +|| (defined(_AIX) && (defined(__xlC__) && (__xlC__ >= 0xD01)))
> #ifdef ARM
>   #define JNIEXPORT
> __attribute__((externally_visible,visibility("default")))
>   #define JNIIMPORT
> __attribute__((externally_visible,visibility("default")))
> 
>
> If "osSupport.hpp" was included, XLC++ compiler complained.
> I could not understand, which part was invalid...
> I'm not sure, "osSupport.hpp" is really required on
> NativeImageBuffer.cpp or not...
>
> I added __xlC__ macro to determine XLC/C++'s version# into jni_md.h.
> [1]
> 0xD01 means 13.1 by hexadecimal.
>
> I checked symbol table by "dump -Tv -X64". [2]
> It s

RFR : 8204541 Correctly support AIX xlC 13.1 symbol visibility flags

[Ichiroh Takiguchi]

Hello.

Could you review it ?
Bug:https://bugs.openjdk.java.net/browse/JDK-8204541
Change: http://cr.openjdk.java.net/~clanger/webrevs/8204541.0/

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

 Original Message 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on 
xlc 12.1

Date: 2018-06-07 20:43
From: "Langer, Christoph" 
To: Ichiroh Takiguchi 
Cc: "'build-dev@openjdk.java.net'" , 
"ppc-aix-port-...@openjdk.java.net" , 
"core-libs-...@openjdk.java.net" , 
"Lindenmaier, Goetz" , "Baesken, Matthias" 



Hi Ichiroh,

your proposal seems to make sense. I have created a bug for this: 
https://bugs.openjdk.java.net/browse/JDK-8204541


Can you please generate a webrev (referencing this bug, -c option of 
webrev.ksh) and mail it over to me. Then I'll upload it and you can post 
an official RFR mail.


Best regards
Christoph


-Original Message-
From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
Sent: Dienstag, 5. Juni 2018 08:59
To: Baesken, Matthias 
Cc: Langer, Christoph ; 'build-
d...@openjdk.java.net' ; ppc-aix-port-
d...@openjdk.java.net; core-libs-...@openjdk.java.net; Lindenmaier,
Goetz 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on 
xlc 12.1


Hello Matthias and Christoph.
Thank you for your explanations.

I did not have enough knowledge about "visibility".

I created following patches.


diff -r 02934b0d661b
src/java.base/share/native/libjimage/NativeImageBuffer.cpp
--- a/src/java.base/share/native/libjimage/NativeImageBuffer.cppWed
May
30 14:46:28 2018 +0200
+++ b/src/java.base/share/native/libjimage/NativeImageBuffer.cpp	Tue 
Jun

05 12:10:41 2018 +0900
@@ -39,7 +39,9 @@
  #include "imageFile.hpp"
  #include "inttypes.hpp"
  #include "jimage.hpp"
+#if !defined(_AIX)
  #include "osSupport.hpp"
+#endif

  #include "jdk_internal_jimage_NativeImageBuffer.h"

diff -r 02934b0d661b src/java.base/unix/native/include/jni_md.h
--- a/src/java.base/unix/native/include/jni_md.hWed May 30 14:46:28
2018 +0200
+++ b/src/java.base/unix/native/include/jni_md.hTue Jun 05 12:10:41
2018 +0900
@@ -29,7 +29,8 @@
  #ifndef __has_attribute
#define __has_attribute(x) 0
  #endif
-#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
(__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
+#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
(__GNUC_MINOR__ > 2))) || __has_attribute(visibility) \
+|| (defined(_AIX) && (defined(__xlC__) && (__xlC__ >= 0xD01)))
#ifdef ARM
  #define JNIEXPORT
__attribute__((externally_visible,visibility("default")))
  #define JNIIMPORT
__attribute__((externally_visible,visibility("default")))


If "osSupport.hpp" was included, XLC++ compiler complained.
I could not understand, which part was invalid...
I'm not sure, "osSupport.hpp" is really required on
NativeImageBuffer.cpp or not...

I added __xlC__ macro to determine XLC/C++'s version# into jni_md.h. 
[1]

0xD01 means 13.1 by hexadecimal.

I checked symbol table by "dump -Tv -X64". [2]
It seemed it was fine, some symbols were hidden.

Does someone review my code?

[1]
https://www.ibm.com/support/knowledgecenter/en/SSGH2K_13.1.3/com.i
bm.xlc1313.aix.doc/compiler_ref/xlmacros.html
[2]
https://www.ibm.com/developerworks/aix/library/au-aix-symbol-
visibility/index.html

On 2018-06-01 21:12, Baesken, Matthias wrote:
> Hi ,  my change  8202322   just  handled  the fact   that  the
> visibility - flags   are not supported  with  xlc 12.1  ,  so  setting
> them generated a TON of compile - time  warnings .
>
> The introduction of   the  "-qvisibility=hidden"came with the
> mapfile removal changes :
>
> 8200358: Remove mapfiles for JDK executables
> http://hg.openjdk.java.net/jdk/jdk/rev/210cf224b690
>
> 8200178: Remove mapfiles for JDK native libraries
> http://hg.openjdk.java.net/jdk/jdk/rev/396ea30afbd5
>
> I guess it  might  need further  testing+adjustments  to make  the
> "visibility hiding" work nicely   with XLC13  ,  but currently  we
> build only with XLC12 .
>
> As a workaround  you might want to  remove  the  "-qvisibility=hidden"
>  setting for XLC 13 as well  ,  like I did  for XLC12 with  the change
>   8202322   .
>
>
> Best regards, Matthias
>
>
>
>
>> -Original Message-
>> From: Langer, Christoph
>> Sent: Freitag, 1. Juni 2018 10:57
>> To: Ichiroh Takiguchi 
>> Cc: Baesken, Matthias ; 'build-
>> d...@openjdk.java.net' ; ppc-aix-port-
>> d...@openjdk.java.net; core-libs-...@openjdk.java.net; Lin

RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

[Ichiroh Takiguchi]

Hello Matthias and Christoph.
Thank you for your explanations.

I did not have enough knowledge about "visibility".

I created following patches.


diff -r 02934b0d661b 
src/java.base/share/native/libjimage/NativeImageBuffer.cpp
--- a/src/java.base/share/native/libjimage/NativeImageBuffer.cpp	Wed May 
30 14:46:28 2018 +0200
+++ b/src/java.base/share/native/libjimage/NativeImageBuffer.cpp	Tue Jun 
05 12:10:41 2018 +0900

@@ -39,7 +39,9 @@
 #include "imageFile.hpp"
 #include "inttypes.hpp"
 #include "jimage.hpp"
+#if !defined(_AIX)
 #include "osSupport.hpp"
+#endif

 #include "jdk_internal_jimage_NativeImageBuffer.h"

diff -r 02934b0d661b src/java.base/unix/native/include/jni_md.h
--- a/src/java.base/unix/native/include/jni_md.h	Wed May 30 14:46:28 
2018 +0200
+++ b/src/java.base/unix/native/include/jni_md.h	Tue Jun 05 12:10:41 
2018 +0900

@@ -29,7 +29,8 @@
 #ifndef __has_attribute
   #define __has_attribute(x) 0
 #endif
-#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) && 
(__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
+#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) && 
(__GNUC_MINOR__ > 2))) || __has_attribute(visibility) \

+|| (defined(_AIX) && (defined(__xlC__) && (__xlC__ >= 0xD01)))
   #ifdef ARM
 #define JNIEXPORT 
__attribute__((externally_visible,visibility("default")))
 #define JNIIMPORT 
__attribute__((externally_visible,visibility("default")))



If "osSupport.hpp" was included, XLC++ compiler complained.
I could not understand, which part was invalid...
I'm not sure, "osSupport.hpp" is really required on 
NativeImageBuffer.cpp or not...


I added __xlC__ macro to determine XLC/C++'s version# into jni_md.h. [1]
0xD01 means 13.1 by hexadecimal.

I checked symbol table by "dump -Tv -X64". [2]
It seemed it was fine, some symbols were hidden.

Does someone review my code?

[1] 
https://www.ibm.com/support/knowledgecenter/en/SSGH2K_13.1.3/com.ibm.xlc1313.aix.doc/compiler_ref/xlmacros.html
[2] 
https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility/index.html


On 2018-06-01 21:12, Baesken, Matthias wrote:

Hi ,  my change  8202322   just  handled  the fact   that  the
visibility - flags   are not supported  with  xlc 12.1  ,  so  setting
them generated a TON of compile - time  warnings .

The introduction of   the  "-qvisibility=hidden"came with the
mapfile removal changes :

8200358: Remove mapfiles for JDK executables
http://hg.openjdk.java.net/jdk/jdk/rev/210cf224b690

8200178: Remove mapfiles for JDK native libraries
http://hg.openjdk.java.net/jdk/jdk/rev/396ea30afbd5

I guess it  might  need further  testing+adjustments  to make  the
"visibility hiding" work nicely   with XLC13  ,  but currently  we
build only with XLC12 .

As a workaround  you might want to  remove  the  "-qvisibility=hidden"
 setting for XLC 13 as well  ,  like I did  for XLC12 with  the change
  8202322   .


Best regards, Matthias





-Original Message-
From: Langer, Christoph
Sent: Freitag, 1. Juni 2018 10:57
To: Ichiroh Takiguchi 
Cc: Baesken, Matthias ; 'build-
d...@openjdk.java.net' ; ppc-aix-port-
d...@openjdk.java.net; core-libs-...@openjdk.java.net; Lindenmaier,
Goetz 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support 
on xlc 12.1


Hi Ichiroh,

we do not use the XLC 13 compiler on AIX yet here at SAP and I believe
nobody of my colleagues has played with it yet. So you are on a new
playground here 😊

However, I believe the idea in OpenJDK with the abolition of map files 
is that
symbols should be invisible externally unless they are declared 
exported,
e.g. JNIEXPORT. So I would think "-qvisibility=hidden" should be the 
correct

default and whatever JNIEXPORT expands to should contain the right
attributes to get that symbol visible.

Can you check if either my assumption is completely wrong, JNIEXPORT 
does
not expand to the right thing, XLC 13 has a bug or maybe just sume 
specific

required symbols are not declared correctly?

Best regards
Christoph

> -Original Message-
> From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
> Sent: Donnerstag, 31. Mai 2018 09:55
> To: Langer, Christoph 
> Cc: Baesken, Matthias ; 'build-
> d...@openjdk.java.net' ; ppc-aix-port-
> d...@openjdk.java.net; core-libs-...@openjdk.java.net; Lindenmaier,
> Goetz 
> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 
12.1
>
> Hello.
> 8202322 was integrated into jdk-11+15.
> I'm using XLC 13.1.3 on AIX 7.1.4.
> Build was failed because of "-qvisibility=hidden" on
> make/lib/LibCommon.gmk.
> According to "XL C/C++ for AIX 13.1.

RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

[Ichiroh Takiguchi]

Hello.
8202322 was integrated into jdk-11+15.
I'm using XLC 13.1.3 on AIX 7.1.4.
Build was failed because of "-qvisibility=hidden" on 
make/lib/LibCommon.gmk.

According to "XL C/C++ for AIX 13.1.3" documentation [1],
"-qvisibility=hidden" cannot create shared libraries entry points.
For example, libverify.so was there, but entry points were not resolved 
by "-lverify" option.

I think it should be "-qvisibility=default" (I tried, it worked)
or "-qvisibility=protected" (I had not tried) ?
I'm not familiar with -qvisibility option, but I'd like to find out 
right way.


[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_13.1.3/com.ibm.xlcpp1313.aix.doc/compiler_ref/opt_visibility.html


On 2018-05-16 16:08, Langer, Christoph wrote:

Hi Matthias,

yes, reviewed.

Best regards
Christoph

From: Baesken, Matthias
Sent: Mittwoch, 16. Mai 2018 09:06
To: Langer, Christoph ;
'build-dev@openjdk.java.net' ;
ppc-aix-port-...@openjdk.java.net; core-libs-...@openjdk.java.net
Cc: Lindenmaier, Goetz 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on 
xlc 12.1


Hi  Christoph can I add you as second reviewer  (other reviewer was
Erik Joelsson) can push the change ?

Best regards, Matthias



From: Langer, Christoph
Sent: Donnerstag, 26. April 2018 16:38
To: Baesken, Matthias
mailto:matthias.baes...@sap.com>>;
'build-dev@openjdk.java.net'
mailto:build-dev@openjdk.java.net>>;
ppc-aix-port-...@openjdk.java.net;
core-libs-...@openjdk.java.net
Cc: Simonis, Volker 
mailto:volker.simo...@sap.com>>
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on 
xlc 12.1


Hi Matthias,

to me the change in principal looks good.

I'm wondering if it is possible to do a comparison like xlc < 13 (e.g.
extract major number before the first dot, then compare numerically) -
but maybe it is too complicated and the current single version compare
suits our needs ?

Best regards
Christoph

From: Baesken, Matthias
Sent: Donnerstag, 26. April 2018 16:14
To: 'build-dev@openjdk.java.net'
mailto:build-dev@openjdk.java.net>>;
ppc-aix-port-...@openjdk.java.net;
core-libs-...@openjdk.java.net
Cc: Langer, Christoph
mailto:christoph.lan...@sap.com>>; Simonis,
Volker mailto:volker.simo...@sap.com>>
Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 
12.1


Hello  ,  could you please review this small adjustment to  the symbol
visibility compilation settings on AIX ?
Currently  we use  XLC 12.1  to compile  JDK on AIX .

However XLC 12.1   does not support  the "-qvisibility=hidden"
setting currently set on AIX.
It was introduced with  XLC 13.1 . Christoph found some info about it 
here :


https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html

Setting it  only generates  hundreds  of warnings  in the build log ,
warnings look like this :
XlC12.1

bash-4.4$ xlC -qversion
IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)
Version: 12.01..0019

bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc
1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list
of valid options.

Compare to XLC13.1

bash-3.00$ xlC -qversion
IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07)
Version: 13.01..0008
bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc
bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc


So it is better to avoid  setting these flags when using xlc12.1   .
Please review :

Bug :

https://bugs.openjdk.java.net/browse/JDK-8202322

Change :

http://cr.openjdk.java.net/~mbaesken/webrevs/8202322/


Best regards, Matthias

Re: RFR: 8201429: Support AIX Input Method Editor (IME) for AWT Input Method Framework (IMF)

[Ichiroh Takiguchi]

Hello Christoph.

Our team tested your fixed code on Linux (RHEL7) and AIX (7.1).

resetCompositionState() was missing in 
src/java.desktop/aix/classes/sun/awt/X11InputMethod.java


--- a/src/java.desktop/aix/classes/sun/awt/X11InputMethod.java  Wed May 
09 09:05:32 2018 +0900
+++ b/src/java.desktop/aix/classes/sun/awt/X11InputMethod.java  Mon May 
14 21:25:50 2018 +0900

@@ -56,6 +56,21 @@
 }

 /**
+ * Reset the composition state to the current composition state.
+ */
+protected void resetCompositionState() {
+if (compositionEnableSupported && haveActiveClient()) {
+try {
+/* Restore the composition mode to the last saved 
composition

+   mode. */
+setCompositionEnabled(savedCompositionState);
+} catch (UnsupportedOperationException e) {
+compositionEnableSupported = false;
+}
+}
+}
+
+/**
  * Activate input method.
  */
 public synchronized void activate() {


Otherwise,
we could not find out any functional difference on Linux.
we could not find out any functional difference between our modified 
code

and your code on AIX.

By code check, we found following difference.

--- a/src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod.c 
Wed May 09 09:05:32 2018 +0900
+++ b/src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod.c 
Mon May 14 21:25:50 2018 +0900

@@ -248,6 +248,10 @@
  "flushText",
  "()V");
 JNU_CHECK_EXCEPTION_RETURN(env, NULL);
+if ((*env)->ExceptionOccurred(env)) {
+(*env)->ExceptionDescribe(env);
+(*env)->ExceptionClear(env);
+}
 /* IMPORTANT:
The order of the following calls is critical since 
"imInstance" may
point to the global reference itself, if 
"freeX11InputMethodData" is called



Did you remove this code intentionally ?

Otherwise, I think the other changes were acceptable.

Thanks,

On 2018-05-09 00:24, Erik Joelsson wrote:

Build change looks good.

/Erik


On 2018-05-08 02:54, Langer, Christoph wrote:

Hi Eric,

thanks for that excellent suggestion. I already thought there should 
be some means to do that but was not aware of how that could be 
accomplished. I updated the webrev in place.


Thanks
Christoph


-Original Message-
From: Erik Joelsson [mailto:erik.joels...@oracle.com]
Sent: Freitag, 4. Mai 2018 17:45
To: Langer, Christoph ; awt-
d...@openjdk.java.net
Cc: build-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
Subject: Re: RFR: 8201429: Support AIX Input Method Editor (IME) for 
AWT

Input Method Framework (IMF)

Hello,

It looks like what you are trying to achieve is to override
awt_InputMethod.c with an OS specific version of that file. We have a
construct for this in SetupNativeCompilation that should handle it
automatically, if you just list the source dirs in priority order. I
would suggest leveraging this by making this change instead:

First in the list of LIBAWT_XAWT_DIRS (line 272), prepend a line like 
this:


$(wildcard
$(TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS)/native/libawt_xawt)
\

/Erik


On 2018-05-04 07:07, Langer, Christoph wrote:

Hi,

please help reviewing the contribution of the support for the AIX 
Input

Method Editor (IME) in AWT's Input Method Framework.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201429.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8201429

I took Ichiroh's initial proposal [1] and tried to integrate it 
better with

existing code. I have split
src/java.desktop/unix/classes/sun/awt/X11InputMethod.java into

a) a base class containing the common code:

src/java.desktop/unix/classes/sun/awt/X11InputMethodBase.java

b) a specific class for the common Linux/Unixes:

src/java.desktop/unix/classes/sun/awt/X11InputMethod.java and

c) a specific class for AIX:

src/java.desktop/aix/classes/sun/awt/X11InputMethod.java

The AIX specific additions to the native code of
src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c were 
so

much that I decided to add a specific implementation file for AIX:
src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod_aix.c. 
The

changes to the former file are some cleanups.
I'm still in the process of testing the changes - but maybe you can 
run
further tests, especially on non-AIX unixes to make sure we didn't 
break

something.

Thanks & Best regards
Christoph

[1]: http://mail.openjdk.java.net/pipermail/awt-dev/2018-

April/013869.html