Can one of the admins verify this patch?

FWIW I'm a Red Hat employee and thus covered by Red Hat's Eclipse Contributor Agreement, and that's visible on my Eclipse profile, so I don't know why the automated check on GitHub fails.

EDIT: The problem was a trailing space in the "First name" field in my Eclipse profile 🤦

Is the intention for this to be a service release of the API jar for EE10 or is would it be required to wait for EE11?

Is the intention for this to be a service release of the API jar for EE10 or is would it be required to wait for EE11?

From my point of view this is essentially a bugfix. As far as I'm concerned, this would need to be included in a bugfix release in EE10 (e.g. jakarta.transaction-api 2.0.2).

In my opinion there's also no need to wait for EE11, because this change shouldn't break any application. As I understand it, users either are not using CDI and then removing the dependency won't affect them negatively, or they are using CDI and then all the related modules are already in their modulepath and will continue to be exposed to users transitively through this module-info.

However, I am not familiar with the Jakarta release process at all. Maybe it's too late for such change in EE10? You probably know better than I do.

Thank you for the clarification

ok to test

Hey. It looks like the build got stuck or was aborted for some reason... can anyone restart it?

retest this please

The testing infra seems to be having problems:

[ERROR] Error executing Maven.
[ERROR] The specified user settings file does not exist: /home/jenkins/agent/workspace/eclipse-ee4j_jta-api-pulls-openjdk-jdk11-latest/glassfish/snapshots/settings.xml
Build step 'Execute shell' marked build as failure

retest this please

I have commented out a line of the build config mvn -s ./snapshots/settings.xml clean install -f ./snapshots/pom.xml - snapshots/ folder is not in the current glassfish master branch.

[ERROR] Rule 0: org.apache.maven.plugins.enforcer.RequireMavenVersion failed with message:
Detected Maven Version: 3.8.5 is not in the allowed range 3.8.6.

:)

retest this please

Thanks, the job was configured to use apache-maven-latest but maybe this is not updated yet to point to a version that would satisfy the check. I found apache-maven-3.8.6 in the list though and so have configured it to be that

Thank you for working on this :)

Next one...

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-antrun-plugin:3.1.0:run (do stuff) on project glassfish: An Ant BuildException has occured: The following error occurred while executing this line:
[ERROR] /home/jenkins/agent/workspace/eclipse-ee4j_jta-api-pulls-openjdk-jdk11-latest/glassfish/appserver/distributions/glassfish/target/antrun/build-main.xml:9: The archive microprofile-jwt-auth-api.jar doesn't exist
[ERROR] around Ant part ...<jarupdate basedir="/home/jenkins/agent/workspace/eclipse-ee4j_jta-api-pulls-openjdk-jdk11-latest/glassfish/appserver/distributions/glassfish/src/main/patches/microprofile-jwt-auth-api" destfile="/home/jenkins/agent/workspace/eclipse-ee4j_jta-api-pulls-openjdk-jdk11-latest/glassfish/appserver/distributions/glassfish/target/stage/glassfish7/glassfish/modules/microprofile-jwt-auth-api.jar" includes="META-INF/MANIFEST.MF" />... @ 17:430 in /home/jenkins/agent/workspace/eclipse-ee4j_jta-api-pulls-openjdk-jdk11-latest/glassfish/appserver/distributions/glassfish/target/antrun/build-main.xml

You are welcome :)

I wonder if Glassfish is building fine on their main branch now. I will try to have a look.

The reason we use Glassfish is to run the CTS with changes to make sure nothing is unexpectedly breaking.

I have a feeling this might affect the current master branch of Glassfish. The most recent weekly build did pass (https://ci.eclipse.org/glassfish/view/GlassFish/job/glassfish_weekly_build/171/) but there are changes in Glassfish since 93176e2555176091c8522e43d1d32a0a30652d4a

I am not sure why those approvals were just provided from those accounts. I don't think they are committers though: https://projects.eclipse.org/projects/ee4j.jta/who

I am not sure why those approvals were just provided from those accounts. I don't think they are committers though: https://projects.eclipse.org/projects/ee4j.jta/who

Sorry Tom. We approved it because our team/project ran into exactly the same issue.
Having these requires as static would solve these issues for us.
We just upgraded our project from Spring boot 2 > 3 (which requires an javax > jakarta upgrade), but due to that upgrade, we were required to add a dependency on jakarta cdi as well (due to reason Yoann posted here).

We would've prefered a thumbs up / vote as it seemed this PR was a bit forgotten, but that wasn't possible. So we wanted to raise some awareness of this PR again. Although we're not familiar with the Jakarta release process, so this might still have been on the radar though.

Ah, great - thank you for clarifying!

Hello @tomjenkinson! Is there something I can help with to get this merged?

Hey @tomjenkinson , is there any chance to get this merged?

retest this please

@yrodiere I have requested CI to retest this, thanks

retest this please

There was something failing in JWT. I found a SHA in CI of something I hope to pass. It might need JDK upgrade though to 17 as the job is currently using JDK 11.

I am trying a build locally. I tried a few versions of a build earlier but couldn't get the build to fail for a missing microprofile-jwt-auth-api.jar but I am using the same SHA now. Also with 78a3424568a59097da7c6479c74cb7ccc42a749e it does seems to be at least getting the build started with JDK 11

Ah, 78a3424568a59097da7c6479c74cb7ccc42a749e (Glassfish) has just failed for me locally with JDK 11

Trying locally with JDK 17, will see if that works

JDK 17 build worked locally but I think Glassfish is expecting to be able to build with JDK 11

I think there is something not working with the Glassfish build and JDK 11. I think that with 7.0.12 MVN_EXTRA=-DskipTests ./gfbuild.sh dev_build is not working and I think with JDK 17 it will.

Alright... does that mean we should ask the help from someone working on Glassfish?

Alternatively, maybe the master branch should use an older version of Glassfish for its builds? IMO this would make sense as long as this repo is not trying to build something for a new Jakarta EE release. After all, we currently want the code on the master branch to still work on JDK 11, no?

And it would make sense to not tie the specific change in this PR to a newer Jakarta EE release, since it's essentially a hotfix for something related to packaging that is not covered by the spec itself.

I did raise the problem on glassfish-dev on Thursday (from the timestamp on https://www.eclipse.org/lists/glassfish-dev/msg01541.html).

I think the version should be a 7 version as that is EE10 (https://github.com/eclipse-ee4j/glassfish/blob/master/README.md) - do you agree? If so I think this is probably still their master branch as I don't think I spot a different one we can use: https://github.com/eclipse-ee4j/glassfish/branches.

I did raise the problem on glassfish-dev on Thursday (from the timestamp on https://www.eclipse.org/lists/glassfish-dev/msg01541.html).

Great, thanks. Let's wait for answers then.

I think the version should be a 7 version as that is EE10 (https://github.com/eclipse-ee4j/glassfish/blob/master/README.md) - do you agree? If so I think this is probably still their master branch as I don't think I spot a different one we can use: https://github.com/eclipse-ee4j/glassfish/branches.

I'm not very familiar with Glassfish but from what I can see, you're right.

I have raised an issue on the Glassfish repo: eclipse-ee4j/glassfish#24794

I have made a job change to create a patch to get GlassFish to build with JDK 11 and to apply it

retest this please

The good news is that I have seen a build that is running the TCK now, but the bad news is there are a number of failures (https://ci.eclipse.org/jta/view/Enabled/job/eclipse-ee4j_jta-api-pulls-openjdk-jdk11-latest/67/console). I have retriggered the run and thus a build will be hooked into the GitHub checks. I think it may well be not set up correctly yet still because it's consistently the "from_standalone" tests (exceptionally for the signature test which does pass in the "from_standalone" variant).

I think it is probably something to do with "Caused by: java.lang.ClassNotFoundException: org.glassfish.main.jul.GlassFishLogger"...

This class was added in GlassFish 7 (eclipse-ee4j/glassfish@b68a6a8) but it looks like we used GlassFish 6 for EE 9 (https://jakarta.ee/specifications/transactions/2.0/). Perhaps something will need configuring to expose this Jar

retest this please

I am trying with to see if I can add ${webServerHome}/lib/bootstrap/glassfish-jul-extension.jar${pathsep} into something that hopefully controls a relevant classpath for the 2.0.2 TCK. If it works that seems OK for now but we would want a change in to get it into future TCKs (so master version of https://github.com/jakartaee/platform-tck/blob/aa2b698/install/jta/bin/ts.jte#L68)

Well I certainly didn't expect this PR to cause you so much trouble :] Thanks for staying on top of this, and let me know if I can help.

Even though we are closing in on getting the CI issue resolved, @arjantijms has raised a good point in #214 about the appropriateness of doing this in an update. @yrodiere please can I ask you to provide input on that thread too?

retest this please

Great, it passes CI - thank you :)

Input on this issue was solicited on the jta-dev email list. I have no stake at all in this issue one way or the other. My input is: see https://mail.openjdk.org/pipermail/jigsaw-dev/2023-April/014850.html (and following, and preceding), where the intent of requires static is discussed. Short version: it does not mean an optional dependency, apparently, but many users wish it would.

I am glad to see activity on transactions, however I am unaware of any plan to include an update on transactions in EE 11.

https://jakartaee.github.io/platform/jakartaee11/JakartaEE11ReleasePlan

Please confirm.

Input on this issue was solicited on the jta-dev email list. I have no stake at all in this issue one way or the other. My input is: see https://mail.openjdk.org/pipermail/jigsaw-dev/2023-April/014850.html (and following, and preceding), where the intent of requires static is discussed. Short version: it does not mean an optional dependency, apparently, but many users wish it would.

I can't comment on what requires static is supposed to mean, but in practice it achieves what we want it to achieve, so as far as I'm concerned I'd happily trade the current (unsatisfying) behavior for that of requires static, even if it may become unsatisfying with future JDK/Jakarta Transaction changes (maybe).

I am glad to see activity on transactions, however I am unaware of any plan to include an update on transactions in EE 11.

https://jakartaee.github.io/platform/jakartaee11/JakartaEE11ReleasePlan

Please confirm.

This is a year-old pull-request on what is essentially an unspecified aspect, and arguably a bug, of the published JARs of Jakarta Transaction.

This is not a request to change the spec, and I personally do not think changing the spec is necessary, considering the content of module-info in spec JARs is often unspecified, both here and in other Jakarta specs.

Hey @tomjenkinson , about the static keyword in Jakarta Transaction's module info for the CDI dependency... I'm not sure what the conclusion was, be it here or in #214.

Do you know who would have authority to settle the issue, one way or another?

Reminder: this is an unspecified aspect of Jakarta Transactions. I could not even find the keyword "module" in the spec, at least in version 2.0, let alone a precise description of its content -- which makes sense IMO. So the issue is mainly about the API JAR, not about the spec itself.

I think the conclusion I came to was in #214 (comment) (as in, CDI is not optional from a specification point of view). I would be happy to be convinced of a different perspective but it should respond to the aspect that CDI is not (currently) optional for Jakarta Transactions.

Maybe my read is too narrow though and maybe the API does not have to enforce that constraint unnaturally? Please can we discuss on the issue or even more preferably on the Jakarta Transactions mailing list thread: https://www.eclipse.org/lists/jta-dev/msg00315.html (or even Jakarta Platform mailing list: https://www.eclipse.org/lists/jakartaee-platform-dev/msg04441.html)