-
-
Notifications
You must be signed in to change notification settings - Fork 905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix DOM2DTM to work in J9 #1760
Conversation
J9 wasn't coping with hot-patching standard library packages with a user class. This was done to work around a package private constructor. This is fixed by moving into a local package and copying the class with the package private constructor into a local copy.
would prefer keeping the class in
not sure, with JRuby we generally avoid copy-ing code from OracleJDK which is essentially OpenJDK although licensing terms does change with different release. |
@Taywee why did you copy |
@jvshahid Ignorance, probably. I saw DOM2DTMExt being almost exactly identical to the version in the OpenJDK tree and assumed that's where it had come from. My knowledge of typical licensing issues in the Java world is relatively limited (I don't work in Java much), mostly just used this as a patch to get it running in J9 on AIX. If these are issues, I'd be happy to rectify them and roll those changes into this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change looks ok. It doesn't add anything new that wasn't there already. We had an Apache Licensed file from Xalan that was introduced in #1604.
@flavorjones I think LICENSE-DEPENDENCIES.md
cover Xalan, I don't think we need to do anything.
@kares I'm not sure how we ended down this path in the first place. I saw your comment on #1604 that Xalan isn't maintained anymore, but it looks to me like XalanJ is still maintained, or at least the mailing list & JIRA are still active (see http://mail-archives.apache.org/mod_mbox/xalan-dev/201805.mbox/browser). Is it possible to move your fixes from last year to the upstream Xalan repo so we can get rid of those patches ?
yeah sorry about that, I personally have a lot on my OSS plate to spend more "free" time on this, atm. |
Did a bit of research and I think @kares is right. Xalan-J development has stalled according to this thread. It looks like there is no one around to maintain it and release it anymore. I'm ok with merging this PR as is. In the thread I linked to above, there is a mention of another library Saxon which has more advanced features and is better maintained. I think we should try it out. |
smt of a more recent XSLT impl to try would be awesome esp. as things are getting 'hacky' with xalan |
OK, I'm going to rebase this branch and, assuming it goes green, will merge it. @jvshahid do you want me to create a new Issue to track exploration of using Saxon? |
Yes please
On Dec 3, 2018 17:15, "Mike Dalessio" <[email protected]> wrote:
OK, I'm going to rebase this branch and, assuming it goes green, will merge
it.
@jvshahid <https://github.com/jvshahid> do you want me to create a new
Issue to track exploration of using Saxon?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1760 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASKlS8ZKyX08bfjlF7VN7brHtDxz5PSks5u1aJsgaJpZM4T-tIO>
.
|
Created #1829 for exploring a replacement for Xalan-J. |
I've rebased and attempted to resolve a logical conflict at #1830. If that goes green, I'll merge and we'll ship it in the next release. |
J9 wasn't coping with hot-patching standard library packages with a user class. This was done to work around a package private constructor. This is fixed by moving into a local package and copying the class with the package private constructor into a local copy.
Note that DOM2DTMdefaultNamespaceDeclarationNode in copied almost entirely ver-batim out of the OpenJDK source tree. The only changes are the package and the import of the DTMException, which now uses the external version instead of the internal one.
closes #1759