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-15546 Move WildFly Preview to a native Jakarta namespace variant of the jpa subsystem module #14866

Merged
merged 5 commits into from Nov 5, 2021

Conversation

scottmarlow
Copy link
Contributor

… of the jpa subsystem module

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Nov 1, 2021
Copy link
Contributor

@yersan yersan left a comment

Choose a reason for hiding this comment

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

Hello @scottmarlow , looking at the "Wildfly Preview - Linux - JDK 17" traces, the wildfly-jpa subsystem is still being transformed there:

[19:42:41] : [Step 2/5] [INFO] EE9: transformed wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar

There is still something in the subsystem which is being transformed. I had similar issues when I worked on JSF, take a look at this comment to know how you can check what's pending:

#14823 (comment)

@@ -49,6 +49,7 @@
<module>ee</module>
<module>ee-security</module>
<module>jpa/spi</module>
<module>jpa/subsystem</module>
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor formatting issue

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@scottmarlow
Copy link
Contributor Author

Hello @scottmarlow , looking at the "Wildfly Preview - Linux - JDK 17" traces, the wildfly-jpa subsystem is still being transformed there:

[19:42:41] : [Step 2/5] [INFO] EE9: transformed wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar

There is still something in the subsystem which is being transformed. I had similar issues when I worked on JSF, take a look at this comment to know how you can check what's pending:

#14823 (comment)

I do not see a comment at https://github.com/wildfly/wildfly/blob/main/ee-9/feature-pack/pom.xml#L322.

Was it the following line that you uncommented?

 <!--<jakarta-transform-verbose>true</jakarta-transform-verbose>-->

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@yersan
Copy link
Contributor

yersan commented Nov 2, 2021

Ups, yes, this code is changing very quickly, yes uncomment the <!--<jakarta-transform-verbose>true</jakarta-transform-verbose>--> to get more information about why an artifact is being transformed

@bstansberry
Copy link
Contributor

@scottmarlow https://gist.github.com/bstansberry/5df0cbd64ae3b35c6a60964b00058f0b shows the details about why wildfly-jpa-jakarta is being transformed.

@scottmarlow
Copy link
Contributor Author

@scottmarlow https://gist.github.com/bstansberry/5df0cbd64ae3b35c6a60964b00058f0b shows the details about why wildfly-jpa-jakarta is being transformed.

Some of what I see:

[INFO] Stop processing [ org/jboss/as/jpa/service/PersistenceUnitServiceImpl$1$1.class ] using [ CLASS ]: Content changes
[INFO] Stop processing [ org/jboss/as/jpa/service/PhaseOnePersistenceUnitServiceImpl.class ] using [ CLASS ]: Content changes
[INFO] Stop processing [ org/jboss/as/jpa/service/PhaseOnePersistenceUnitServiceImpl$2$1.class ] using [ CLASS ]: Content changes
[INFO] Stop processing [ org/jboss/as/jpa/service/PhaseOnePersistenceUnitServiceImpl$1$1.class ] using [ CLASS ]: Content changes
[INFO] Stop processing [ org/jboss/as/jpa/service/PersistenceUnitServiceImpl.class ] using [ CLASS ]: Content changes

And:

Stop processing [ /Users/bstansberry/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ] using [ JAR ]: Content changes
[INFO] ================================================================================
[INFO] [ Input  ] [ /Users/bstansberry/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]
[INFO]            [ /Users/bstansberry/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]
[INFO] [ Output ] [ /Users/bstansberry/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]
[INFO]            [ /Users/bstansberry/dev/wildfly/wf2/wildfly/ee-9/feature-pack/target/jakarta-transform-maven-repo/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]
[INFO] ================================================================================
[INFO] [ Immediate changes: ]
[INFO] --------------------------------------------------------------------------------
[INFO] [          All Resources ] [    172 ] Unselected [      0 ] Selected [    172 ]
[INFO] --------------------------------------------------------------------------------
[INFO] [            All Actions ] [    172 ]  Unchanged [    167 ]  Changed [      5 ]
[INFO] [           Class Action ] [    134 ]  Unchanged [    129 ]  Changed [      5 ]
[INFO] [ Properties file relocate ] [      2 ]  Unchanged [      2 ]  Changed [      0 ]
[INFO] [        Manifest Action ] [      1 ]  Unchanged [      1 ]  Changed [      0 ]
[INFO] [  Service Config Action ] [      2 ]  Unchanged [      2 ]  Changed [      0 ]
[INFO] [            Text Action ] [      4 ]  Unchanged [      4 ]  Changed [      0 ]
[INFO] [            Null Action ] [     29 ]  Unchanged [     29 ]  Changed [      0 ]
[INFO] ================================================================================
[INFO] EE9: transformed wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar
@scottmarlow

So to complete the transformation of wildfly-jpa into wildfly-jpa-jakarta, we need additional changes to also transform the identified five class files for the identified constant pool entries.

@bstansberry
Copy link
Contributor

@ropalka FYI. This looks like another case where the rules that get applied for source transformation don't handle things that class transformation does. I'm not asking you to do anything about this; just giving you an fyi so you have context if we ever attack this issue.

@scottmarlow I suggest you fix this in the existing code by loading the BeanManager and ValidatorFactory classes and checking the beginning of the FQCN of the loaded class. If it's 'jakarta' use that in the constants, otherwise use javax. My assumption there is we want the constants to vary between jakarta and javax. :). If not what the Galleon plugin transformation is doing is wrong, and then the fix would be to break up those strings so they no longer match the transformation rule.

…n doesn't currently cover

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@scottmarlow
Copy link
Contributor Author

Looks better now with your suggested changes @bstansberry!

Stop processing [ /home/smarlow/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ] using [ JAR ]: No changes$
[INFO] ================================================================================$
[INFO] [ Input  ] [ /home/smarlow/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]$
[INFO]            [ /home/smarlow/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]$
[INFO] [ Output ] [ /home/smarlow/.m2/repository/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]$
[INFO]            [ /home/smarlow/work/wildfly/ee-9/feature-pack/target/jakarta-transform-maven-repo/org/wildfly/wildfly-jpa-jakarta/26.0.0.Beta1-SNAPSHOT/wildfly-jpa-jakarta-26.0.0.Beta1-SNAPSHOT.jar ]$

Thanks @yersan for helping with ^!

@bstansberry
Copy link
Contributor

bstansberry commented Nov 5, 2021

@scottmarlow My guess is the test failures relate to "jakarta" not being handled in https://github.com/wildfly/wildfly/blob/main/jpa/subsystem/src/main/java/org/jboss/as/jpa/classloader/TempClassLoader.java#L73

Because of that the jakarta.persistence.Entity associated with entityClass in the test != the real one because it has a different defining classloader.

Copy link
Contributor

@bstansberry bstansberry left a comment

Choose a reason for hiding this comment

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

Need to resolve test failures.

@ropalka
Copy link
Contributor

ropalka commented Nov 5, 2021

Thanks @bstansberry for letting me know. I see @scottmarlow already fixed it. Yes, there are sometimes some tweaks needed to make things work.

…g. spec api) classes as not to be loaded by temp classloader.

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@bstansberry bstansberry added the EE9 label Nov 5, 2021
@bstansberry bstansberry merged commit 0a39b14 into wildfly:main Nov 5, 2021
@bstansberry
Copy link
Contributor

Thanks, @scottmarlow!

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