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

Add default version of export to selfimports #6238

Conversation

chrisrueger
Copy link
Contributor

@chrisrueger chrisrueger commented Aug 27, 2024

This is an thought-experiment / prove of concept / based on discussion for #6220.
Relates to #6220 (comment)

In this PR

  • after exports and imports have been processed in Builder.analyze()
  • we calculate self-imports
  • and then call augmentImports(selfImports, exports) to applyVersionPolicy() to any of those self-imports which
    a) do not yet have a version or
    b) specify a range-mask with the rang macro (e.g. Import-Package: example.c;version="${range;[==,=+)}")
  • -nodefaultversion true still works and could disable the behavior

Based on the observation in #6220 (comment) the following will happen for this bnd:

.bnd:

Bundle-Version: 1.0.1
-exportcontents: example.a, example.b, example.c
Import-Package: example.c;version="${range;[==,=+)}", *

MANIFEST.MF:

Bundle-Version: 1.0.1
Export-Package: example.a;uses:="example.b";version="1.0.1",example.b;version="1.0.1",example.c;version="1.0.1"
Import-Package: example.b;version="[1.0,2)",java.lang,example.c;version="[1.0,1.1)"

instead of as it was before:

Export-Package: example.a;uses:="example.b";version="1.0.1",example.b;version="1.0.1",example.c;version="1.0.1"
Import-Package: example.b,java.lang,example.c;version="${range;[==,=+)}"

Why or When would that be useful?

As outlined in #6220 3rd-party depdencies like Gson etc. use bnd to create OSGi bundles via the maven or gradle plugins.
But often there is no OSGi-knowledge present hence there are often no version information at the package-level like:

  • packageinfo file in the package directory (recommended)
  • package-info.java, bnd’s Version annotation
  • Manifest of the exporting JAR

Only a Bundle-Version is usually mainted.

For all those dependencies out there the previous behavior already added the Bundle-Version as the last resort to Exports, while self-imports would not get any version information.

But now the self-imports would get the version information from the export even it is the "last resort" Bundle-Version

Open Questions

  • what about if this be the (new) default behavior?
  • does it even make sense?
  • what could go wrong if this would be the new default? What could it break?
  • or should there be a new option / instruction to enable this behavior?

Feedback appreciated @pkriens @bjhargrave

@chrisrueger chrisrueger force-pushed the add-version-of-export-to-selfimport branch from cda6565 to c5e66ab Compare August 27, 2024 11:15
@chrisrueger chrisrueger changed the title Add version of export to selfimports Add default version of export to selfimports Aug 27, 2024
reuse augmentImports() method to add version information from exports to self-imports, after exports have been processed

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger chrisrueger force-pushed the add-version-of-export-to-selfimport branch from c5e66ab to 0999acd Compare August 27, 2024 12:16
@bjhargrave
Copy link
Member

bjhargrave commented Aug 27, 2024

This is a bad idea. Because export defaults to using Bundle-Version does not mean it is a valid choice on the import side. There is probably no other bundle which exports the package at the Bundle-Version of the importer. So matching the import at the Bundle-Version will be pointless. Bundle-Version is not semantic and thus not suitable for use on imports.

Exporters need to use proper versions. Importing at the Bundle-Version is a bad idea. So I would reject this proposed change.

@chrisrueger
Copy link
Contributor Author

Thanks @bjhargrave for the feedback.

I am just wondering if there is any other way out of the situation described in eclipse-jdtls/eclipse.jdt.ls#3236 (comment) other than manually fixing in upstream (gson).

The problem, as you've noted, is that both gson bundles import just "com.google.gson.annotations" (implied >= 0.0.0) leaving the door open for resolution between different versions of the same library 😐 gson 2.10.1 should import with a range of [2.10.1,2.11.0) and gson 2.11.0 should import with a range of [2.11.0,2.12.0) to be safe.

com.google.gson_2.10.1.jar
Import-Package = sun.misc;resolution:=optional,com.google.gson.annotations;version="[2.10.1,2.11.0)"

com.google.gson_2.11.0.jar
Import-Package = sun.misc;resolution:=optional,com.google.gson.annotations;version="[2.11.0,2.12.0)"

My interpretation is that gson "expects" that its own com.google.gson.annotations is imported, but apparently another bundle of the same library got wired.

@bjhargrave
Copy link
Member

The solution is always to fix the upstream :-)

In the alternate, if the bundle must not import the package from another bundle, then its bnd instructions should decorate the export with -noimport:=true. Then it won't self-import and will just export. When the bundle goes to access the package, it will always use its own copy since it can never import the package.

@chrisrueger
Copy link
Contributor Author

Closing. Continue at comment #6220 (comment)

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.

2 participants