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-15542] Connector subsystem Jakarta migration #15019

Merged
merged 2 commits into from Feb 21, 2022

Conversation

tadamski
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Dec 10, 2021
ee-9/pom.xml Outdated
Comment on lines 420 to 430
<dependency>
<groupId>jakarta.security.auth.message</groupId>
<artifactId>jakarta.security.auth.message-api</artifactId>
<version>${version.jakarta.security.auth.message-api}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

This dependency should be jakarta.authentication:jakarta.authentication-api:2.0.0 instead, which is already available in the pom, so I would not expect to introduce this variant (2.0.0-RC1) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed; I found out that this can be removed

Maintain separation between the artifact id and the version to help prevent
merge conflicts between commits changing the GA and those changing the V.
-->
<version>26.0.0.Final-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will need updating to the current version in use on main.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@darranl
Copy link
Contributor

darranl commented Jan 7, 2022

@tadamski I think this one still needs some updates before we test again.

@tadamski
Copy link
Contributor Author

tadamski commented Jan 7, 2022

@darranl I plan to fix and rebase this PR after transactions migration is merged.

@darranl darranl added the EE9 label Jan 12, 2022
@bstansberry
Copy link
Contributor

@tadamski The transactions subsystem one is merged now.

@tadamski
Copy link
Contributor Author

tadamski commented Feb 6, 2022

@bstansberry I have rebased and fixed the PR; should work fine now

ee-9/pom.xml Outdated
Comment on lines 774 to 869
<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-common-api-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-common-impl-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-common-spi-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-core-api-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-core-impl-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<!--FIXME removing this results in NPE during compilation but I can't see any particular dependency blocked by the exclusion-->
<!-- <exclusions>-->
<!-- <exclusion>-->
<!-- <groupId>*</groupId>-->
<!-- <artifactId>*</artifactId>-->
<!-- </exclusion>-->
<!-- </exclusions>-->
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-deployers-common-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-jdbc-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-validator-jakarta</artifactId>
<version>${version.org.jboss.ironjacamar}</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

For any dependency added here that is in the Jakarta name space, you have to exclude the non-jakarta one on the ee-9/feature-pack module from the Galleon feature pack that is pulling it. Besides avoiding the pollution of the classpath, that will prevent getting it transformed unnecessarily. An example:

<exclusion>
<groupId>org.jboss.narayana.jts</groupId>
<artifactId>narayana-jts-integration</artifactId>
</exclusion>

Then you need to enable the JBoss Modules module for such a dependency to use the -jarkarta suffix, for example:

<artifact name="\${org.jboss.narayana.jts:narayana-jts-integration@module.jakarta.suffix@}"/>

Then you probably will need to add the license file for the -jakarta one, for example:

<dependency>
<groupId>org.jboss.narayana.jts</groupId>
<artifactId>narayana-jts-integration-jakarta</artifactId>
<licenses>
<license>
<name>GNU Lesser General Public License v2.1 only</name>
<url>http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html</url>
<distribution>repo</distribution>
</license>
</licenses>
</dependency>
<dependency>

And finally, in order to get it processed correctly by Galleon, you will need to add the -jakarta one to the ee-9/common module, for example:

wildfly/ee-9/common/pom.xml

Lines 380 to 389 in 5043f7d

<dependency>
<groupId>org.jboss.narayana.jts</groupId>
<artifactId>narayana-jts-integration-jakarta</artifactId>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

ee-9/pom.xml Outdated
Comment on lines 826 to 832
<!--FIXME removing this results in NPE during compilation but I can't see any particular dependency blocked by the exclusion-->
<!-- <exclusions>-->
<!-- <exclusion>-->
<!-- <groupId>*</groupId>-->
<!-- <artifactId>*</artifactId>-->
<!-- </exclusion>-->
<!-- </exclusions>-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Following the same pattern, we have to exclude all the transitive dependencies. After doing it, you have to check your wildfly-connector-jakarta and add there all the dependencies needed to get your module compiled.

You are getting NPE during the compilation because you are missing dependencies brought in transitively by this module. You will need to review them and add them to your wildfly-connector-jakarta module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

ee-9/pom.xml Outdated
Comment on lines 1384 to 1377
<!-- <exclusions>-->
<!-- <exclusion>-->
<!-- <groupId>*</groupId>-->
<!-- <artifactId>*</artifactId>-->
<!-- </exclusion>-->
<!-- </exclusions>-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Following the same pattern, we have to exclude all the transitive dependencies. After doing it, you have to check your wildfly-connector-jakarta and add there all the dependencies needed to get your module compiled.

You are getting NPE during the compilation because you are missing dependencies brought in transitively by this module. You will need to review them and add them to your wildfly-connector-jakarta module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

<relativePath>../pom.xml</relativePath>
</parent>

<artifactId>wildfly-connector-jakarta</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition to the other -jakarta dependencies added to ee-9/pom.xml, you will need to exclude the old one (wildfly-connector) from the Galleon Pack that is pulling it on the ee-9/feature-pack module, modify its JBoss Module module to enable the -jarkarta suffix, and review the license file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@tadamski tadamski force-pushed the WFLY-15542 branch 3 times, most recently from ce54185 to 8cd9981 Compare February 15, 2022 13:22
@tadamski
Copy link
Contributor Author

@yersan thanks for all the feedback - I have fixed the problems that you mentioned above

@fjuma @darranl IronJacamar still relies on PicketBox. Because of that, I had to temporary restore PicketBox dependency to make ee-9 connector compile:
tadamski@8cd9981#diff-31d9e3bb0671159c056658b3acb69396cbcf1a43a00db007c39c99817177c190R262

there is a integration layer that I think should be migrated to Elytron:
https://github.com/ironjacamar/ironjacamar/tree/main/core/impl/src/main/java/org/jboss/jca/core/security/picketbox

I have created https://issues.redhat.com/browse/JBJCA-1441 to deal with this issue.

The question is whether it should be implemented before this PR is fixed. For now, I have updated the branch so that it can be merged before the fix.

Comment on lines 786 to 789
<exclusion>
<groupId>org.jboss.ironjacamar</groupId>
<artifactId>ironjacamar-common-impl-jakarta</artifactId>
</exclusion>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @tadamski , you have to exclude the old non -jakarta versions from this feature pack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @yersan, fixed

ee-9/pom.xml Outdated
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

This dependency no longer exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@bstansberry bstansberry merged commit b3c7296 into wildfly:main Feb 21, 2022
@bstansberry
Copy link
Contributor

Thanks @tadamski! :) And @yersan for the review

@tadamski tadamski deleted the WFLY-15542 branch July 6, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
4 participants