Hi

As said I did it already but forgot to push.

I will take a look about your change to check if it’s the same I did.

Regards
JB

Le sam. 23 juil. 2022 à 21:46, Arthur Naseef <a...@amlinv.com> a écrit :

> Got the test application working, a PR with a fix that provides
> simultaneous support for JMS 1.1 and JMS 2.0 via the same Karaf feature
> (activemq-client).
>
> Please take a look at the comment on the ticket:
> https://issues.apache.org/jira/browse/AMQ-8971
>
> The good news about this fix is that it fixes backward compatibility for
> JMS 1.1 applications while retaining JMS 2.0 compatibility (i.e. does not
> further break backward compatibility).
>
> Art
>
> On Wed, Jun 22, 2022 at 9:15 AM Arthur Naseef <a...@amlinv.com> wrote:
>
> > Still working on a test project - almost got it working.
> >
> > Art
> >
> >
> > On Tue, Jun 21, 2022 at 8:27 AM Arthur Naseef <a...@amlinv.com> wrote:
> >
> >> Agreed on fixing it going forward and not simply reverting - that would
> >> really just create another non-backward-compatible change and increase
> the
> >> size of the problem.  The 5.16.3 - 5.17.1 releases are already in this
> >> state, and we can't fix that - hopefully anyone updating goes right for
> the
> >> latest (once we release a "fix"), and anyone else searching on the
> problem
> >> can find the jira ticket, this discussion, or similar resources which
> can
> >> point them at a work-around.
> >>
> >> I started writing a small test to reproduce the problem and try
> solutions.
> >>
> >> For the idea of providing both spec bundles, that could be a decent
> >> solution.  My only concern is that it could get messy for resolution
> >> because there would be 2 sets of classes, from different bundles, that
> >> could end up in the dependency chain.  In other words, some users could
> >> have some bundles wire to the 1.1 spec bundle, others wire to the 2.0
> spec
> >> bundle, and any wiring amongst those would fail because their JMS
> classes
> >> aren't the same ones.  You know, the dreaded, because it is exposed to
> >> package '...' from resources ... via two dependency chains.
> >>
> >> One solution I'm thinking here - use the feature file's "capability" to
> >> advertise the existing JMS 2 spec as providing the JMS 1.1 packages.  If
> >> the JMS 2 classes are truly backward-compatible, I believe that could
> "just
> >> work" for both cases (JMS 1.1 and JMS 2.0 applications).
> >>
> >> Thoughts?
> >>
> >> Art
> >>
> >>
> >> On Tue, Jun 21, 2022 at 7:50 AM Robbie Gemmell <
> robbie.gemm...@gmail.com>
> >> wrote:
> >>
> >>> I would fix it on 5.17.x as well unless theres some reason not to that
> >>> im missing, it really seems no different than it is for 5.16.x. People
> >>> can upgrade to 5.17.x from <=5.16.2 as well, and reasonably wouldnt
> >>> expect to hit a breakage for this any more than they should on 5.16.x,
> >>> since it also does not implement JMS 2 either.
> >>>
> >>> On Tue, 21 Jun 2022 at 15:36, Jean-Baptiste Onofré <j...@nanthrax.net>
> >>> wrote:
> >>> >
> >>> > Agree: I should not have changed on 5.16.x, keep it for 5.17.x.
> >>> >
> >>> > Now that it has been released, I think the best approach is to
> provide
> >>> both
> >>> > spec bundles.
> >>> >
> >>> > Let me test and create PR.
> >>> >
> >>> > Regards
> >>> > JB
> >>> >
> >>> > Le mar. 21 juin 2022 à 16:07, Robbie Gemmell <
> robbie.gemm...@gmail.com>
> >>> a
> >>> > écrit :
> >>> >
> >>> > > The obvious "why not" answer would be however easy it is, its
> perhaps
> >>> > > not so obvious to people, and it certainly doesnt seem like it
> should
> >>> > > be necessary. Those with things which only use JMS 1.1 and
> previously
> >>> > > worked with <=5.16.2 (its not just 5.15.x upgraders affected) would
> >>> > > not typically expect to be broken by a simple update to using
> >>> 5.16.3+,
> >>> > > or to necessarily understand they can work around the feature
> problem
> >>> > > by using the JMS 2 spec when their stuff isnt using that and they
> are
> >>> > > still clearly using a client implementing 1.1.
> >>> > >
> >>> > > If having both versions provided is possible, fixes simple upgrades
> >>> > > for all the existing JMS 1.1 users on <= 5.16.2, and still allows
> >>> > > those already working with JMS 2 to use it as now, then that would
> >>> > > seem a reasonable middle ground. The spec jar isnt exactly a
> >>> monstrous
> >>> > > overhead after all, especially not compared to the client feature
> >>> > > already supplying [most of] the broker etc.
> >>> > >
> >>> > > Or, you suggested earlier what would happen currently is it would
> >>> only
> >>> > > use/supply 2.0 unless something provided 1.1 first. Can it do the
> >>> > > reverse, i.e can it provide 1.1 as it did before but still allow
> for
> >>> > > using 2 if already supplied, falling back to using its provided 1.1
> >>> if
> >>> > > they dont?
> >>> > >
> >>> > >
> >>> > > On Tue, 21 Jun 2022 at 14:01, Jean-Baptiste Onofré <
> j...@nanthrax.net>
> >>> > > wrote:
> >>> > > >
> >>> > > > OK, now I understand the confusion:
> >>> > > >
> >>> > > > Karaf activemq-client feature uses activemq-osgi bundle, not
> >>> > > > activemq-client bundle. The activemq-client bundle is not used at
> >>> all
> >>> > > > in the Karaf features: we use the activemq-osgi uber bundle.
> >>> > > >
> >>> > > > So, if a user uses activemq-client bundle (without the feature),
> it
> >>> > > > will have to install geronimo-spec-jms 1.1 bundle:but nothing
> >>> changed
> >>> > > > there, it's as it was before.
> >>> > > >
> >>> > > > Now, strictly speaking of the activemq-client karaf feature, it's
> >>> fine
> >>> > > > as it uses activemq-osgi bundle, with the
> >>> javax.jms,version="[1.1,3)"
> >>> > > > range.
> >>> > > >
> >>> > > > Regarding Art's issue, the problem is that activemq-client karaf
> >>> > > > feature provides JMS 2.0 by default, but Art's bundle still
> import
> >>> > > > [1.1,2) (not [1.1,3)).
> >>> > > >
> >>> > > > I see three options here:
> >>> > > > 1. Art can fix his bundles header to use the extended range
> >>> [1.1,3).
> >>> > > > 2. The user who wants to still use JMS 1.1, they can stay with
> >>> ActiveMQ
> >>> > > 5.15.x
> >>> > > > 3. The user who wants to still use JMS 1.1, we can add
> >>> geronimo-spec
> >>> > > > jms 1.1 in activemq-client karaf feature, meaning that we will
> have
> >>> > > > both JMS 1.1 and 2.0 packages at runtime.
> >>> > > >
> >>> > > > Honestly, why not extending the range, easy to do and it works
> fine
> >>> > > > (it's what Karaf and Camel are using) ?
> >>> > > >
> >>> > > > Regards
> >>> > > > JB
> >>> > > >
> >>> > > >
> >>> > > >
> >>> > > >
> >>> > > >
> >>> > > >
> >>> > > > On Tue, Jun 21, 2022 at 1:53 PM Jean-Baptiste Onofré <
> >>> j...@nanthrax.net>
> >>> > > wrote:
> >>> > > > >
> >>> > > > > I tested at runtime on activemq-osgi bundle used by
> >>> activemq-client.
> >>> > > > >
> >>> > > > > The feature verify would not work with this range.
> >>> > > > >
> >>> > > > > Let me take a look but I doubt it's the case.
> >>> > > > >
> >>> > > > > On Tue, Jun 21, 2022 at 11:53 AM Robbie Gemmell
> >>> > > > > <robbie.gemm...@gmail.com> wrote:
> >>> > > > > >
> >>> > > > > > The javax.jms; version="[1.1,2)" value I quoted was directly
> >>> from the
> >>> > > > > > Import-Package manifest entry of the 5.16.3 and 5.16.5
> >>> > > activemq-client
> >>> > > > > > jars on maven central. On checking 5.17.1 it lists the same.
> >>> > > > > >
> >>> > > > > > On Tue, 21 Jun 2022 at 09:56, Jean-Baptiste Onofré <
> >>> j...@nanthrax.net>
> >>> > > wrote:
> >>> > > > > > >
> >>> > > > > > > activemq-client 5.16.3 does use the right range:
> >>> > > > > > >
> >>> > > > > > >    javax.jms;version="[1.1,3)",
> >>> > > > > > >
> >>> > > > > > > Else it won't work.
> >>> > > > > > >
> >>> > > > > > > And by the way, before the change, I sent a couple of
> >>> messages on
> >>> > > the
> >>> > > > > > > mailing list as a discussion thread.
> >>> > > > > > >
> >>> > > > > > > Regards
> >>> > > > > > > JB
> >>> > > > > > >
> >>> > > > > > > On Tue, Jun 21, 2022 at 10:37 AM Robbie Gemmell
> >>> > > > > > > <robbie.gemm...@gmail.com> wrote:
> >>> > > > > > > >
> >>> > > > > > > > I believe the 5.16.x client doesnt have the below,
> instead
> >>> > > saying:
> >>> > > > > > > >     javax.jms; version="[1.1,2)"
> >>> > > > > > > > despite the Feature only supplying the 2.0 version which
> >>> appears
> >>> > > > > > > > incompatible with this. Maybe thats whats tripping Art's
> >>> usage up
> >>> > > > > > > > since he was clearly using <= 5.16.2 before?
> >>> > > > > > > >
> >>> > > > > > > > On Tue, 21 Jun 2022 at 09:24, Jean-Baptiste Onofré <
> >>> > > j...@nanthrax.net> wrote:
> >>> > > > > > > > >
> >>> > > > > > > > > By the way, you can see in activemq-client:
> >>> > > > > > > > >
> >>> > > > > > > > >     javax.jms;version="[1.1,3)",
> >>> > > > > > > > >
> >>> > > > > > > > > So:
> >>> > > > > > > > > 1. if your application uses the same range, it works
> >>> > > > > > > > > 2. if your application use [1.1,2), than, simple add
> >>> javax.jms
> >>> > > > > > > > > (geronimo) 1.1 bundle
> >>> > > > > > > > >
> >>> > > > > > > > > Regards
> >>> > > > > > > > > JB
> >>> > > > > > > > >
> >>> > > > > > > > > On Mon, Jun 20, 2022 at 7:45 PM Arthur Naseef <
> >>> a...@amlinv.com>
> >>> > > wrote:
> >>> > > > > > > > > >
> >>> > > > > > > > > > I created the following ticket to address
> applications
> >>> > > failing to load into
> >>> > > > > > > > > > Karaf with AMQ 5.16.3 - 5.17.1 due to an incompatible
> >>> change
> >>> > > in the
> >>> > > > > > > > > > activemq-client feature.
> >>> > > > > > > > > >
> >>> > > > > > > > > > https://issues.apache.org/jira/browse/AMQ-8971
> >>> > > > > > > > > >
> >>> > > > > > > > > >
> >>> > > > > > > > > > Looks to me like the right fix here is to revert the
> >>> change
> >>> > > to the JMS 1.1
> >>> > > > > > > > > > spec in the feature because all of the AMQ internals
> >>> are
> >>> > > still 100% on the
> >>> > > > > > > > > > JMS 1.1 spec.  The maven-bundle-plugin for client
> >>> > > applications is doing the
> >>> > > > > > > > > > right thing by generating "Package-Import" lines with
> >>> > > version range
> >>> > > > > > > > > > "1.1,2.0)", but the feature doesn't match it.
> >>> > > > > > > > > >
> >>> > > > > > > > > > It seems we have sacrificed the core case to solve an
> >>> edge
> >>> > > case.
> >>> > > > > > > > > >
> >>> > > > > > > > > > Thoughts?
> >>> > > > > > > > > >
> >>> > > > > > > > > > Art
> >>> > >
> >>>
> >>
>

Reply via email to