Dependency management improvements

[ppalaga]

Motivation: as we started depending on quarkus-cxf in Camel Quarkus, we'd very much like to rely on quarkus-cxf-bom. To be able to do so, the BOM needs to

• Cover all necessary artifacts (from both CXF and Quarkus CXF) and
• Be minimal (not bring anything that Quarkus CXF extensions do not require).

I hope this is fine for you?
I am open to discuss adding back or removing specific BOM entries.

[shumonsharif]

@ppalaga This looks good to me, I believe the failed build is something minor that needs to be addressed, though I did not dig in there yet.

@famod Given you put together the initial version of quarkus-cxf-bom, are you able to review this as well?

[ppalaga]

the failed build is something minor

Checking

[shumonsharif]

@ppalaga Thanks for the additional commits. One minor comment I had - do we need all these banned dependencies, I'm guessing there could be some potential cleanup from this list?
https://github.com/quarkiverse/quarkus-cxf/pull/487/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R193-R289

Let's give a couple days for Falko to review -- otherwise I will push it through and get you a release over the weekend; hope that works for you.

[ppalaga]

do we need all these banned dependencies, I'm guessing there could be some potential cleanup from this list?

We certainly don't but OTOH, it does not matter much if there is more than needed at the moment. It can happen that we add a dependency in the future which transitively pulls some of those. For such situations, it is perhaps better to have them all. In the long term, I'd like to replace the static copy with a config file produced by Quatkus - see quarkusio/quarkus#24880

Let's give a couple days for Falko to review

I am big proponent of reviews and I'll gladly wait for @famod to review!

otherwise I will push it through and get you a release over the weekend; hope that works for you.

I'd personally prefer getting a release right after Quarkus 2.12.0.CR1, planned for August 17th
I'd like to review the existing tests and code till then, possibly proposing some improvements.
Would that work for you?

[shumonsharif]

I'd personally prefer getting a release right after Quarkus 2.12.0.CR1, planned for August 17th I'd like to review the existing tests and code till then, possibly proposing some improvements. Would that work for you?

Yes, absolutely, that works. I'm really glad to see the PRs coming in - thank you for your efforts! JFYI, other than @famod, none of the maintainers, including myself, have used this extension in a production deployment - it's just been a side project for us. So there's much to be done to clean up the extension, add unit and integration tests, etc. So thanks in advance, and we heartily welcome the contributions.

[famod]

Looks good overall, just some smaller findings.

[famod]

I have noticed the discussion about moving to ExternalRules eventually, but it seems we lost the exclusion for org.ow2.asm:asm?

[ppalaga]

org.ow2.asm:asm some tests were failing for me when I excluded it for all modules. IIRC, originally it was excluded only for some specific module. Do you know of any specific reason why org.ow2.asm:asm should be banned?

[famod]

It was excluded due to #226 (and banned in a later PR).

The runtime part of quarkus-cxf should not depend on asm.

[ppalaga]

@famod addressed all your comments except for the org.ow2.asm:asm ban, where I am waiting for feedback

[ppalaga]

As for org.ow2.asm:asm, when banned, it would have to be excluded from the follwing BOM entries:

[ERROR]     io.quarkiverse.cxf:quarkus-cxf:1.5.0-SNAPSHOT:jar pulls banned dependencies [org.ow2.asm:asm]
[ERROR]     io.quarkiverse.cxf:quarkus-cxf-rt-features-logging:1.5.0-SNAPSHOT:jar pulls banned dependencies [org.ow2.asm:asm]
[ERROR]     io.quarkiverse.cxf:quarkus-cxf-rt-features-metrics:1.5.0-SNAPSHOT:jar pulls banned dependencies [org.ow2.asm:asm]
[ERROR]     io.quarkiverse.cxf:quarkus-cxf-rt-transports-http-hc5:1.5.0-SNAPSHOT:jar pulls banned dependencies [org.ow2.asm:asm]
[ERROR]     org.apache.cxf:cxf-rt-ws-security:3.5.3:jar pulls banned dependencies [org.ow2.asm:asm]
[ERROR]     org.apache.cxf:cxf-rt-ws-mex:3.5.3:jar pulls banned dependencies [org.ow2.asm:asm]
[ERROR]     io.quarkiverse.cxf:quarkus-cxf-rt-ws-security:1.5.0-SNAPSHOT:jar pulls banned dependencies [org.ow2.asm:asm]

I am no CXF expert, but at the first sight, this does not look like it can safely be excluded from them all?

[ppalaga]

I have filed a followup #493 for the org.ow2.asm:asm ban to unblock this PR.

So this is good to merge, from my point of view.

[ppalaga]

Could please this one be merged so that I have a stable base for #488 ?

[famod]

I have no permissions to merge.

I'd like to point out that there is still something to clarify w.r.t netty server: #488 (comment)
(and the ASM bit)
Though, this can be improved in a subsequent PR.

[ppalaga]

I'd like to point out that there is still something to clarify w.r.t netty server: #488 (comment)

Filed #496

(and the ASM bit)

Has an issue not to forget and to discuss #493

@gastaldi has now granted me permissions to merge (thanks!). So I hope it is OK to merge this one.

[gastaldi]

@gastaldi has now granted me permissions to merge (thanks!). So I hope it is OK to merge this one

Technically it was @shumonsharif who did this by approving it, I just created the PR :)

[ppalaga]

@gastaldi has now granted me permissions to merge (thanks!). So I hope it is OK to merge this one

Technically it was @shumonsharif who did this by approving it, I just created the PR :)

Thanks @shumonsharif then!

[shumonsharif]

Sure thing @ppalaga please feel free to merge!

[shumonsharif]

I have no permissions to merge.

@famod We'll get this corrected, will submit a PR later tonight!