Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSGi Export-Package versions don't honour semantic versioning preventing multiple minor versions coexisting #3017

Closed
paul-mcculloch opened this issue Jan 15, 2021 · 4 comments

Comments

@paul-mcculloch
Copy link

The MANIFEST.MF Export-Package section uses the version of the parent project for the version attribute of each package. For example, from jackson-databind-2.8.3.jar, we have:

Export-Package: com.fasterxml.jackson.databind;version="2.8.3"

And in jackson-databind-2.10.5

Export-Package: com.fasterxml.jackson.databind;version="2.10.5"

According to https://bnd.bndtools.org/chapters/170-versioning.html a minor version change of a package should be used when backward compatible changes are made.

In the case of these two versions there is a breaking change - the method com.fasterxml.jackson.databind.SerializationConfig.withSerializationInclusion() has been removed.

What this means is that a bundle built against 2.8.3 and using this method will fail when run in an OSGi environment where both 2.8.3 & 2.10.5 are installed. This is because the using bundle's manifest will declare an Import-Package such as

Import-Package: com.fasterxml.jackson.databind; version="[2.8, 3)"

Which indicates it requires a package >= 2.8 && <3

The package exported by 2.10.5 satisfies this dependency and will be used in preference to 2.8.3 (where both versions are available).

To give a real work example https://mvnrepository.com/artifact/io.hawt/hawtio-insight-log-osgi/1.5.0 imports 2.8.3 & https://mvnrepository.com/artifact/com.microsoft.azure/msal4j/1.7.1 imports 2.10.5. It is not possible to run both of these bundles in the same OSGi container as they both import databind with declared minor version differences but actual major version differences.

@paul-mcculloch paul-mcculloch added the to-evaluate Issue that has been received but not yet evaluated label Jan 15, 2021
@cowtowncoder
Copy link
Member

This is bit of a tricky situation since Jackson tries to follow SemVer for what is considered its user-facing public API (methods users would use to interact with it), but not so much for its internal interfaces, functionality intended only for packages themselves or other core components. Since Java does not really have tools to enforce such division (esp. before module system), it is possible to build things that will rely on internal APIs (especially so with sub-classing) and where minor-version compatibility does not apply or only extends across smaller number of minor versions.
Having said that, even with internal interfaces attempt is made to minimize number of incompatible changes; but in cases where those have to be made (usually in order to support new functionality), old methods are deprecated and the goal is to at least ensure "adjacent" minor version compatibility (so that code that only relies on non-deprecated methods / constructors should work with at least the next minor version).

At this point I will note that wrt API compatibility, only part of API of SerializationConfig users should use are accessors: construction of new instances with withSerializationInclusion() should be only used by databind package itself.
There should be ObjectMapper method to do the necessary change; neither users nor other libraries/frameworks should try to create new instances on their own.
Still, technically speaking, there is nothing to prevent them from doing that which results in possible issues.

Having said that, in practice the best way to try to minimize compatibility issues is for library / framework maintainers to work with Jackson team during Release Candidate phase of the new minor version to try to verify actual level of compatibility for actual usage by downstream dependencies. Sometimes accidental changes (removal of a protected constructor, without deprecation marker) can be reverted before .0 release. If not, we have sometimes fixed such cases in .1 patch release too (see #2990 for a recent example).

So. I do not think there is much we can fully solve the fundamental problem -- major version change is a very big undertaking, and while there will be one coming on medium-term, it will not be the mechanism to use for all changes, for practical reasons -- but there are practical things we can do to try to reduce likelihood of problematic changes, and to uncover specific issues.

In specific case of this problem, I suspect that the only way to address it would be to replace use of withSerializationInclusion() with code to configure ObjectMapper.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Jan 29, 2021
@cowtowncoder
Copy link
Member

I am not sure what specifically is being asked, closing; may be reopened with specific request or suggestion for changes. But I do not see anything specifically actionable here.

@paul-mcculloch
Copy link
Author

OSGi does provide a solution to the "Java does not really have tools to enforce such division" problem - only public classes and interfaces should be in a package which is in the Export-Package of the manifest. I agree that solving the problem retrospectively is hard.

From a Jackson 2.x perspective all that can be done to fix the problem is to re-instate the missing method so that older code with a dependency on 2.8 can continue to work with recent 2.x Jackson without changes.

Longer term I think the correct (from an OSGi perspective) solution would be to manually maintain the semantic version (rather than deriving it from the project version) for each package, and ensure that only public APIs are present in exported packages.

@cowtowncoder
Copy link
Member

@paul-mcculloch I am not against adding removed methods were possible (there are some removals that cannot be reversed), but I'd need those reported (and evaluated) on case by case basis, based on observed problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants