Repository navigation
[MSHARED-1274] Deprecate the xml bits from maven-shared-utils #331
Description
Activity
Guillaume Nodet commented
Elliotte Rusty Harold sjaranowski thoughts on that one ?
Elliotte Rusty Harold commented
I'm not sure who if anyone uses this. I've deprecated some of it already. Ideally I'd like to get rid of all of this AND all of plexus-xml. It's a mess that has never properly implemented the XML specs, and building Maven around what plexus-xml thinks XML is makes it very challenging to use tools that do properly implement XML like XSLT to do anything with poms.
To make matters worse parts of the plexus version at least are exposed in the public APIs of many plugins so we can't just swap in a better parser.
I'm not sure about half measures that only address the maven-shared-utils half of the problem. But if indeed no one is using it, then it wouldn't hurt to deprecate it. Do we have a quick way to see across all the various projects who is importing this?
If we do want to keep plexus-xml can we bring it under official Apache governance without changing the package?
Elliotte Rusty Harold commented
searching github I only see one use of org.apache.maven.shared.utils.xml.pull.XmlPullParserException from outside the maven-shared-utils repo and its forks by Maven team members:
https://github.com/search?q=org.apache.maven.shared.utils.xml.pull.XmlPullParserException&type=code
We can check other classes but since this exception is pretty fundamental to any use of the other classes in this package, it's likely safe to go ahead and deprecate the entirety of org.apache.maven.shared.utils.xml
Guillaume Nodet commented
Fwiw, I don't see any real problem with the plexus-xml bits. The parser is very fast and lightweight and I would be opposed to switching to a different parser at this point. And the Xpp3Dom is now empty (because the real implementation is now in maven).
I don't really see the problem with the "governance" either. Not all projects have to be inside the ASF to be able to be consumed by maven.
Elliotte Rusty Harold commented
"fast and lightweight" is often a euphemism for, "Does not correctly implement the XML spec"
The problem with governance is not that it's not inside the ASF. It's that ownership, licensing, and development of plexus is very murky. None of that matters until some company buys some other company and the new lawyers go looking for a way to extract rents from existing IP. Even if you win, responding to the lawsuits can easily cost millions. Yes, this has happened multiple times in the past and it will happen in the future.
Elliotte Rusty Harold commented
org.apache.maven.shared.utils.xml.PrettyPrintXmlWriter does seem to be used here and there:
https://github.com/search?q=org.apache.maven.shared.utils.xml.PrettyPrintXmlWriter&type=code
However, it's likely to be separable from the rest of this package.
Elliotte Rusty Harold commented
Hmm, not sure about plexus-xml but these classes do not, contrary to what I thought, use XPP3. They're based on the JDK's SAX parser. However they don't configure it properly for namespaces, and they're still not used anywhere so probably OK to delete.
Guillaume Nodet commented
I missed that they had been rewritten too.
Anyway, I'd like to challenge a bit the assertionIt's a mess that has never properly implemented the XML specs, and building Maven around what plexus-xml thinks XML is makes it very challenging to use tools that do properly implement XML like XSLT to do anything with poms.
I think the problem is more in the generated modello readers which do not handle namespaces, rather than the xpp3 parser which can not read them. Changing the underneath parser won't have any effect unless we change the way the xml is actually processed. Anyway, I'm going to experiment a bit...
Guillaume Nodet commented
Here's my experiments: https://github.com/gnodet/maven/tree/xml-experiments
The results of the performance tests are the following:
Benchmark Mode Cnt Score Error Units Xpp3DomPerfTest.readWithStaxAlto avgt 5 180.467 ± 1.921 ms/op Xpp3DomPerfTest.readWithStaxJdk avgt 5 308.447 ± 7.208 ms/op Xpp3DomPerfTest.readWithStaxXpp3 avgt 5 219.544 ± 3.496 ms/op Xpp3DomPerfTest.readWithXpp3 avgt 5 197.684 ± 3.535 ms/opSo I'm willing to investigate switching to the Stax API and Aalto XML parser. The work is already half done with the above branch anyway...
Elliotte Rusty Harold commented
I haven't noticed aalto before. I'd have to look at it very carefully. I have yet to encounter an "ultra-high performance" parser that correctly implemented XML. Usually the so-called performance wins are achieved only by chopping out the parts of XML the parser author doesn't like. Maybe aalto is the first. I don't know.
Guillaume Nodet opened MSHARED-1274 and commented
The xml bits from plexus-utils are a de-facto part of the maven 3.x api and we should not have two conflicting versions of it. Now that it has been extract in a separate project in plexus-xml, I think it is time to deprecate those classes in maven-shared-utils. Fwiw, the core implementation classes are mainly implemented with Maven 4.x maven-xml-impl module, and the Xpp3Dom class from plexus-xml is mainly a wrapper around the new XmlNode/XmlNodeImpl class from maven, however the parser is still present in plexus-xml.
Issue Links:
Remote Links: