search

smallrye / smallrye-common

Common utilities for SmallRye
Apache License 2.0
21 stars 24 forks source link

The release archive of smallrye-common-annotation has been modified to exclude jandex. #273

Closed taichi closed 9 months ago

taichi commented 9 months ago

This change was implemented because if a jandex.idx, not rebuilt during the build process, is unintentionally included in the UberJar, it may prevent the application from functioning correctly.

It should be noted that within this project, the only archive released that includes jandex.idx is annotations.

dmlloyd commented 9 months ago

/cc @cescoffier (re: #129)

cescoffier commented 9 months ago

Hum, we need the index for the annotations. Otherwise we would need to edit every Quarkus extension that may use these annotations to produce the index during the Quarkus build process. That would be a breaking change.

dmlloyd commented 9 months ago

Perhaps the correct fix would be to modify the Quarkus uberjar process (presumably that's what we're talking about here) to merge all indexes?

Ladicek commented 9 months ago

This change was implemented because if a jandex.idx, not rebuilt during the build process, is unintentionally included in the UberJar, it may prevent the application from functioning correctly.

I assume this is when building some uberjar that later becomes part of an application? Perhaps not even related to Quarkus? Here's some advice regarding that situation: https://smallrye.io/jandex/jandex/3.1.6/maven/shading.html

taichi commented 9 months ago

According to cescloffier, Jandex is necessary for the proper functioning of smallrye-common-annotation. Therefore, applications that include smallrye-common-annotation as part of UberJar currently need to rebuild Jandex entirely. I believe this includes applications that use Quarkus.

Please take a look at a simple test I made.

You can see that just having smallrye-common-annotation in the dependencies of an application running on resteasy-undertow-cdi causes the application to fail. Here is the link for your reference: https://github.com/taichi/resteasy-cdi/actions/runs/7463000910/job/20306655486

The same application operates correctly when smallrye-common-annotation is removed from its dependencies. You can see this here: https://github.com/taichi/resteasy-cdi/actions/runs/7463016295/job/20306707556

Additionally, the application starts working again when Jandex is rebuilt after adding smallrye-common-annotation to the same application's dependencies. This can be verified at: https://github.com/taichi/resteasy-cdi/actions/runs/7463059034/job/20306843138

dmlloyd commented 9 months ago

I would say then that the correct fix is to always rebuild the jandex index when building an uber-JAR which uses jandex.

We should be doing this in Quarkus already; if we are not, then the correct thing to do would be to open an issue in the Quarkus project to fix that bug.

For other usages, this might best be done as a documentation update to jandex itself.

Ladicek commented 9 months ago

According to cescloffier, Jandex is necessary for the proper functioning of smallrye-common-annotation.

This is correct in Quarkus. Other environments don't necessarily have to use Jandex, but when they do, they tend to trust an existing index if it's there. This becomes problematic in case of an uberjar. If you're creating an uberjar from multiple archives, you should either drop Jandex entirely (the target runtime will likely either fail or create its own index), or reindex entirely (as described in the Jandex documentation).