Exclude the old annotation and cdi api dependencies

[sberyozkin]

Fixes #166

[rdebusscher]

These dependencies have the scope runtime. So it is possible that the TCK runners of the vendors no longer work due to this exclusion.

So I would not merge this change as it has unknown effects.

[sberyozkin]

@rdebusscher I'm not sure it may have side-effects as we have Jakarta dependencies offering the equivalent and more ? At the very least we should be able to pick up the uptodate DenyAll/etc annotation types, otherwise the legacy annotation types are interfering as in this case. Would it work if I update PR to exclude javax.annotation only ?

[sberyozkin]

I'll double check as well on our end, please try running the TCK with these exclusions for your own implementation, if it works in at least 2 cases then it will be pretty safe to exclude :-)

[radcortez]

TCK or Runtimes shouldn't rely on getting these dependencies transitively from Shrinkwrap, so I think its fine to exclude then.

Anyway, I'll run the TCK in SmallRye with these change to see how it looks like.

[sberyozkin]

@radcortez I agree.
@rdebusscher I'm pretty sure we can go ahead with this PR but I won't rush and keep it open for a few days for you and other MP JWT colleagues to make sure it is safe

[rdebusscher]

@sberyozkin I verified and the CDI dependency is today ignored because the CDI v 2.0 is also available at runtime. So the PR will not break our TCK.

But I do not agree with the reason why the PR is created, to circumvent a BUG in Eclipse IDE.

So I will not approve the PR.

[radcortez]

Is this because of Eclipse? I thought we wanted to get rid of javax.* dependencies.

[sberyozkin]

@rdebusscher Why is it an Eclipse bug or rather, what is it to do with Eclipse in the end of the day ? Using Eclipse helped to identify the problem which is: we have the legacy APIs leaking into the TCK set of dependencies which is a problem regardless of what IDE is used or if IntelliJ or whatever else is smarter in the way it orders the dependencies.

As per the above discussion, TCK or anything in MP JWT must not depend on anything coming from the non Jakarta dependencies. I'm sorry but I'd like a stronger reason to stop this PR from going in. Provide it to me please.

CC @dblevins, @chunlongliang-ibm, others - any concerns on your end - please let me know

[sberyozkin]

@rdebusscher By the way, thanks for testing it with your implementation. I'd like to hear from others too.
IMHO there is no reason to be concerned about this update, it is a small clean up which will make MP JWT a tiny bit neater, without any agenda :-).

[rdebusscher]

@sberyozkin

Why is it an Eclipse bug or rather, what is it to do with Eclipse in the end of the day ? Using Eclipse helped to identify the problem which is:

Eclipse IDE should not consider runtime scoped dependencies within development.

And by excluding those dependencies from shrinkwrap-resolver-depchain it is possible that shrinkwrap resolver isn't working anymore (because it doesn't have the required dependencies anymore when running the TCK)

So if you want to get rid of the old dependencies, do an upgrade of the shrinkwrap resolver to a newer version which contains the correct dependencies.

[sberyozkin]

And by excluding those dependencies from shrinkwrap-resolver-depchain it is possible that shrinkwrap resolver isn't working anymore (because it doesn't have the required dependencies anymore when running the TCK)

I'm sorry, instead of theorizing, given that yourself, and myself too, have confirmed it is not the case, I prefer dealing with the concrete technical objections.

MP JWT, across the board, now uses Jakarta dependencies - these dependencies must, due to their guarantee of the backward compatibility, support anything that any other dependency in MP JWT needs that is now supported by Jakarta dependencies - if it is not the case, then something would be broken in Jakarta dependencies.

So if you want to get rid of the old dependencies, do an upgrade of the shrinkwrap resolver to a newer version which contains the correct dependencies.

Really ? Jakarta dependencies are here in MP JWT poms

[rdebusscher]

I'm sorry, instead of theorizing, given that yourself, and myself too, have confirmed it is not the case, I prefer dealing with the concrete technical objections.

Because we have MP Rest Client in our TCK runner. But does everyone have this? In fact, our engineers are against excluding this, although it will not break our TCK tests.

Exclusion of transitive dependency is very risky business.

But go ahead if you want to merge this in.

[sberyozkin]

@rdebusscher That is fine, thanks for this update. Lets revisit this issue in a few weeks just before 1.2 and make a final decision, cheers

[sberyozkin]

Hi All, I'm marking it for 2.0.
@chunlongliang-ibm, @dblevins If you think it would be better to fix it for 1.2 then let me know please (myself, Roberto think 1.2 is OK but if someone has a concern then lets sort it out for 2.0)