Remove OSGi annotations from the API #716
Comments
|
@rotty3000 this is the annotation you added for OSGi CDI. If I remembered correctly, this annotation should not cause any issues for non-OSGi environament. Apparently it caused problems for Quarkus. We might have to remove these annotations if this can't be resolved. |
|
Just a clarification, this has nothing to do with Quarkus. You can reproduce the warning just with the MP Config API jar and with compiler warnings turned on. |
|
Though this is not a runtime limitation in any way, I don't think I know a way to prevent the compiler warning based on how the annotation is designed (to use a enum as property value). I would ask @bjhargrave for advice on this. |
|
This is because the the osgi annotations jar is not a compile scope dependency of the eclipse microprofile config jar. Generally tools need to see annotation definitions to see if they may be meta-annotated with annotations the tool needs to process. Javadoc wants to look for Documented meta-annotations, Bnd wants to look for meta-annotations it must process. Javac is likely also wanting to do the same. Thus the warning to indicate there is an annotation whose definition cannot be examined. The solution is for the eclipse microprofile config jar to have a compile scope dependency on the osgi.annotation jar which would provide javac, javadoc, etc. which visibility to the annotation definitions. Based upon this feedback, we are making updates to the OSGi api jars about to be published for the Compendium Release 8 to make sure the maven pom metadata properly indicates the compile scope dependency on osgi.annotation. @Emily-Jiang I can prepare a PR for eclipse microprofile config for this change (compile scope dependency on osgi.annotation) if you want. |
|
Yes, I know that adding the OSGi dependency would fix the issue. On the other hand, I think that we need to be very careful about this decision. Yes, I know it should be harmless, but by principle, I believe that the MP APIs shouldn't push any additional dependencies. This creates a blurred boundary on what it is acceptable to add as metadata from external APIs and what is not. If other APIs would like to add their own metadata, should we add it as well and include their dependencies? |
|
This is not pushing any runtime dependencies. This is about preventing compiler warnings (which are harmless but we should support clean builds) via compile time dependencies. The OSGi annotations allow the artifacts to be OSGi-ready. A number of Microprofile stakeholders and users use OSGi as a module system and need the artifacts to be OSGi-ready. Without the OSGi annotations in the source code, the OSGi metadata would need to be manually maintained in the pom file (or external bnd file) and this would become out of date as developers forget or don't know to update the information. This is the value of annotations: the information is in the source code where the developers are working.
This is a slippery-slope argument and not very compelling. We have an actual need now for OSGi metadata in the artifacts. If other module systems (e.g. JPMS) need source code (e.g. module-info.java), I am sure Microprofile will do what is needed for the stakeholders and users of Microprofile. This sort of decision is best made when there are actual expressed needs rather than attempting to wave them away up front. |
|
I'm not at all familiar with the internals of MP Config, but have you ever considered shipping two flavours? One with and one without OSGi? |
|
Yes, please remove the OSGi annotations, any annotation that makes use of an While using annotations improves the maintenance of OSGi metadata, there is an issue when those annotations use enums, I honestly think that until those annotations remove the enum coupling, the OSGi metadata should be maintained in the pom file or externally. |
|
@rotty3000 please see the above comments. The use case is for OSGi specification to use MicroProfile Config. @jorsol I think it can be changed to |
|
I added the following snipped to my code to get rid of this annoying warning: package org.osgi.annotation.bundle;
/**
* Workaround for "warning: unknown enum constant Resolution.OPTIONAL" during compilation. This class is not used at runtime.
* See also https://github.com/eclipse/microprofile-config/issues/716
*/
@SuppressWarnings("unused")
public class Requirement {
public enum Resolution {
OPTIONAL
}
}While this works it is an ugly hack. My question is, if this dependency is not required then why is it referenced. Somewhere seems to be a false dependency scope or a class in the wrong dependency. Or the package that depends on |
|
The problem is those optional (or compile-only) annotations should NEVER use enums, so until the OSGi annotations are fixed and remove the enum dependency this will be a problem. Enums make the code more type-safe, but if the annotations are intended to be used optionally or just at compile-time (for generation of metadata or source code), then using enums is definitely a bad practice. |
|
@jorsol the dependency should be marked |
|
The problem is that the dependency should not be of scope I can just add the dependency (or if it's declared as <dependency>
<groupId>org.osgi</groupId>
<artifactId>osgi.annotation</artifactId>
<version>8.0.1</version>
</dependency>The result will be that a dependency ends up in the final application directory for no good reason. This is especially true for an annotation that the main purpose is to generate a bundle's manifest, there is no use of the dependency beyond that. |
|
FTR, I'm using following workaround in my Quarkus project (in root parent pom.xml): <!-- workaround for https://github.com/quarkusio/quarkus/issues/19970 -->
<dependency>
<groupId>org.osgi</groupId>
<artifactId>osgi.annotation</artifactId>
<version>7.0.0</version>
<scope>provided</scope>
</dependency>This way it at least doesn't end up in the production jar. |
Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See quarkusio/quarkus#19970 and eclipse/microprofile-config#716. To address this, we replace the enum classes with simple classes holding string constants and change the annotation elements returning the enum values to return string values. This is generally source compatible with usage of the OSGi annotations. Since these are CLASS retention annotations, they are not visible at runtime and so only tools, like Bnd, which process the annotations at tool time are affected. Bnd 6.2 is being taught to seamlessly handle old and new annotations using the old enum values or the new string values. So moving to using the updated OSGi annotations will also require moving to use Bnd 6.2 or later. Signed-off-by: BJ Hargrave <[email protected]>
|
FYI, I am working a fix to the OSGi annotations to replace use of enum types with String along with a corresponding fix to Bnd to support this change. |
Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See quarkusio/quarkus#19970 and eclipse/microprofile-config#716. To address this, we replace the enum classes with simple classes holding string constants and change the annotation elements returning the enum values to return string values. This is generally source compatible with usage of the OSGi annotations. Since these are CLASS retention annotations, they are not visible at runtime and so only tools, like Bnd, which process the annotations at tool time are affected. Bnd will seamlessly handle old and new annotations using the old enum values or the new string values. So moving to using the updated OSGi annotations will not require updating to use a newer version of Bnd. Bnd 6.2 is being updated to better handle these changes including validation of the string values since a string return type is open ended while an enum return type is not. Signed-off-by: BJ Hargrave <[email protected]>
Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See quarkusio/quarkus#19970 and eclipse/microprofile-config#716. To address this, we support the use of enum values or string values as the annotation element value. This will support the OSGi change in osgi/osgi#404 We seamlessly handle old and new annotations using the old enum values or the new string values. So moving to using the updated OSGi annotations will also require moving to use Bnd with this fix. Signed-off-by: BJ Hargrave <[email protected]>
Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See quarkusio/quarkus#19970 and eclipse/microprofile-config#716. We support the use of enum values or string values as the annotation element value through existing conversion support. The OSGi change in osgi/osgi#404 will move from using enum types to use string values which are case-insensitive equivalent to the enum value names. We seamlessly handle old and new annotations using the old enum values or the new string values. Prior to this fix, Bnd already handled the change through a fallback in Converter which uppercased the string value before converting to an internal enum value. This change avoids the internal enum type and processes the string value or string name of the enum value when processing older versions of the OSGi annotations. Signed-off-by: BJ Hargrave <[email protected]>
Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See quarkusio/quarkus#19970 and eclipse/microprofile-config#716. To address this, we replace the enum classes with simple classes holding string constants and change the annotation elements returning the enum values to return string values. This is generally source compatible with usage of the OSGi annotations. Since these are CLASS retention annotations, they are not visible at runtime and so only tools, like Bnd, which process the annotations at tool time are affected. Bnd will seamlessly handle old and new annotations using the old enum values or the new string values. So moving to using the updated OSGi annotations will not require updating to use a newer version of Bnd. Bnd 6.2 is being updated to better handle these changes including validation of the string values since a string return type is open ended while an enum return type is not. Signed-off-by: BJ Hargrave <[email protected]>
Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See quarkusio/quarkus#19970 and eclipse/microprofile-config#716. We support the use of enum values or string values as the annotation element value through existing conversion support. The OSGi change in osgi/osgi#404 will move from using enum values to use string values which are equivalent to the former enum value names. We seamlessly handle old and new annotations using the old enum values or the new string values. Prior to this fix, Bnd already handled the change through the Converter which converts the string value to an internal enum value. This change avoids the internal enum type and processes the string value or string name of the enum value when processing older versions of the OSGi annotations. Signed-off-by: BJ Hargrave <[email protected]>
The OSGi annotations added in #381, cause annoying compiler warnings.
The issue is that we mark the OSGi dependencies as provided and a reference to
org.osgi.annotation.bundle.Requirement.Resolution#OPTIONALis missing from the dependencies when compiling a project that uses@ConfigProperty.Adding the OSGi dependency as compile does fix the issue, but I believe an MP API should not transitively push this kind of dependency to their consumers (especially if they are not using OSGi).
This can be reproduced just by using the MP Config API JAR, use the
@ConfigPropertyannotation and compile code with warnings on:-Dmaven.compiler.showWarnings=true. It generates the following warning:This causes confusion to users, and we should remove this warning.
The text was updated successfully, but these errors were encountered: