Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-01-25 Thread Rob McKenna
Ack, apologies, thats what I get for rushing. (which I suppose I'm doing again) That code was actually supposed to be in the getDnsUrls method. Updated in place: http://cr.openjdk.java.net/~robm/8160768/webrev.02/ I'll add a couple of tests for the SecurityManager along with some input checking

Re: Updated RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-25 Thread Mandy Chung
> On Jan 25, 2017, at 12:58 PM, Henry Jen wrote: > > Updated webrev address feedbacks so far can be found at > http://cr.openjdk.java.net/~henryjen/jdk9/8170832/5/webrev/ Looks good. thanks Mandy

RFR: 8172309: classpath wildcards code does not support --class-path

2017-01-25 Thread Henry Jen
Hi, Please review the webrev[1], the fix is to ensure —class-path and —class-path= is processed correctly to expand wildcard. Changes are made in jdk repo. However, test case to verify the bug fix is in langtool repo. Cheers, Henry [1] http://cr.openjdk.java.net/~henryjen/jdk9/8172309/0/ [2]

Re: Updated RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-25 Thread Kumar Srinivasan
Thanks Henry, Looks good!. Kumar On Jan 25, 2017, at 12:14 PM, Henry Jen wrote: On Jan 25, 2017, at 11:32 AM, Kumar Srinivasan wrote: Hi Henry, I was somewhat surprised to see changes to launcher_LANG.properties, I usually make the

Updated RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-25 Thread Henry Jen
> On Jan 25, 2017, at 12:14 PM, Henry Jen wrote: > > >> On Jan 25, 2017, at 11:32 AM, Kumar Srinivasan >> wrote: >> >> >> Hi Henry, >> >> I was somewhat surprised to see changes to launcher_LANG.properties, I >> usually >> make the

Re: RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-25 Thread Henry Jen
> On Jan 25, 2017, at 11:32 AM, Kumar Srinivasan > wrote: > > > Hi Henry, > > I was somewhat surprised to see changes to launcher_LANG.properties, I usually > make the change in the english/default locale and allow the L1ON team to make > the > locale specific

Re: RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-25 Thread Kumar Srinivasan
Hi Henry, I was somewhat surprised to see changes to launcher_LANG.properties, I usually make the change in the english/default locale and allow the L1ON team to make the locale specific changes, but if you are confident of the changes, that is fine.

Re: RFR: 8173372 Add tests for multi-release module jar API validator

2017-01-25 Thread Paul Sandoz
> On 25 Jan 2017, at 10:54, Andrey Nazarov wrote: > > Hi, > > I’ve added tests to check what MR Jar API validator handles changes in > module-info.class > > Review: http://cr.openjdk.java.net/~anazarov/8173372/webrev.01/webrev/ > JBS:

Re: RFR: 8029019: (ann) Optimize annotation handling in core reflection

2017-01-25 Thread Peter Levart
Hi Claes, On 01/24/2017 03:34 PM, Claes Redestad wrote: Hi Peter, thanks for the thorough examination of this issue. Thanks for looking into this change... After going through the patch I do like what I see, but I have a few comments: AnnotationInvocationHandler: in invoke, cleaning up

RFR: 8173372 Add tests for multi-release module jar API validator

2017-01-25 Thread Andrey Nazarov
Hi, I’ve added tests to check what MR Jar API validator handles changes in module-info.class Review: http://cr.openjdk.java.net/~anazarov/8173372/webrev.01/webrev/ JBS: https://bugs.openjdk.java.net/browse/JDK-8173372 —Andrey

Re: RFR: 8029019: (ann) Optimize annotation handling in core reflection

2017-01-25 Thread Peter Levart
Hi Chris, On 01/24/2017 04:07 PM, Chris Hegarty wrote: Peter, On 2017-01-17 15:10, Peter Levart wrote: Hi, This is a preview of a patch that addresses the following issue: https://bugs.openjdk.java.net/browse/JDK-8029019 This very good work. I have not done a complete review, but what

Re: RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Mandy Chung
> On Jan 25, 2017, at 3:05 AM, Pavel Rappo wrote: > > Hello, > > Could you please review the following change for [1]? > > http://cr.openjdk.java.net/~prappo/8170116/webrev.00/ sun/corba/Bridge.java 99 // Bridge.get() is always called in doPrivileged()

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-01-25 Thread Daniel Fuchs
Hi Rob, I believe you should move the security check to before the class is actually loaded, before the call to 171 List urls = getDnsUrls(url, env); best regards, -- daniel On 25/01/17 17:44, Rob McKenna wrote: I neglected to include a security check so I've cribbed the one

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-01-25 Thread Rob McKenna
I neglected to include a security check so I've cribbed the one from OBJECT_FACTORIES (NamingManager.setObjectFactoryBuilder()) - see: http://cr.openjdk.java.net/~robm/8160768/webrev.02/ Note, this wraps the SecurityException in a NamingException. Presumably its better to throw something than

Re: JDK 9 RFR of JDK-8169903: Refactor spliterator traversing tests into a library

2017-01-25 Thread Paul Sandoz
Hi Any, Much better. I suspect out of laziness when hacking i annotated the test methods with: @SuppressWarnings({"unchecked", "rawtypes”}) Can we remove those with an appropriate adjustment to the test method signatures? SpliteratorTraversingAndSplittingTest — 897 Object[][]

Re: RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-25 Thread Henry Jen
> On Jan 24, 2017, at 12:41 PM, Mandy Chung wrote: > > >> On Jan 24, 2017, at 10:20 AM, Henry Jen wrote: >> >> Hi, >> >> Please review the webrev[1] that add support for JAVA_OPTIONS environment >> variable. The bug[2] describes how

[RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-01-25 Thread Rob McKenna
Hi folks, I'm looking for feedback on this suggested fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-816076 http://cr.openjdk.java.net/~robm/8160768/webrev.01/ This is something that has come up a few times. Basically in certain environments (e.g. MS Active Directory) there

Re: RFR: 8075617, 8075616 Create tests to check wsgen, schemagen work with multi-version jar

2017-01-25 Thread Andrey Nazarov
> On 25 Jan 2017, at 19:00, Alan Bateman wrote: > > On 25/01/2017 15:25, Andrey Nazarov wrote: > >> : >> What is the right location? > These tools are maintained in the upstream Metro project, I suspect the tests > for these tools are located there too but I'm not

Re: RFR: 8167063: spurious message "A JNI error has occurred" if start-class cannot be initialized

2017-01-25 Thread Kumar Srinivasan
116 if (result.isOK()) { 117 throw new Exception("Test Passed Unexpectedly!"); 118 } else if (result.contains("JNI error")) { 119 result.testOutput.forEach(System.err::println); 120 throw new Exception("Test Failed with JNI error!"); 121

Re: RFR: 8075617, 8075616 Create tests to check wsgen, schemagen work with multi-version jar

2017-01-25 Thread Alan Bateman
On 25/01/2017 15:25, Andrey Nazarov wrote: : What is the right location? These tools are maintained in the upstream Metro project, I suspect the tests for these tools are located there too but I'm not sure. There are handful of JAXB and JAX-WS tests in the jdk repo but I don't think they

Re: RFR: 8075617, 8075616 Create tests to check wsgen, schemagen work with multi-version jar

2017-01-25 Thread Andrey Nazarov
> On 25 Jan 2017, at 18:03, Alan Bateman wrote: > > On 25/01/2017 14:45, Andrey Nazarov wrote: > >> Hi, >> >> I’ve added sanity checks that wsgen and schemagen tools works with >> multi-release jar files. >> >> Review

Re: RFR: 8075617, 8075616 Create tests to check wsgen, schemagen work with multi-version jar

2017-01-25 Thread Alan Bateman
On 25/01/2017 14:45, Andrey Nazarov wrote: Hi, I’ve added sanity checks that wsgen and schemagen tools works with multi-release jar files. Review http://cr.openjdk.java.net/~anazarov/8075617/webrev.00/webrev/ JBS: https://bugs.openjdk.java.net/browse/JDK-8075616

RFR: 8075617, 8075616 Create tests to check wsgen, schemagen work with multi-version jar

2017-01-25 Thread Andrey Nazarov
Hi, I’ve added sanity checks that wsgen and schemagen tools works with multi-release jar files. Review http://cr.openjdk.java.net/~anazarov/8075617/webrev.00/webrev/ JBS: https://bugs.openjdk.java.net/browse/JDK-8075616 ,

Re: RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Daniel Fuchs
On 25/01/17 12:32, Alan Bateman wrote: On 25/01/2017 11:05, Pavel Rappo wrote: Hello, Could you please review the following change for [1]? http://cr.openjdk.java.net/~prappo/8170116/webrev.00/ Hi Alan, I agree with Daniel on the name. Also the comment in the Bridge constructor

Re: RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Alan Bateman
On 25/01/2017 11:05, Pavel Rappo wrote: Hello, Could you please review the following change for [1]? http://cr.openjdk.java.net/~prappo/8170116/webrev.00/ I agree with Daniel on the name. Also the comment in the Bridge constructor says "Bridge.get() is always called in doPrivileged()

Re: RFR: 8167063: spurious message "A JNI error has occurred" if start-class cannot be initialized

2017-01-25 Thread Alan Bateman
On 25/01/2017 10:10, Ramanand Patil wrote: Hi Kumar, Thank you for the review and suggestions for the test case. Here is the updated Webrev: http://cr.openjdk.java.net/~rpatil/8167063/webrev.02/ I see Kumar is looking at the test, I'll stick with LauncherHelper. The update to

Re: JDK 9 RFR of JDK-8169903: Refactor spliterator traversing tests into a library

2017-01-25 Thread Amy Lu
Thank you Paul for reviewing. Webrev updated: http://cr.openjdk.java.net/~amlu/8169903/webrev.01/ Note that SpliteratorTestHelper.java now still under test/java/util/stream/bootlib/java.base but changed to "java.util" (instead of java.util.stream). It may be re-located to

Re: RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Daniel Fuchs
Hi Pavel, Looks good to me. I might have kept the 'defaultPresentationManager' name though. Even though it's global it feels like a better name than 'globalPM'. best regards, -- daniel On 25/01/17 11:05, Pavel Rappo wrote: Hello, Could you please review the following change for [1]?

RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Pavel Rappo
Hello, Could you please review the following change for [1]? http://cr.openjdk.java.net/~prappo/8170116/webrev.00/ Thanks, -Pavel [1] https://bugs.openjdk.java.net/browse/JDK-8170116

RE: RFR: 8167063: spurious message "A JNI error has occurred" if start-class cannot be initialized

2017-01-25 Thread Ramanand Patil
Hi Kumar, Thank you for the review and suggestions for the test case. Here is the updated Webrev: http://cr.openjdk.java.net/~rpatil/8167063/webrev.02/ Regards, Ramanand. -Original Message- From: Kumar Srinivasan Sent: Tuesday, January 24, 2017 2:50 AM To: Ramanand Patil

Re: RFR of JDK-8173326: Problem list java/rmi/registry/readTest/CodebaseTest.java on Windows

2017-01-25 Thread David Holmes
Looks good! Thanks, David On 25/01/2017 5:10 PM, Hamlin Li wrote: Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8173326 Thank you -Hamlin diff -r a468135ebe8e