EXIficient / exificient-grammars

Java Implementation of EXI (grammars part)
http://exificient.github.io/java/
MIT License
4 stars 9 forks source link

exificient-grammars pulls in xercesImpl #11

Open rovarga opened 5 years ago

rovarga commented 5 years ago

exificient-grammars is pulling in xercesImpl, which is a full JAXP implementation, where it only needs a few implementation packages (org.apache.xerces.impl.xs, org.apache.xerces.xs, org.apache.xerces.xni). This has the downside of requiring users to deal with multiple implementations being present on the classpath and also increasing footprint significantly.

It would be much better if exificient had its own jar containing relocated parts of xercesImpl and relied on that rather than coupling it with complete xercesImpl. That way those classes become an implementation detail without interfering with other classpath tenants. One possibility would be to use https://maven.apache.org/plugins/maven-shade-plugin/examples/class-relocation.html to directly embed those pieces inside exificient-grammars.

danielpeintner commented 5 years ago

Let me check more closely the implications...

Is the main issue for this request

rovarga commented 5 years ago

The main issue is the surprise presence of another JAXP implementation being present in OSGi environment -- where class resolution works differently from usual the single-classloader world. Getting xercesImpl.jar working as a DocumentBuilderFactory there requires shading anyway, because it uses xml-apis-1.4.01, which causes org.w3c.dom package being split between JVM-provided version and xml-apis. xercesImpl bundle must pick up the xml-apis version, because in Java <=8 org.w3c.dom.ElementTraversal does not exist, leading to a NoClassDefFoundError when xercesImpl is used.

This is reported downstream in https://jira.opendaylight.org/browse/CONTROLLER-1867 and while we will fix it downstream, it would be nice if we could just use exificient-grammars without this surprise.

danielpeintner commented 5 years ago

FYI: with this change the GrammarFactory would change the public API and might break existing implementations given that it expects XMLEntityResolver class of package org.apache.xerces.xni.parser.

Having said that, I think this might be the only issue.

danielpeintner commented 5 years ago

I played a bit around with the Apache Shade Plugin and created a dedicated branch (see https://github.com/EXIficient/exificient-grammars/tree/xerces-shade).

It kind of works for mvn package but does not work for mvn assembly:single for example.

Moreover, I think the idea is to not just relocating packages but also eliminating code that is not needed, correct? I fail doing so. @rovarga do you have used this plugin before? Feel free to provide input on the newly created branch.

When it comes to XMLEntityResolver, which is used in the exificient-grammars interface I would say we need to create a own resolver so that externally it looks like exificient-grammars does not depend on xerces packages at all, right?

danielpeintner commented 5 years ago

Note: Yet another solution could be to do what Sun did by creating its own packages com.sun.org.apache.xerces.internal (essentially copying the needed packages)...

My feeling is that using the shade plugin may cause other issues.

danielpeintner commented 5 years ago

@rovarga what do you think about the latest proposal?