bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
531 stars 305 forks source link

Baselining: Adding an element to an annotation type with no default value is an imcompatible change #1292

Closed kwin closed 8 years ago

kwin commented 8 years ago

Currently if one is adding an element to an annotation type the change is only considered to require a minor version upgrade (independent if it is provider or consumer type). The rule should rather be that it requires a major version upgrade for consumer types and minor only for provider type in case the element has no default value. In case it has a default value I would consider that a compatible change for consumers i.e. only upgrade of the minor version required.

bjhargrave commented 8 years ago

BJ to investigate.

bjhargrave commented 8 years ago

I am not sure what you mean by the distinction of consumer type and provider type for annotations.

If one is applies annotations to source code, then adding an element with no default value is a source compatibility breakage. But I am not sure there is any binary compatibility issue.

If one uses an annotation like an interface (as you can now do with DS 1.3 for component property types), then adding a new element had no source code or binary incompatibilities. SCR must proxy an instance of the annotation, but it must proxy the actual annotation type visible at runtime so SCR would not know about the addition of a new element.

Can you provide a more complete example of the problem your are thinking of?

kwin commented 8 years ago

Actually I am not sure about the distinction of consumer and provider type for an annotation myself. But for sure using an annotation to annotate a class/method/field is similar to implementing an interface while just using the annotation at run time through reflection is similar to just using an interface.

What I have in mind is really an annotation with a run time retention which is used at run time through some reflection methods.

The concrete example is Sling Models (https://sling.apache.org/documentation/bundles/models.html), which defines a @Model annotation. This annotation is used by all consumers of that API to annotate certain classes. At run time the annotation is evaluated by the provider of this API.

If you now add a new primitive element without a default type, this will require only a minor version upgrade according to baselining. Unfortunately that version still fits in the default import range of the consumer bundle, although the already defined annotations within that bundle don't set a value for this (required) element. If you try to evaluate now the value of this element in your provider bundle you run into an exception (because no value has been provided for that primitive in the client).

Although adding an element is a binary compatible change (https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.5.7) it is definitely not source compatible.

bjhargrave commented 8 years ago

At run time the annotation is evaluated by the provider of this API.

So the "provider" is sensitive to minor version increments. It needs to import the package of the annotation, org.apache.sling.models.annotations, at the provider range [1.2,1.3). Then when a new element is added to the Model annotation and the package version is incremented to 1.3, the old provider will not resolve against the new package.

Providers are always sensitive to minor increments since, as the provider of the API, they must understand all parts of the API. Consumer only need to understand the parts of the API they use.

Although adding an element is a binary compatible change (https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.5.7) it is definitely not source compatible.

OSGi Semantic Versioning is about binary compatibility. Source compatibility is a separate issue.

kwin commented 8 years ago

Actually what I am saying is the opposite. The provider was adjusted and is working fine, but the consumer bundle is still resolvable against the API although it hasn't defined the necessary element in its usage of the model annotation. What I am saying is, that in this case the consumer bundles should no longer be resolvable, because it obviously needs the change and otherwise it will lead to an exception when being consumed by the provider bundle.

bjhargrave commented 8 years ago

I don't see how the consumer should not be resolved here. It does not use the new element added to the annotation. The provider should be prepared for old consumers. If the provider takes an error, then the provider needs a bug fix since it can easily handle an old consumer.

kwin commented 8 years ago

Let me summarize:

  1. You change the API to add an additional element to the annotation being used by consumers (they annotate classes with that annotation) and by providers (they evaluate the annotation at at run time)
  2. The provider is adjusted to use the new element at runtime
  3. The consumer bundle stays as is and can still be resolved because even the increased API package version is considered compatible!

I would rather argue that in this case the consumer is no longer compatible with the new API and should not resolve! It would be the same for interfaces, where an addition of a method to an interface which is implemented by consumer bundles should break compatibility with old consumer bundles.

bjhargrave commented 8 years ago

This is how it works for interfaces.

1) You add new method to a @ProviderType interface. Package version increments from 1.0 to 1.1. 2) Provider is changed to implement new method in interface. Changes import range from [1.0,1.1) to [1.1,1.2). 3) Consumer does not change since it does not call new method. Import range remains at [1.0,2.0).

So consumer remains compatible with new 1.1 version of package.

With annotations, a bundle which uses annotations is NOT a provider of the API. It is a consumer. A bundle which processes the annotations at runtime is the provider of the API and must handle old consumers who use older versions of the API. This means that annotations are all @ProviderType because annotations users never implement the annotation or use them directly at runtime.

We have a similar situation in bnd which processes DS and Metatype annotations. Just because bnd supports the very latest version of the annotations should not mean it cannot build bundles using older (minor) versions of the annotations.

Basically we disagree whether an annotation is @ProviderType or @ConsumerType. I argue it is the former.