Re: [OpenJDK 2D-Dev] AWT Dev RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-26 Thread Iris Clark
Hi.

 http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/

 + src/solaris/classes/java/lang/UNIXProcess.java.aix
 2  * Copyright (c) 1995, 2013, Oracle and/or its affiliates. All rights 
 reserved.
 3  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

Oracle copyright in acceptable location and uses correct format.

 + src/aix/porting/porting_aix.h
 2  * Copyright 2012, 2013 SAP AG. All rights reserved.
 3  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

SAP copyright in acceptable location.

 + src/aix/native/sun/tools/attach/AixVirtualMachine.c
 2  * Copyright (c) 2008, 2013, Oracle and/or its affiliates. All rights 
 reserved.
 3  * Copyright 2013 SAP AG. All rights reserved.
 4  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER

Oracle and SAP copyrights in acceptable locations.  Oracle format is correct.

The licenses (GPL+CP) look fine to me.

Thanks,
iris

-Original Message-
From: Sergey Bylokhov 
Sent: Tuesday, November 26, 2013 10:52 AM
To: Alan Bateman; Volker Simonis
Cc: 2d-dev@openjdk.java.net; serviceability-...@openjdk.java.net; security-dev; 
ppc-aix-port-...@openjdk.java.net; awt-...@openjdk.java.net; Java Core Libs; 
net-dev
Subject: Re: AWT Dev RFR(L) - 2nd round: 8024854: Basic changes and files to 
build the class library on AIX

On 26.11.2013 21:03, Alan Bateman wrote:
 On 26/11/2013 16:23, Volker Simonis wrote:
 Hi,

 thanks to everybody for the prompt and helpful reviews. Here comes 
 the final webrev which incorporates all the corrections and 
 suggestions from the second review round:

 http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/

 I've successfully build (and run some smoke tests) with these changes 
 on Linux (x86_32, x86_64, ppc64), Solaris/sparcv9, Windows/x86_64, 
 MacOSX and AIX (5.3, 7.1).

 I've skimmed over the last webrev with focus on:

 NetworkingLibraries.gmk where I see this is now fixed for all platforms.

 net_util.* and the platform specific net_util_md.* where I see you've 
 added platformInit so it's much cleaner.
I have a question about boolean_t in the [1]. Is it correct to skip it in the 
new block?
http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/src/share/native/sun/security/ec/impl/ecc_impl.h.frames.html

Also I have a question about headers in the added files. Looks like different 
templates are used:

+ src/solaris/classes/java/lang/UNIXProcess.java.aix
2  * Copyright (c) 1995, 2013, Oracle and/or its affiliates. All rights 
reserved.
3  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ src/aix/porting/porting_aix.h
2  * Copyright 2012, 2013 SAP AG. All rights reserved.
3  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ src/aix/native/sun/tools/attach/AixVirtualMachine.c
2  * Copyright (c) 2008, 2013, Oracle and/or its affiliates. All rights 
reserved.
3  * Copyright 2013 SAP AG. All rights reserved.
4  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

etc..

Do we have some rules about that?

 UnixNativeDispatcher.c where the error translation is now removed (and 
 looks fine).

 So overall it looks good to me and should be pushed to the staging 
 forest once you hear from others that commented previously.

 -Alan


--
Best regards, Sergey.



Re: [OpenJDK 2D-Dev] AWT Dev RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-26 Thread Phil Race
Looking only at what you needed to change this time round, all seems 
fine now.


-phil.

On 11/26/13 8:23 AM, Volker Simonis wrote:

Hi,

thanks to everybody for the prompt and helpful reviews. Here comes the 
final webrev which incorporates all the corrections and suggestions 
from the second review round:


http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/ 
http://cr.openjdk.java.net/%7Esimonis/webrevs/8024854.v3/


I've successfully build (and run some smoke tests) with these changes 
on Linux (x86_32, x86_64, ppc64), Solaris/sparcv9, Windows/x86_64, 
MacOSX and AIX (5.3, 7.1).


So if nobody vetoes these changes are ready for push into our staging 
repository.


@Vladimir: can I push them or do you want to run them trough JPRT?

Thank you and best regards,
Volker

PS: compared to the last webrev 
(http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/ 
http://cr.openjdk.java.net/%7Esimonis/webrevs/8024854.v2/), I've 
slightly changed the following files:


makefiles/lib/Awt2dLibraries.gmk
makefiles/lib/NetworkingLibraries.gmk
makefiles/Setup.gmk
src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
src/aix/classes/sun/nio/ch/AixPollPort.java
src/aix/classes/sun/nio/fs/AixFileSystem.java
src/aix/native/java/net/aix_close.c
src/aix/porting/porting_aix.c
src/share/native/java/net/net_util.c
src/share/native/java/net/net_util.h
src/share/native/sun/awt/medialib/mlib_sys.c
src/solaris/bin/java_md_solinux.c
src/solaris/classes/sun/nio/ch/Port.java
src/solaris/native/java/net/net_util_md.c
src/solaris/native/sun/java2d/x11/XRBackendNative.c
src/solaris/native/sun/management/OperatingSystemImpl.c
src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c
src/windows/native/java/net/net_util_md.c

Most of the changes are cosmetic, except the ones in:

src/share/native/java/net/net_util.c
src/share/native/java/net/net_util.h
src/solaris/native/java/net/net_util_md.c
src/windows/native/java/net/net_util_md.c

where I renamed 'initLocalAddrTable()' to 'platformInit()'. For AIX, 
this method will now call 'aix_close_init()' as suggested by Alan.


The changes to src/solaris/native/com/sun/ 
management/UnixOperatingSystem_md.c are now in 
src/solaris/native/sun/management/OperatingSystemImpl.c because that 
file was moved by an upstream change.




On Wed, Nov 20, 2013 at 7:26 PM, Volker Simonis 
volker.simo...@gmail.com mailto:volker.simo...@gmail.com wrote:


Hi,

this is the second review round for 8024854: Basic changes and
files to build the class library on AIX
https://bugs.openjdk.java.net/browse/JDK-8024854. The previous
reviews can be found at the end of this mail in the references
section.

I've tried to address all the comments and suggestions from the
first round and to further streamline the patch (it perfectly
builds on Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and
Windows/x86_64). The biggest change compared to the first review
round is the new aix/ subdirectory which I've now created under
jdk/src and which contains AIX-only code.

The webrev is against our
http://hg.openjdk.java.net/ppc-aix-port/stage repository which has
been recently synchronised with the jdk8 master:

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/
http://cr.openjdk.java.net/%7Esimonis/webrevs/8024854.v2/

Below (and in the webrev) you can find some comments which explain
the changes to each file. In order to be able to push this big
change, I need the approval of reviewers from the lib, net, svc,
2d, awt and sec groups. So please don't hesitate to jump at it -
there are enough changes for everybody:)

Thank you and best regards,
Volker

*References:*

The following reviews have been posted so far (thanks Iris for
collecting them :)

- Alan Bateman (lib): Initial comments (16 Sep [2])
- Chris Hegarty (lib/net): Initial comments (20 Sep [3])
- Michael McMahon (net): Initial comments (20 Sept [4])
- Steffan Larsen (svc): APPROVED (20 Sept [5])
- Phil Race (2d): Initial comments  (18 Sept [6]); Additional
comments (15 Oct [7])
- Sean Mullan (sec): Initial comments (26 Sept [8])

[2]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
[3]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
[4]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
[5]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
[6]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
[7]:
http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
[8]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html


*Detailed change description:*

The new jdk/src/aix subdirectory contains the following new and
AIX-specific files for now:

  

Re: [OpenJDK 2D-Dev] AWT Dev RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-26 Thread Sergey Bylokhov

HI, Volker.
Thanks for clarification. The fix looks good.

On 11/27/13 12:48 AM, Volker Simonis wrote:

Hi Sergey,

on AIX, there's already a typedef for boolean_t in sys/types.h 
that's why we have to omit it from ecc_impl.h to avoid compile errors.


The question regarding the copyright headers has been already answered 
by Iris (thanks Iris).


Thank you and best regards,
Volker

On Tuesday, November 26, 2013, Sergey Bylokhov wrote:

On 26.11.2013 21:03, Alan Bateman wrote:

On 26/11/2013 16:23, Volker Simonis wrote:

Hi,

thanks to everybody for the prompt and helpful reviews.
Here comes the
final webrev which incorporates all the corrections and
suggestions from
the second review round:

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/
http://cr.openjdk.java.net/%7Esimonis/webrevs/8024854.v3/

I've successfully build (and run some smoke tests) with
these changes on
Linux (x86_32, x86_64, ppc64), Solaris/sparcv9,
Windows/x86_64, MacOSX and
AIX (5.3, 7.1).

I've skimmed over the last webrev with focus on:

NetworkingLibraries.gmk where I see this is now fixed for all
platforms.

net_util.* and the platform specific net_util_md.* where I see
you've added platformInit so it's much cleaner.

I have a question about boolean_t in the [1]. Is it correct to
skip it in the new block?

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/src/share/native/sun/security/ec/impl/ecc_impl.h.frames.html

http://cr.openjdk.java.net/%7Esimonis/webrevs/8024854.v3/src/share/native/sun/security/ec/impl/ecc_impl.h.frames.html

Also I have a question about headers in the added files. Looks
like different templates are used:

+ src/solaris/classes/java/lang/UNIXProcess.java.aix
   2  * Copyright (c) 1995, 2013, Oracle and/or its affiliates.
All rights reserved.
   3  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ src/aix/porting/porting_aix.h
   2  * Copyright 2012, 2013 SAP AG. All rights reserved.
   3  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ src/aix/native/sun/tools/attach/AixVirtualMachine.c
   2  * Copyright (c) 2008, 2013, Oracle and/or its affiliates.
All rights reserved.
   3  * Copyright 2013 SAP AG. All rights reserved.
   4  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

etc..

Do we have some rules about that?


UnixNativeDispatcher.c where the error translation is now
removed (and looks fine).

So overall it looks good to me and should be pushed to the
staging forest once you hear from others that commented
previously.

-Alan



-- 
Best regards, Sergey.





--
Best regards, Sergey.