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

Solving module conflicts with bundled jar #36

Merged
merged 10 commits into from
Nov 4, 2022

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Oct 21, 2022

Solves:
#30
#32
#33
#35

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos jbescos marked this pull request as ready for review October 24, 2022 07:37
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>

module example.app {
requires jakarta.mail;
requires java.management;
Copy link
Contributor

Choose a reason for hiding this comment

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

requires java.logging too?

Copy link
Member Author

@jbescos jbescos Oct 26, 2022

Choose a reason for hiding this comment

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

Yes, it should because there is a reference to that in one class. But this should throw a compilation error and it is not happening. What could be the reason of this missing error?.

Copy link
Contributor

@jmehrens jmehrens Oct 27, 2022

Choose a reason for hiding this comment

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

I think it works because jakarta.mail requires logging too. As in: requires transitive java.logging; is present in other module info files that is being required in this one.

So I think this is non-issue.

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos
Copy link
Member Author

jbescos commented Nov 1, 2022

I'm going to update this PR to support again JDK8. Setting it to draft.

@jbescos jbescos marked this pull request as draft November 1, 2022 07:26
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos
Copy link
Member Author

jbescos commented Nov 1, 2022

It supports JDK8, I have verified it with:
https://github.com/jbescos/jakartaEE10MailDemo/tree/jdk8

And tests are running with modules, as can be seen here:

[DEBUG] Path to args file: /home/jbescos/workspace/angus-mail/demos/demo/target/surefire/surefireargs-20221101084122542_300
[DEBUG] args file content:
--module-path
"/home/jbescos/workspace/angus-mail/demos/demo/target/classes:/home/jbescos/workspace/angus-mail/providers/angus-mail/target/angus-mail-1.0.1-SNAPSHOT.jar:/home/jbescos/.m2/repository/jakarta/activation/jakarta.activation-api/2.1.0/jakarta.activation-api-2.1.0.jar:/home/jbescos/.m2/repository/jakarta/mail/jakarta.mail-api/2.1.0/jakarta.mail-api-2.1.0.jar:/home/jbescos/.m2/repository/org/eclipse/angus/angus-activation/1.0.0/angus-activation-1.0.0.jar"
--class-path
"/home/jbescos/.m2/repository/org/apache/maven/surefire/surefire-booter/3.0.0-M7/surefire-booter-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/surefire-api/3.0.0-M7/surefire-api-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/surefire-logger-api/3.0.0-M7/surefire-logger-api-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/surefire-shared-utils/3.0.0-M7/surefire-shared-utils-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/surefire-extensions-spi/3.0.0-M7/surefire-extensions-spi-3.0.0-M7.jar:/home/jbescos/workspace/angus-mail/demos/demo/target/test-classes:/home/jbescos/.m2/repository/junit/junit/4.13.2/junit-4.13.2.jar:/home/jbescos/.m2/repository/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/surefire-junit4/3.0.0-M7/surefire-junit4-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/common-java5/3.0.0-M7/common-java5-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/common-junit3/3.0.0-M7/common-junit3-3.0.0-M7.jar:/home/jbescos/.m2/repository/org/apache/maven/surefire/common-junit4/3.0.0-M7/common-junit4-3.0.0-M7.jar"
--patch-module
example.app="/home/jbescos/workspace/angus-mail/demos/demo/target/test-classes"
--add-opens
example.app/example.app=ALL-UNNAMED
--add-modules
example.app
--add-reads
example.app=ALL-UNNAMED
org.apache.maven.surefire.booter.ForkedBooter
[DEBUG] Forking command line: /bin/sh -c cd '/home/jbescos/workspace/angus-mail/demos/demo' && '/home/jbescos/programs/java/jdk-11.0.13/bin/java' '@/home/jbescos/workspace/angus-mail/demos/demo/target/surefire/surefireargs-20221101084122542_300' '/home/jbescos/workspace/angus-mail/demos/demo/target/surefire' '2022-11-01T08-37-35_862-jvmRun1' 'surefire-20221101084122542_298tmp' 'surefire_90-20221101084122542_299tmp'
[DEBUG] Fork Channel [1] connected to the client.
[INFO] Running example.app.ModulesTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.148 s - in example.app.ModulesTest

@jbescos jbescos marked this pull request as ready for review November 1, 2022 08:23
@adamretter
Copy link

It supports JDK8

Could we add JDK 8 to GitHub Actions to avoid future regressions?

@jbescos
Copy link
Member Author

jbescos commented Nov 2, 2022

It supports JDK8

Could we add JDK 8 to GitHub Actions to avoid future regressions?

Yes, it is good idea because it is actually failing with that issue. I created a PR already: #40

@adamretter
Copy link

@jbescos Thank you so much :-)

Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

package declaration belongs bellow license header (with an empty line in between)

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos
Copy link
Member Author

jbescos commented Nov 2, 2022

package declaration belongs bellow license header (with an empty line in between)

Done, thank you

@lukasj lukasj merged commit 29c2825 into eclipse-ee4j:master Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants