Skip to content

Remove dependency on xml-apis-xerces on the printing extension #7809

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 23, 2024
Merged

Remove dependency on xml-apis-xerces on the printing extension #7809

merged 1 commit into from
Dec 23, 2024

Conversation

groldan
Copy link
Member

@groldan groldan commented Aug 6, 2024 •
edited by pull-request-badge bot
Loading

xml-apis:xml-apis-xerces:2.7.1 is the same as
xml-apis:xml-apis but from 2012 and unnecessary since Java 1.6.

Checklist

For core and extension modules:

  • New unit tests have been added covering the changes.
  • Documentation has been updated (if change is visible to end users).
  • The REST API docs have been updated (when changing configuration objects or the REST controllers).
  • There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
  • Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
  • Bug fixes and small new features are presented as a single commit.
  • Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

@groldan groldan added dependencies Pull requests that update a dependency file backport 2.25.x Instructs the bot to create a 2.25.x backport PR on merge labels Aug 6, 2024
@aaime
Copy link
Member

aaime commented Aug 8, 2024

@jodygarnett I'd feel better if you could have a look at this one.

@jodygarnett
Copy link
Member

Understood

@jodygarnett
Copy link
Member

@groldan how does the interact with mapfish-print-v2 release?

@groldan
Remove dependency on xml-apis-xerces on the printing extension
83e1780 
`xml-apis:xml-apis-xerces:2.7.1` is the same as
`xml-apis:xml-apis` but from 2012 and unnecessary since Java 1.6.
@groldan groldan force-pushed the printing_extension_remove_xml-apis_dependency branch from 0ccf37e to 83e1780 Compare August 11, 2024 15:35
@groldan
Copy link
Member Author

groldan commented Aug 11, 2024

@jodygarnett your question made me realize I didn't remove the <include>xml-apis-xerces-*.jar</include> include from src/release/ext-printing.xml. Just did.

That said, I'm not sure I understand your question.
The xml-apis dependency is not necessary since Java 1.6. The xml-apis-xerces dependency contains exactly the same classes as xml-apis:xml-apis. xml-apis-xerces is just released a year before xml-apis:xml-apis, and they don't even include the StAX apis.
So they're just ancient and all their classes have been included in the standard JRE system library for a long time (javax.xml, org.s3c.dom, and org.xml.sax namespaces).

@jodygarnett
Copy link
Member

jodygarnett commented Aug 11, 2024 •
edited
Loading

Thanks I was trying to ask if any similar change is useful for mapfish-print-v2 (since as you say xerces dependency not useful). Is any dependency changes needed there, or do any dependency excludes need to be added …

@aaime
Copy link
Member

aaime commented Oct 7, 2024

@groldan @jodygarnett I don't understand if this should be closed and moved upstream to mapfish-print-v2, or merged here...

@jodygarnett
Copy link
Member

@groldan can I remove xerces dependency from mapfish print?

@groldan
Copy link
Member Author

groldan commented Oct 11, 2024 •
edited
Loading

@jodygarnett you definitely can remove the xml-apis-xerces dependency from mapfishprint, I'm guessing it's at least on Java 8, and the apis are part of the standard lib since 1.6.

Since this PR is about removing the dependency explicitly declared in the printing extension's pom, this pr should be merged regardless.

@groldan groldan added the backport 2.26.x Instructs the bot to create a 2.26.x backport PR on merge label Nov 4, 2024
@aaime
Copy link
Member

aaime commented Dec 2, 2024

Let's go then

@aaime aaime merged commit 33510b7 into geoserver:main Dec 23, 2024
10 checks passed
This was referenced Dec 23, 2024
Merged
Merged
@groldan groldan deleted the printing_extension_remove_xml-apis_dependency branch January 9, 2025 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mprins mprins mprins left review comments

Assignees
No one assigned
Labels
backport 2.25.x Instructs the bot to create a 2.25.x backport PR on merge backport 2.26.x Instructs the bot to create a 2.26.x backport PR on merge dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@groldan @aaime @jodygarnett @mprins