Skip to content
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

[WFLY-18644] Removing xerces from distribution #17295

Merged
merged 1 commit into from Oct 20, 2023

Conversation

ropalka
Copy link
Contributor

@ropalka ropalka commented Oct 13, 2023

@github-actions github-actions bot added the deps-changed Dependencies have been checked, and there are changes highlighted in a comment label Oct 13, 2023
@github-actions
Copy link

Dependency Tree Analyzer Output:

Removed Dependencies:

  • xerces:xercesImpl:jar:2.12.0.SP05:compile

CC @wildfly/prod

@jbliznak
Copy link
Contributor

So we have this covered

ee-feature-pack/galleon-shared/pom.xml:            <groupId>xerces</groupId>
ee-feature-pack/galleon-shared/pom.xml:            <artifactId>xercesImpl</artifactId>
ee-feature-pack/galleon-shared/src/main/resources/modules/system/layers/base/org/apache/xerces/main/module.xml:<module xmlns="urn:jboss:module:1.9" name="org.apache.xerces">
ee-feature-pack/galleon-shared/src/main/resources/modules/system/layers/base/org/apache/xerces/main/module.xml:        <artifact name="${xerces:xercesImpl}"/>
ee-feature-pack/galleon-shared/src/main/resources/license/licenses.xml:            <groupId>xerces</groupId>
ee-feature-pack/galleon-shared/src/main/resources/license/licenses.xml:            <artifactId>xercesImpl</artifactId>

but there are more places using xerces that IMHO should be also removed. If not, why?

boms/common-ee/pom.xml:                <groupId>xerces</groupId>
boms/common-ee/pom.xml:                <artifactId>xercesImpl</artifactId>
boms/common-ee/pom.xml:                <version>${version.org.apache.xerces}</version>
pom.xml:        <version.org.apache.xerces>2.12.0.SP05</version.org.apache.xerces>
webservices/tests-integration/pom.xml:            <groupId>xerces</groupId>
webservices/tests-integration/pom.xml:            <artifactId>xercesImpl</artifactId>
testsuite/integration/basic/pom.xml:            <groupId>xerces</groupId>
testsuite/integration/basic/pom.xml:            <artifactId>xercesImpl</artifactId>

@ropalka
Copy link
Contributor Author

ropalka commented Oct 16, 2023

There are some webservices tests that are using xerces directly @jbliznak . @jimma is there any plan to remove these xerces specific tests from WildFly code base?

@jimma
Copy link
Contributor

jimma commented Oct 16, 2023

@ropalka @jbliznak Except the tests are created to test the xerces jar file packaged in the deployment, the xerces dependency can all be removed.

@ropalka
Copy link
Contributor Author

ropalka commented Oct 17, 2023

PR updated as suggested by @jimma @jbliznak

Copy link
Contributor

@jbliznak jbliznak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of desired functionality, this PR is now working as expected.

I will leave for WF guardians if something more is required to tide this up, eg. extract xerces version to property or even manage xerces for tests elsewhere in some test-only BOM (wildfly-standard-test-bom?)

@asoldano asoldano merged commit 20a2b12 into wildfly:main Oct 20, 2023
18 checks passed
@ropalka ropalka deleted the WFLY-18644 branch October 27, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-changed Dependencies have been checked, and there are changes highlighted in a comment
Projects
None yet
4 participants