Sorry, this one is the right webrev: http://cr.openjdk.java.net/~clanger/webrevs/8150470.1/
From: Langer, Christoph Sent: Dienstag, 23. Februar 2016 21:13 To: 'Aleksej Efimov' <aleksej.efi...@oracle.com>; huizhe wang <huizhe.w...@oracle.com> Subject: RE: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE Hi Aleksej, Joe, sorry, I already had a bad feeling with that line... Here is a bug: https://bugs.openjdk.java.net/browse/JDK-8150470 And here is a webrev for fix: http://cr.openjdk.java.net/~clanger/webrevs/8150470.0/ Feel free to add more data to the bug and do more tests. I only had the jtreg tests at hand. Shall I post this in the mailing list? Best regards Christoph From: Aleksej Efimov [mailto:aleksej.efi...@oracle.com] Sent: Dienstag, 23. Februar 2016 18:26 To: huizhe wang <huizhe.w...@oracle.com<mailto:huizhe.w...@oracle.com>> Cc: Langer, Christoph <christoph.lan...@sap.com<mailto:christoph.lan...@sap.com>> Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE Sure! Will do it tomorrow. Best Regards, Aleksej On 02/23/2016 07:50 PM, huizhe wang wrote: Hi Aleksej, Thanks for catching the issue and identifying the fix! Once you come back from the holiday, could you submit a review of the fix? Best, Joe On 2/23/2016 8:15 AM, Aleksej Efimov wrote: Hi again, Got an update on this issue: The fix introduced back this one issue: https://bugs.openjdk.java.net/browse/JDK-8021366 The root cause is that Entity.DEFAULT_XMLDECL_BUFFER_SIZE and XMLEntityManager.DEFAULT_XMLDECL_BUFFER_SIZE have different values: 32 and 64. This change erased the JDK-8021366 change (64->32) and introduced back the issue: - len = fCurrentEntity.DEFAULT_XMLDECL_BUFFER_SIZE; + len = DEFAULT_XMLDECL_BUFFER_SIZE; Tested the reversion of the fix for only this line and JCK test passes now: - len = DEFAULT_XMLDECL_BUFFER_SIZE; + len = fCurrentEntity.DEFAULT_XMLDECL_BUFFER_SIZE; With Best Regards, Aleksej On 02/23/2016 06:46 PM, Aleksej Efimov wrote: Hi Christoph, Joe, I think we may have a problem with this fix. I'm observing one new JCK test failure with the fix. If the fix is stripped from JDK9 repo - no failure observed. Failed test is api/xsl/conf/copy/copy19.html#copy19 The failure message is: MultiJVM group agent ID: 1 java version "9-internal" OpenJDK Runtime Environment (build 9-internal+0-2016-02-23-012828.aefimov.9) OpenJDK 64-Bit Server VM (build 9-internal+0-2016-02-23-012828.aefimov.9, mixed mode) Failed to parse as XML. Cause: warning;org.xml.sax.SAXParseException; lineNumber: 2; columnNumber: 10; XML document structures must start and end within the same entity. Parsing as text. Failed to parse as XML. Cause: warning;org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 53; XML document structures must start and end within the same entity. Parsing as text. mismatch-value;TEXT()len=43;TEXT()len=62;lengths do not match mismatch-value-gold;TEXT(); <?xml version="1.0" encoding="ISO-8859-1"?> mismatch-value-text;TEXT(); <?xml version="1.0" encoding="ISO-8859-1"?><out>abcd\u00e8fgh</out> XHTFileCheckService results were not equal. Expected result: <?xml version="1.0" encoding="ISO-8859-1"?> <out>abcd\u00e8fgh</out> Actual result: <?xml version="1.0" encoding="ISO-8859-1"?><out>abcd\u00e8fgh</out> result: Failed. test cases: 1; all failed; first test case failure: testTransformation test result: Failed. test cases: 1; all failed; first test case failure: testTransformation Today is a holiday in SPb. Tomorrow I can log a bug to track this failure (new line is missing in actual result output). Best Regards, Aleksej On 02/23/2016 02:27 PM, Langer, Christoph wrote: Hi Aleksej, that's great, please go ahead. Sooner or later I would have started this effort, too. But right now I'm working on another fix for the XALAN which I'll post soon :-) Best regards Christoph PS: @Joe: I noticed that in the commit I was not the author of the change but only mentioned in the "Contributed-by" tag. Couldn't you push the change on my behalf but with maintaining me as the author? -----Original Message----- From: Aleksej Efimov [mailto:aleksej.efi...@oracle.com] Sent: Dienstag, 23. Februar 2016 12:21 To: Langer, Christoph <christoph.lan...@sap.com><mailto:christoph.lan...@sap.com> Cc: Joe Wang <huizhe.w...@oracle.com><mailto:huizhe.w...@oracle.com> Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE Hi Christoph, I would like to backport your fix to JDK8. Just wanted to get your confirmation that you're not working on that and we are not doing the same task =) With Best Regards, Aleksej On 02/18/2016 10:33 PM, Langer, Christoph wrote: Hi Joe, I fixed the copyright in http://cr.openjdk.java.net/~clanger/webrevs/8149915.3/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8149915.3/>. So I think I'm done. Can you review and push the change then? Thanks Christoph -----Original Message----- From: huizhe wang [mailto:huizhe.w...@oracle.com] Sent: Donnerstag, 18. Februar 2016 19:19 To: Langer, Christoph <christoph.lan...@sap.com><mailto:christoph.lan...@sap.com> Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE On 2/18/2016 5:15 AM, Langer, Christoph wrote: Hi Joe, here is the next version: http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8149915.2/> Looks good! Thanks. Copyright year in XSAttributeChecker.java is still incorrect. I meant to say the year "2016" needs to have a comma, so in this case, it needs to be "2015, 2016, " rather than "2015, 2016". Best, Joe Hopefully the final one :-) Yes, it's good to have some cleanups. For the generic initialization, it's preferable to have no redundant type arguments, for example, instead of: protected Stack<Entity> fEntityStack = new Stack<Entity>(); do: protected Stack<Entity> fEntityStack = new Stack<>(); Some of the "cleanup" in the webrev were reversing <> with added type argument(s), that would be unnecessary. I think I did that on all the places that I touched now. For the obsolete Vector, it would be good to replace it with ArrayList. There are so many Vector instances in this code that it's out of scope for me now to touch them all... Copyright year "2016" in XSAttributeChecker needs to be "2016," or else the script would complain. For that one I thought that it would look like "2015,2016" if the initial copyright was done in 2015. But I changed it as you suggested. In XSDHandler, note that there's another call to get SECURITY_MANAGER towards the end of the reset method (at 3572 in your new version) that may be removed. You may also consolidate most of those setFeature/Property statements in one try-catch block if you want. OK, I did some further refacturing here. However, I've left the try/catch blocks as I think it's the intention to silently catch some exceptions but then try to go on with the next property. For the test, the @bug tag is still desirable in the general comment section. For now, it's @bug 8149915. As we add more test to the class in the future, we'd then append more bug ids, @bug 8149915, xxxxxxx. Done. I also left the @bug in the comments for the test method. I'm also wondering why some files have a copyright header like this, e.g. src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11D TDScannerImpl.java: /* * reserved comment block * DO NOT REMOVE OR ALTER! */ Shouldn't there be the standard copyright header or is there some reason behind it? The block is used by the licensing script to swap with license information required by licensees. For code that came from Xerces, we should keep the block as is with the Apache License. But when we make changes, we're required to put in the copyright header. Since the changes you made are cleanup / removing unused fields, it's okay to keep the block as is. OK :) Thanks again for reviewing. All the Best Christoph
