Sorry, this one is the right webrev:
http://cr.openjdk.java.net/~clanger/webrevs/8150470.1/
<http://cr.openjdk.java.net/%7Eclanger/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/
<http://cr.openjdk.java.net/%7Eclanger/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
<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
inhttp://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
