Re: Code review request: 8004235: Disable native JGSS provider on Mac
Looks ok to me. Thanks. -kto On Dec 7, 2012, at 2:39 AM, Weijun Wang wrote: The native JGSS provider on Mac is not ready yet. Disable the native lib creation. Please review the code changes at http://cr.openjdk.java.net/~weijun/8004235/webrev.00/ *build-dev*: I don't know how to write 2 ifneq on a single line. Thanks Max
Re: code review request: 7083664: test hard code of using c:/temp but this dir might not exist
Looks ok to me. -kto On Aug 30, 2011, at 12:34 AM, Weijun Wang wrote: Hi All 7083664: test hard code of using c:/temp but this dir might not exist Webrev is at -- http://cr.openjdk.java.net/~weijun/7083664/webrev.00/ Some of our regression tests set TMP variables on different platforms, and on Windows, it's c:\temp. Unfortunately one of the test machines does not have this directory and a call to zip fails. This fix simply removes all TMP setting lines in security-related tests. Most are in sun/security/tools, with the exception of lib/security/java.policy/Ext_AllPolicy.sh My opinion is that touching TMP is simply a bad idea. A JPRT run also shows it's not needed. I remember there was a time that a certain test harness using by the SQE team stripped all existing environment variables, and caused troubles when a test needed it. Little by little, the harness added some variables and the tests were happy. I don't know if this code change will break it, but I'm optimistic because in most other parts of OpenJDK, shell script tests do not set TMP. Or maybe that test harness (forget the name) is already not in use. Webrev for closed tests will go in another mail. Thanks Max
Re: 2nd round code review request: 7055363: jdk_security3 cleanup
On Aug 4, 2011, at 11:06 AM, Weijun Wang wrote: Max, if we can get an agree, would you please update the Makefile to run JSSE in othervm mode in your fix? So that we don't have to fill a new CR. There are two ways to do this: 1. Add @run main/othervm to all JSSE tests. This means a lot of code changes, but I can automate it. 2. Create new test set for JSSE. This is what I had done before in http://cr.openjdk.java.net/~weijun/7055363/webrev.00/ many changes to Makefile, jprt.properties and jprt.properties in the parent repo. I'd choose #1. What's your opinion? Just a comment from the peanut gallery... ;^) I think #1 is the way to go. One thing we wanted people to be able to do is just cd to the test directory and run 'jtreg -samevm' or 'jtreg -agentvm' and by changing the test itself, this can work. If at some point later someone fixes the test or is able to make it work with samevm, they can remove the main/othervm. If you do it with the Makefile, you only fixed it for when people use the Makefile or with JPRT. -kto BTW, you mentioned HTTPS and JNDI codes. If they are not inside sun/security, I won't touch them this time. Also, at least I know tests inside test/closed/sun/security/ssl/java/security/KeyStore can run in samevm. Can you give a whitelist? Thanks Max On 08/04/2011 12:05 PM, Xuelei Fan wrote: - JSSE tests, Xuelei is working on them It may be a bad news for the purpose to quicken the testing run time. Most of the JSSE test may not be able to under samevm/agentvm mode. 1. JSSE does not support dynamic system properties For performance tuning and synchronization, JSSE does not support dynamic system properties of: java.protocol.handler.pkgs javax.net.ssl.keyStore javax.net.ssl.keyStorePassword javax.net.ssl.trustStore javax.net.ssl.trustStorePassword The JSSE default implementation will use these properties when initializing the static objects, and then any changes to these properties will be ignored in the same VM. There are around 90 of all 170+ JSSE regression tests depends on system properties. 2. JSSE caches TLS sessions, as will result in unexpected reuse of TLS sessions in samevm/agentvm mode. There are a lot of tests using the default SSL/TLS context, which will the same TLS session cache. There are only a very very few tests could be updated to run in samevm mode. Because samevm-safe in JSSE tests will result in unnecessary complex, and the performance improvement is very little, I would suggest tun all JSSE tests at othervm mode. What do you think? Max, if we can get an agree, would you please update the Makefile to run JSSE in othervm mode in your fix? So that we don't have to fill a new CR. Thanks, Xuelei On 8/2/2011 3:02 PM, Weijun Wang wrote: Hi All http://cr.openjdk.java.net/~weijun/7055363/webrev.01/ *Attention*: please note that the jtreg mode for jdk_security3 is still RunOthervmBatch. Xuelei is now working on the JSSE tests. Once he is ready, he will change the mode to RunSamevmBatch. The code changes: * Still included in test/ProblemList.txt - JSSE tests, Xuelei is working on them - ec tests on solaris-i586, they fail even if run standalone * PKCS11/EC tests: - New Providers.setAt, to ensure provider is added to desired position: test/sun/security/pkcs11/ec/* - PKCS11Test.safeReload, to ensure shared library can be loaded again: test/sun/security/pkcs11/PKCS11Test.java test/sun/security/pkcs11/SecmodTest.java - Test using config files need to run in othervm: test/sun/security/pkcs11/Secmod/* test/sun/security/pkcs11/fips/* - Restoring provider lists: test/sun/security/ec/TestEC.java test/sun/security/pkcs11/PKCS11Test.java * Try-with-resources updates: - test/com/sun/security/auth/login/ConfigFile/IllegalURL.java - test/sun/security/pkcs12/PKCS12SameKeyId.java - test/sun/security/tools/keytool/StartDateTest.java * Other tests still need to be in -othervm: Using special policy file: test/sun/security/provider/PolicyFile/Comparator.java AlgorithmId static field oidTable initialization: test/sun/security/x509/AlgorithmId/ExtensibleAlgorithmId.java * JAAS configuration restore: - test/javax/security/auth/login/LoginContext/ResetConfigModule.java * Use localhost as hostname, to make name resolution fast - test/sun/security/jgss/spnego/NoSpnegoAsDefMech.java After these changes, JPRT test run (with RunSamevmBatch and no JSSE tests) shows most tests are OK. There are still intermittent failures which are PKCS11/EC provider related. Thanks Max
Re: code review request for CR 6989705: ECC security code native code compiler warnings
On Jan 21, 2011, at 7:08 AM, Vincent Ryan wrote: On 21/01/2011 14:42, Alan Bateman wrote: Vincent Ryan wrote: Hello Alan, Please review the following webrev which removes 100's of nuisance compiler warnings generated by our ECC code: http://rialto.ireland.sun.com/~vinnie/webrevs/jdk7/6989705/webrev/ Getting rid of the #pragmas and the other changes look okay to me. The only concern is the Makefile changes to hide the remaining warnings. Would there be many changes required to address those so that those warnings can be re-enabled? -Alan The remaining warnings are mostly loss-of-precision warnings - which are safe to inhibit because they are due to wide data types which have been masked down to narrower data types before assignment. I'll take another pass through the code to examine whether any of these warnings can also be eliminated. Normally explicit casts tend to get rid of the narrowing warnings, and makes it obvious in the code that we know it is happening and want it to be happening. It's the 'long' type you have to watch out for, on 64bit Windows it's 32bits as I recall. I'm ok with some special cases warnings that we decide we never want to see because they are like 90% useless or broken, but it makes me nervous to just shut down all occurrences if there might be some value to that warning. I know fixing these warnings is a bit of a pain, but I do think it's worth it in the long run. I appreciate all efforts in this area. Thanks. -kto
Re: review request for 7005608: diamond conversion of JCA and crypto providers
On Dec 23, 2010, at 6:17 PM, Brad Wetmore wrote: On 12/23/2010 12:58 PM, Mandy Chung wrote: On 12/23/10 12:40 PM, Stuart Marks wrote: On 12/22/10 6:17 PM, Brad Wetmore wrote: You need to update the Copyright updates on these files to include 2010. Is this the standard, to update the copyright to the current year anytime the file is touched? If so, then yes, I can do this. This is optional. The RE will do the copyright year update periodically. Kelly has fixed the script for RE to do that. Is this recent? I was told back in March to continue to update the years until rebranding was over, the script was fixed and RE started using it. We're obviously past rebranding, and there was an announcement in June that the script update_copyright_year was updated, but the question was now what to do with it. I never saw the copyright dates updates are now optional annoucements, and don't see anything in my archives/trash. Kelly or RE? If people update it themselves, manually, then they avoid an additional changeset modifying their files. So it's still ok for people to manually update the second year. But it can be considered optional again. It's a tricky thing to fully automate. Ideally, someone needs to eyeball the changes. I'll try and run the script over openjdk7 next week, before 2011. What about for OpenJDK 6 also? Just did it yesterday. All but hotspot. And SunJDK 6/5/1.4.2? (Sorry, OpenJDK folks, but had to ask! :) ) Different problem, leaving it to the sustaining team. Frankly, I'll be glad when it is automated again. Doing/reminding has been a hassle. Yeah. It's not quite automated yet, I'll work with RE on some kind of monthly schedule, maybe do it the last day of the month. -kto Brad
