-
Notifications
You must be signed in to change notification settings - Fork 133
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
[OPENJPA-2762] Update JMS spec to v2.0 and set scope to 'provided' #37
base: master
Are you sure you want to change the base?
Conversation
due to activemq (the real one, the v5 ;)) being widespread, we should stick to jms 1 which will ensure we have the right osgi metadata for amq5 and jms 2 impls IMHO |
@rmannibucau haha =). Good news though, by bumping to JMS v2.0 it won't break ActiveMQ 5.x compatibility. Rather, the JMS code in OpenJPA only uses the JMS v1.1 APIs and the JMS v2.0 API is backwards comparable. Also, ActiveMQ 5.x is adding JMS v2.0 support starting in 5.17.0, so updating this will be a good thing for ActiveMQ 5.x support going forward =). Not to mention, eventually we'll have to bump OpenJPA to use the jakarta.jms (v3.0) jars coming soon. |
@mattrpav well AMQ being so much widespread, 5.17 is not sufficient to upragde ;). Al note that your assumption JMS 2 is backward compatible is wrong in terms of OSGi metadata (osgi.contract) so we must stick on JMS 1 to ensure we run with JMS 1 and JMS 2. For jakarta, and due to our JMS usage (and future usage), I think we'll just do as several other asf project and use maven shade plugin to provide a jakarta friendly packaging without breaking javax which stays mainstream for now nor forking the code. |
@rmannibucau what do you mean by "wrong in terms of OSGi metadata"? There are only two classes in OpenJPA that make use of JMS, and they both use the API signatures that are compatible with both JMS 1.1 and 2.0. If we do not change the OSGi import, OpenJPA users would not be able to use current versions of IBM MQ or Tibco EMS either. I believe the OSGi import in this PR is correct, saying "OpenJPA will support using v1.1 up to (but not including 3.0)". This is the same OSGi import setting that other Apache projects have also adopted. |
@mattrpav I'm not speaking of code (so don't look in .java) but of MANIFEST.MF data. We have an osgi.contract requiring JMS 1 normally. It means we can work with JMS 1 and 2. What is wrong seems the pom - but your PR does not change anything at that level. Typically:
should now be
(tip: for jms 3 it should be rewritten with the shade plugin transformer). We should probably also ensure we add JMS as an osgi.contract requirement I think but not in a hurry on this one. Hope it makes sense. |
@rmannibucau yep, I'm aware of the osgi manifest declarations. The maven bundle plugin generates those values based on the maven dependencies. Again, by staying with the JMS 1.1 requirement, we are limiting JMS providers, and making it more difficult to integrate OpenJPA with other projects. For example, recent versions of Camel ship with JMS v2.0 jar dependencies. If we force OpenJPA to stay with 1.1, we will never get a valid wiring for a project that depends on Camel+OpenJPA. |
This is wrong, osgi knows that v1 works with v2, this is how all specs are done btw. |
@rmannibucau I'm not following you at all. When would you think it is suitable to upgrade to the v2 of the JMS specification? Note-- in the patch, I set the dependency as provided to allow for users to determine at runtime. |
@mattrpav when we would use jms2 api only (not tomorrow IMHO ;)). Once again, issue is not about the runtime but manifest metadata + proposed api whzn developping the project. If your goal is to run on jms 2 with osgi, please fix the pom as mentionned, no need of a dependency change. If you want to run in a EE container or standalone app, we already do it. |
No description provided.