If a package imported is only containing annotation, make the import optional by default

[laeubi]

Based on this issue here:

• Only import errorprone.annotations with optional (if at all) google/gson#2794

there was a question if/what "we" can improve.

One idea that came into my mind is, that probably bndlib can detect if a package

• is only referenced by annotations
• they are not used anywhere directly (e.g. in a method signature or as a field or parameter type)
• and nothing is configured for that package,

it imports this as optional by default.

The reason for this is, that even if the annotation is RUNTIME retention, the JVM will not fail if it can't be loaded (you then just cant read them by reflection).

FYI @chrisrueger

[timothyjward]

I don’t think that this is a good idea. This would very much break things like Jackson and Jakarta REST, both of which support pure POJO programming using only annotations.

[laeubi]

@timothyjward can you explain how it would break them?

[chrisrueger]

@timothyjward can you explain how it would break them?

I would also be interested in. Are you saying that Jackson itself would break, or other consuming apps using Jackson?

The GSON example @laeubi refers to it is specifically this case:

• gson has applied the annotation@com.google.errorprone.annotations.CheckReturnValue from another static code analysis library "errorprone" on one of their packages https://github.com/google/gson/pull/2693/files#diff-1409b0a5be9a24a5e8473b18e76329cc6907b5157f45b8d9ad48a83d6b273955R23-R24
• because annotation @CheckReturnValue is @Retention(RUNTIME) bnd (as part of gson build) recognizes it as a runtime dependency and adds it to Import-Package in the MANIFEST
• the party which had a problem with this, was Eclipse Dependency Updates eclipse-platform/eclipse.platform.releng.aggregator#2794 because they now also have to pull in com.google.errorprone in their dependencies (Missing requirement: com.google.gson 2.12.0 requires 'java.package; com.google.errorprone.annotations [2.36.0,3.0.0)' but it could not be found)

There was the question from gson team "if there is anything" "we" could do. I can understand that question. Of course it is part of the age old problem that "we OSGi people would like to have proper OSGi meta data, but lib maintainers out there don't know much about OSGi and don't know how to please us or identify that things like above can be problematic for OSGi applications".

So this ticket is an attempt to brainstorm, if there maybe is something bnd (as a helpful tool to build good OSGi bundles using various heuristics) could do about that.

• like identifying an exceptional case where it would be feasible to produce more relaxed meta data
• or output a warning in the build , so that at least something could be noticed

[pkriens]

There is a clear mechanism to handle this case wit the retention annotation. If someone said RUNTIME, who are we to say it is not a runtime dependency? These kind of changes are bound to bite you in the ass in other situations and tend to create more and more complexity.

I am, however, most puzzled why Eclipse has a problem with a small extra dependency?

[timothyjward]

@timothyjward can you explain how it would break them?

The following class would not work without annotations, all of which meet the rules you laid out for optionality.

@Path("hello")
public class MyRestClass {
    @GET 
    @Produces("text/plain")
    public String hello() {
        return "Hello World";
    }
}

Jackson annotated types are similar. They are POJOs but the mapping annotations provide significant behaviour when moving to/from JSON.

[chrisrueger]

If someone said RUNTIME, who are we to say it is not a runtime dependency?

yes, good point.

[laeubi]

The following class would not work without annotations, all of which meet the rules you laid out for optionality.

Why would it not work? It will perfectly load with and work. Just because the import is optional do not mean the information is thrown away.

[timothyjward]

The following class would not work without annotations, all of which meet the rules you laid out for optionality.

Why would it not work? It will perfectly load with and work. Just because the import is optional do not mean the information is thrown away.

The REST container would introspect the class and find that it had no annotations. As a result no endpoint would be created and the user would get a 404 instead of “Hello World”.

For Jackson the generated JSON would be “wrong” without access to the annotations. For example it might have the wrong structure, or use the wrong property names.

[chrisrueger]

The REST container would introspect the class and find that it had no annotations.

I don't understand that.
Are we all talking about the same thing?

When @laeubi said in the initial post

it imports this as optional by default.

it means bnd would create an Import-Package: jackson;version=xxx;resolution:optional; instead of Import-Package: jackson;version=xxxx (I simplified GAV and version) on your own application's bundle MANIFEST (Jackson is untouched here)

[laeubi]

The rest container will require the annotation so it will be in the runtime there and then wired to the import of the provider as well. If there is no rest container no one will ever look at the annotation and then there is no problem :-)

Same for Jackson of course...

[timothyjward]

it means bnd would create an Import-Package: jackson;version=xxx;resolution:optional; instead of Import-Package: jackson;version=xxxx (I simplified GAV and version) on your own application's bundle MANIFEST (Jackson is untouched here)

The rest container will require the annotation so it will be in the runtime there and then wired to the import of the provider as well. If there is no rest container no one will ever look at the annotation and then there is no problem :-)

This shows a misunderstanding of OSGi class spaces. If my code has an optional import for the annotation package then it may or may not be wired at runtime. If it isn’t then my classes will be loaded without any of those annotations.

Let’s assume that we are in a situation where the wire isn’t created, and so as a result my types have no runtime visible annotations present on them.

If I later pass an instance of one of my classes to a library/framework that wants to introspect the type of my object to find the annotations then they will still not be there. It doesn’t matter that the library/framework has access to the annotations, my bundle doesn’t and so the annotations will not appear on the type.

The end result is that my Jakarta REST resource will not work as a resource, or my Jackson mappable DTO will map incorrectly because the mapping metadata is missing.

For a real example of how this might happen, some Jackson annotated POJOs exist in the core API of the sensinact codebase. If the Jackson annotations are imported optionally then this API bundle could be installed and resolved without Jackson. Later someone might add the REST feature to their running gateway, which would add Jackson and use it for JSON support, however the API would be missing the necessary metadata and so those types would end up with the wrong serialization.

[laeubi]

This shows a misunderstanding of OSGi class spaces. If my code has an optional import for the annotation package then it may or may not be wired at runtime. If it isn’t then my classes will be loaded without any of those annotations.

I agree that it could happen when something is installed later, but in such case one should refresh the packages of related bundles anyways.

And I would assume that any such thing would want an require capability on the extender so the extender must be there before installing the "Jakarta REST resource" or at least it has an additional contract capability for such packages so the actual import-package is not the only requirement that will pull something in.

In any case if someone is concerned, I explicitly added

and nothing is configured for that package

so my request is really for that cases where we have nothing specified as seen for most library bundles. It is not perfect but feels like a much better thing to do as for "OSGi aware" bundles/setup there will always be some other requirements that make the package available anyways.

I just noticed jspecify also uses RUNTIME what will become another annoyance, so to answer Peters question

I am, however, most puzzled why Eclipse has a problem with a small extra dependency?

It is not that "Eclipse" has a problem, it is that there is a large fan-out for such things, so each dependency brings another "small" dependency and that has "just one one" and so on and soon you pull in a lot of things that are actually of no use and can cause issues (CVEs, conflicting imports, bad maintained OSGi metadata, ...)

So I think the same arguments apply here why we have conditionalpackage in bnd.

[timothyjward]

I agree that it could happen when something is installed later, but in such case one should refresh the packages of related bundles anyways.

This still doesn’t guarantee that the optional package will be wired.

so my request is really for that cases where we have nothing specified as seen for most library bundles. It is not perfect but feels like a much better thing to do as for "OSGi aware" bundles/setup there will always be some other requirements that make the package available anyways.

It is always the wrong thing to do to misrepresent dependencies. Setting the default in this way leaves most uses of runtime annotations open to failure, in an effort to avoid some annoyance in places where annotation providers have done unpleasant things.

A better answer would be to meta annotate these annotations so that bnd can know to safely make them optional. This would be a class retention annotation which can be scoped so as not to be included in dependency fan-out. It could be set at a type level or package level.

[laeubi]

This still doesn’t guarantee that the optional package will be wired.

I can only find some parts in the spec that mentions if a requirement is not found and optional that then it is not a failure but I can't find any section that allows the OSGi framework to (e.g. randomly) choose not to wire an existing import provider.

A better answer would be to meta annotate these annotations so that bnd can know to safely make them optional. This would be a class retention annotation which can be scoped so as not to be included in dependency fan-out. It could be set at a type level or package level.

Its already hard to convince people to include OSGi manifest information that do not show up in the code, asking them to annotate their code with "something" is almost hopeless I think.

As there seem to be no good way around this I'll just close this, so the only option currently seem to watch out and hope maintainers getting convinced to adjust their metadata after the fact.

[timothyjward]

Its already hard to convince people to include OSGi manifest information that do not show up in the code, asking them to annotate their code with "something" is almost hopeless I think.

These would be meta-annotations on the errorprone annotation types, not where they are used, so we wouldn’t have many people to convince.

[laeubi]

These would be meta-annotations on the errorprone annotation types, not where they are used, so we wouldn’t have many people to convince.

Given that this "not many" people wanted to throw away OSGi metadata altogether, I think it must then be something officially supported by java or similar.