search

jakartaee / transactions

Welcome to the Jakarta EE Transactions API Project (formerly JTA)
https://jakartaee.github.io/transactions/
Other
29 stars 30 forks source link

Update API jar OSGi manifest headers #186

Closed grgrzybek closed 3 years ago

grgrzybek commented 3 years ago

Related issues:

Related commits:

According to Oracle JavaEE 8 components page, JavaEE 8 uses JTA 1.2 and JCA 1.7 specification versions. According to Eclipse EE4J / JakartaEE 8 specification, JakartaEE 8 uses JTA 1.3 and JCA 1.7 specifications, which is clearly explained in Transaction 1.3 vs 1.2 section.

However, https://github.com/eclipse-ee4j/jta-api/commit/79d477b6b1503099af62c2b25fcb854f4b17a7e3 commit confirms that the package (API) and specification versions may differ.

In my opinion, because JakartaEE 8 specification explicitly says:

No API changes were made in this Maintenance Release or after contribution and Java™ Transaction API 1.2, Java™ Transaction API 1.3 and Jakarta Transaction 1.3 are functionally equivalent.

the version of OSGi package for javax.transaction should stay at 1.2 to express (I know it's only in OSGi world, as there is no concept of package versions in JavaEE/JakartaEE) the fact that it didn't change.

This would also fix the problem I described in https://github.com/eclipse-ee4j/jca-api/issues/120 - JCA 1.7 API should use JTA 1.2 API and while it's only a documentation issue at specification level, it's really critical at OSGi package version level.

grgrzybek commented 3 years ago

There are also two confusing OSGi configuration elements:

  1. since https://github.com/javaee/javax.transaction/commit/341344a51baea82a848d6787ba3fea89f2ce2f8d (2013), javax.interceptor package is imported with confusing [1.2,1.2.98) version range. It should be simply [1.2,2.0)
  2. there's <_include>-${basedir}/osgi.bundle</_include>, a leftover from https://github.com/javaee/javax.transaction/commit/cafce67773652a48d37f4f18cd317cfbd6371802#diff-5b28dfac4685527f0aa99f6fde21966a5347c363d64baf1680fd1db0b0f14e73, which doesn't make any sense since introduction of api and spec subdirs in https://github.com/eclipse-ee4j/jta-api/commit/9256156ed6ec5d3bf3bdd5c66ca0071f78b381b1

top level osgi.bundle file contains very bad Require-Bundle: system.bundle, which accidentally makes javax.transaction-api/1.2 bundle work in Karaf even if Karaf contains confusing Export-Package: javax.transaction;partial=true;mandatory:=partial that's somehow matches the trick used in commons-dbcp2, which I'm trying to explain in https://issues.apache.org/jira/browse/DBCP-571 issue...

grgrzybek commented 3 years ago

And last thing, javax.enterprise.context, javax.enterprise.util and javax.interceptor package imports should be marked as optional, as they related to annotations only. JTA API Bundle should resolve without a need to install cdi-api and interceptor-api (which requires also el-api and inject-api bundles).

tomjenkinson commented 3 years ago

Hi @grgrzybek I am not really sure how to deal with this I don't think the OSGi metadata is part of the API of the specification. If necessary feedback could be solicited from the main platform group (https://accounts.eclipse.org/mailing-list/jakartaee-platform-dev) to decide if it is a defect.

The package kind of changed in that the sub-package (.xa) is no longer provided in the specification but as the JDK provides that it should be functionally equivalent so reflecting that in the version number upgrade could still be relevant to OSGi perhaps?

grgrzybek commented 3 years ago

Thanks for your feedback. From the point of view of JavaEE/JakartaEE everything is ok. Removal of javax.transaction.xa package is well explained and justified. Also the OSGi metadata is definitely not a concern of any JakartaEE expert group or API. It's only a technical flavor added by adding maven-bundle-plugin:manifest Maven goal to process-classes build phase.

From the point of view of OSGi, there's no concept of subpackage. There are only two fully-equivalent packages:

The fact that javax.transaction.xa is now (in JTA 1.3) exposed from different JAR (actually from the JDK itself), doesn't matter at all. What matters is the version associated with a package (declared in Export-Package header in META-INF/MANIFEST.MF).

Mind that this issue is not a blocker for me. Simply from the point of view of OSGi, javax.transaction package didn't change at all despite the specification version change from 1.2 to 1.3.

But adding the problem related to top-level osgi.bundle file, which wasn't moved when api submodule was created (see https://github.com/eclipse-ee4j/jta-api/commit/9256156ed6ec5d3bf3bdd5c66ca0071f78b381b1) (and the Jakarta JTA jar lost Require-Bundle header), I think it'd be good to have the manifest reviewed by OSGi dev ;)

grgrzybek commented 3 years ago

PR #187 (I'm not sure about the target branch) contains:

The changes have absolutely no impact on the JavaEE/JakartaEE side if the API jar.

These changes were tested with https://issues.apache.org/jira/browse/KARAF-7032

tomjenkinson commented 3 years ago

Thank you

tomjenkinson commented 3 years ago

@grgrzybek given you are proposing it for removal, please can you clarify why Require-Bundle: system.bundle is bad?

tomjenkinson commented 3 years ago

@grgrzybek was any of the change in the PR related to making "javax.enterprise.context, javax.enterprise.util and javax.interceptor" optional?

grgrzybek commented 3 years ago

@tomjenkinson

Require-Bundle: system.bundle is a "strong coupling" between JTA API Jar (this bundle) and "system bundle" which exports big number of packages (most of JDK's own packages like java.lang). The strong coupling was used because JDK itself provided javax.transaction package with 3 exception classes inside it. Such strong coupling would be needed if jakarta.transaction-api didn't provide those 3 exception classes, but it's not a case. This is (quite confusingly) described in the OSGi Core specification. Because jakarta.transaction-api provides these 3 exception classes (and JDK 11 doesn't even use them), then it's wise to create loose coupling between jakarta.transaction-api and "the system" (represented by "bundle 0" or "system.bundle").

And nothing in my PR concerns the optional import of javax.enterprise.* and javax.interceptor package. Let's leave it as is to not change existing OSGi configurations (for example Karaf transaction feature).

tomjenkinson commented 3 years ago

Thanks @grgrzybek !

tomjenkinson commented 3 years ago

The OSGi headers are updated in master and there is a backport PR of this https://github.com/eclipse-ee4j/jta-api/pull/189 - as the change is merged in master I will close this issue but if you would like to request a 1.3.4 release then please can you send a message on https://accounts.eclipse.org/mailing-list/jta-dev

grgrzybek commented 3 years ago

Thanks @tomjenkinson - I'll send a message to the mentioned mailing list.

grgrzybek commented 3 years ago

FYI, I'm waiting a bit, because I found some issue in JAF as well: https://github.com/eclipse-ee4j/jaf/issues/67

arjantijms commented 2 years ago

@grgrzybek There obviously went some thinking into this and it's not just a random copy/paste error, but why did you declare that the module is exporting javax.transaction, while it really only has jakarta.transaction?

You mentioned something about javax.transaction being in the JDK, but is it?

Maybe this highly onorthodox and very confusing thing needs some explanation in the pom itself?

tomjenkinson commented 2 years ago

@arjantijms FYI when this PR was first raised it was against the Jakarta EE8 version (hence javax.transaction) https://github.com/eclipse-ee4j/jta-api/pull/187/files I think we should consider to change to jakarta.transaction now.

arjantijms commented 2 years ago

I guess just removing the two sections

would be in order then.

tomjenkinson commented 2 years ago

I agree and I do have a PR with this (and some other changes) at: https://github.com/eclipse-ee4j/jta-api/pull/202/files