Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-11 Thread Valerie Peng
Incremental changes look fine. Thanks, Valerie On 5/11/2020 2:27 PM, Mikael Vidstedt wrote: New webrev fixing Basic.policy based on Valeries’s comment: webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.03/security/open/webrev/ incremental:

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-11 Thread Mikael Vidstedt
New webrev fixing Basic.policy based on Valeries’s comment: webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.03/security/open/webrev/ incremental: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.03/security.incr/open/webrev/ Cheers, Mikael > On May 7, 2020, at

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-08 Thread Valerie Peng
Noticed one minor thing: Additional lines can be removed, i.e. other non-SunPKCS11-nss ones can probably removed, i.e. line 15, 16, and 19. Thanks, Valerie On 5/6/2020 10:05 PM, Mikael Vidstedt wrote: New webrev here: webrev:

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Mikael Vidstedt
New webrev: webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.02/security/open/webrev/ incremental: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.02/security.incr/open/webrev/ Items to be resolved: * File follow-up to drop $ISA support in

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Mikael Vidstedt
> On May 7, 2020, at 4:08 PM, Valerie Peng wrote: > > It should be ok to remove the if (secret == null) block of > test/jdk/sun/security/pkcs11/tls/TestPRF.java. > > I verified that the test runs fine on Linux without that block, so the > comment is legit. Thank you for verifying! I’ll

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Valerie Peng
It should be ok to remove the if (secret == null) block of test/jdk/sun/security/pkcs11/tls/TestPRF.java. I verified that the test runs fine on Linux without that block, so the comment is legit. Thanks, Valerie On 5/5/2020 4:17 PM, Mikael Vidstedt wrote: John, Thanks for the review!

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Valerie Peng
One more thing: test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java: Not worth keeping the tests in this directory. We do need the tests under test/jdk/sun/security/pkcs11/Config/, please add them back if you have already removed them. As stated earlier, the config parsing

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Mikael Vidstedt
Will generate a new webrev shortly, meanwhile some comments inline.. > On May 7, 2020, at 1:02 AM, Weijun Wang wrote: > > > >> On May 7, 2020, at 1:05 PM, Mikael Vidstedt >> wrote: >> >> >> New webrev here: >> >> webrev: >>

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Valerie Peng
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java: Maybe we should not support $ISA at all? I suppose it should be ok not supporting $ISA. Existing PKCS11 provider documentation does not mention it anyway. test/jdk/sun/security/pkcs11/Provider/ConfigShortPath.java:

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Weijun Wang
> >> >> * Get confirmation if https://bugs.openjdk.java.net/browse/JDK-8039280 is >> enough to cover >> test/jdk/sun/security/provider/PolicyParser/PrincipalExpansionError.java > > The test has been neglected for a long time, but I think in theory the > SolarisPrincipal can be substituted

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-07 Thread Weijun Wang
> On May 7, 2020, at 1:05 PM, Mikael Vidstedt > wrote: > > > New webrev here: > > webrev: > http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/security/open/webrev/ > incremental: > http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/security.incr/open/webrev/ > >

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-06 Thread Mikael Vidstedt
New webrev here: webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/security/open/webrev/ incremental: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/security.incr/open/webrev/ Items yet to be resolved: * File follow-up to drop $ISA support in

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-06 Thread Mikael Vidstedt
> On May 5, 2020, at 7:29 PM, Weijun Wang wrote: > > > >> On May 6, 2020, at 6:51 AM, Mikael Vidstedt >> wrote: >> >> >> Max, >> >> Thank you so much for the thorough review! I’m working on an incremental >> webrev but could use some help - comments/questions inline.. >> >>> On May

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-05 Thread Weijun Wang
> On May 6, 2020, at 6:51 AM, Mikael Vidstedt > wrote: > > > Max, > > Thank you so much for the thorough review! I’m working on an incremental > webrev but could use some help - comments/questions inline.. > >> On May 4, 2020, at 6:58 AM, Weijun Wang wrote: >> >> I've gone through all

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-05 Thread Mikael Vidstedt
John, Thanks for the review! Comments/questions inline.. > On May 4, 2020, at 4:02 PM, sha.ji...@oracle.com wrote: > > Hi, > Generally, this patch doesn't take care of the solaris/sunos/ucrypto-related > words in test summaries, code comments, configs and READMEs. > E.g. >

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-05 Thread Mikael Vidstedt
Max, Thank you so much for the thorough review! I’m working on an incremental webrev but could use some help - comments/questions inline.. > On May 4, 2020, at 6:58 AM, Weijun Wang wrote: > > I've gone through all files. Many of them are PKCS11-related, it will be nice > if Valerie can

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-04 Thread sha . jiang
Hi, Generally, this patch doesn't take care of the solaris/sunos/ucrypto-related words in test summaries, code comments, configs and READMEs. E.g. test/jdk/javax/net/ssl/templates/SSLEngineTemplate.java test/jdk/sun/security/provider/MessageDigest/TestSHAClone.java

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-04 Thread Weijun Wang
I've gone through all files. Many of them are PKCS11-related, it will be nice if Valerie can confirm my findings. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java: Maybe we should not support $ISA at all?

RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)

2020-05-03 Thread Mikael Vidstedt
Please review this change which implements part of JEP 381: JBS: https://bugs.openjdk.java.net/browse/JDK-8244224 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/security/open/webrev/ JEP: https://bugs.openjdk.java.net/browse/JDK-8241787 Note: When reviewing this, please