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 / bnd: Remove the self-Import of gson.annotations #2735

Conversation

chrisrueger
Copy link
Contributor

@chrisrueger chrisrueger commented Sep 3, 2024

Closes #2725

Purpose

Fixes a runtime issue in OSGi environments using multiple versions of GSON.

Description

Based on bndtools/bnd#6220 (comment) append -noimport:=true to all exports which we do not want to re-import.
This will remove the package from the Import-Package instruction in the resulting MANIFEST.MF

This avoids issues in OSGi environments where multiple versions of gson are deployed and required by different consumers and com.google.gson.annotations causes wiring conflicts between these different gson versions.

For further information about the -noimport check the bnd manual.

Checklist

  • New code follows the Google Java Style Guide
    This is automatically checked by mvn verify, but can also be checked on its own using mvn spotless:check.
    Style violations can be fixed using mvn spotless:apply; this can be done in a separate commit to verify that it did not cause undesired changes.
  • If necessary, new public API validates arguments, for example rejects null
  • New public API has Javadoc
    • Javadoc uses @since $next-version$
      ($next-version$ is a special placeholder which is automatically replaced during release)
  • If necessary, new unit tests have been added
    • Assertions in unit tests use Truth, see existing tests
    • No JUnit 3 features are used (such as extending class TestCase)
    • If this pull request fixes a bug, a new test was added for a situation which failed previously and is now fixed
  • mvn clean verify javadoc:jar passes without errors

Mentioning @Marcono1234 since he was involved in the discussions.

based on google#2725 and bndtools/bnd#6220 (comment)
append -noimport:=true to all exports which we do not want to re-import
This avoids issues in OSGi environments where multiple versions of gson are deployed and com.google.gson.annotations causes wiring conflicts between these different versions
Copy link
Collaborator

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I am still not completely sure what the 'correct' approach is (see the discussions you linked), but it is probably worth a try.

@eamonnmcmanus, what do you think?

@chrisrueger
Copy link
Contributor Author

chrisrueger commented Sep 4, 2024

what the 'correct' approach is

One statement I discovered, which makes me think -noimport is the right thing to do here is this:

Source: https://bugs.eclipse.org/bugs/show_bug.cgi?id=183595#c7

As a best practice I would recommend that library bundles do not import the packages they export. Bundles should only import a package if they are willing to get that package from an unknown source and omit their own export from the framework.

I think Gson qualifies as a library bundle (as opposed to an OSGi based fully fledged application).

I think the important statement is this:

if they are willing to get that package from an unknown source and omit their own export

This could be read e.g. like "I am fine that the package com.google.gson.annotations from gson 2.10 could also be provided by gson 2.11."

  • I think this is almost never the case for libraries (because libs never know in which context they are used. A library cares only for their own packages (I don't mean the 3rd party dependencies the lib might need itself))
  • especially when the library authors itself do not have OSGi knowledge, and "just" want to provide OSGi meta-data (it is often the case that those libraries are not developed with OSGi in mind, but need to provide the meta-data so that it can be consumed by OSGi apps)

This is different to OSGi based fully fledged applications (like Eclipse, AEM, Liferay).
Those applications consist of multiple OSGi bundles making up the whole application. In such "application" scenarios, special tricks like "package substitution" different bundles providing the same package (see #2725) could be desired. But applications are usually a different "beast" and much bigger than libraries.

I will try to discuss this further with the bnd / OSGi developers, and maybe the result could be a recommendation / documentation improvement for this specific problem.

@eamonnmcmanus
Copy link
Member

Thanks, I really don't know anything about OSGi but I'm happy to go with whatever you decide after talking to the OSGi experts.

@chrisrueger
Copy link
Contributor Author

@eamonnmcmanus thanks for your reply . @bjhargrave has approved this PR and he is THE OSGi expert and longtime bndtools committer.

Copy link
Member

@eamonnmcmanus eamonnmcmanus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for doing all this!

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

Successfully merging this pull request may close these issues.

Failure to simultaneously resolve two consumers using each of v2.10.1 and v2.11.0 together in Eclipse IDE
4 participants