[bnd-maven-plugin] Generated substitution import does not define version range

[Marcono1234]

Version: Maven plugin biz.aQute.bnd:bnd-maven-plugin 6.4.0

Hello,
https://github.com/google/gson uses the bnd-maven-plugin to generate the OSGi manifest attributes¹. Recently in google/gson#2725 it was reported that the "substitution" Import-Package for com.google.gson.annotations causes issues when multiple third-party bundles import Gson packages.

In that Gson issue and the linked discussions it was mentioned that the problem might be that Gson's own Import-Package (generated by bnd) does not define a version range for com.google.gson.annotations²:

Import-Package: sun.misc;resolution:=optional,com.google.gson.annotations

Do you think as well that this is the cause here, and that the solution is to use ...;version="${range;[==,=+)}" or similar? But in that case is that something bnd can / should generate on its own?

Disclaimer: I am not very familiar with OSGi, so please let me know if there is any misunderstanding.

Footnotes

1. Maven plugin configuration: https://github.com/google/gson/blob/9615c8d7c895797872d7e601702e690e5ed7a2e8/gson/pom.xml#L125-L157 ↩

2. Can for example be seen in the Gson 2.11.0 JAR, or if you build Gson manually with mvn package; the JAR is then under /gson/target/gson-2.11.1-SNAPSHOT.jar. ↩

[pkriens]

I expect that there is no version available for com.google.gson.annotations when gson is processed? I.e. its JAR contains no version information.

It must then explicitly set in the Import-Package, i.e. don't use a ${range}, just give the version. Best is of course to provide a version for com.google.gson.annotations package in its JAR..

[Marcono1234]

There might be a misunderstanding here, com.google.gson.annotations is part of the regular Gson JAR. So bnd generates both an Export-Package and an Import-Package entry for it¹. And the Export-Package does include the version, but the Import-Package does not include any version range. Sorry if I did not make that clear enough.

Therefore I think the version information should be present to bnd, at least in theory. But maybe it is not accessible to that part of bnd for whatever reason? It seems there is some analysis on this in #6221 (comment).

Footnotes

1. That is what I meant with "substitution import". If I understand it correctly that usage of Import-Package for an exported package is for "substitution" as defined in OSGi Core 8 §3.6.6. ↩

[chrisrueger]

And the Export-Package does include the version, but the Import-Package does not include any version range.

I checked the MANIFEST.MF of gson 2.10.1 as an example:

I guess the version behind the Export-Package are (automatically) taken from the Bundle-Version since they are not specified anywhere else (see here).

My observation is:

1. When No Version is Specified in Export-Package:
   • Export-Package: Bnd automatically assigns the Bundle-Version to the Export-Package version if no version is specified.

• Import-Package (Self-Imports): In this case, Bnd does not assign a version to the Import-Package declaration for self-imports.

<configuration>
    <bnd><![CDATA[
        Bundle-Version: 1.0.0
        -exportcontents: example.a, example.b, example.c;
        Import-Package: example.c;version="${range;[==,=+)}", *
    ]]></bnd>
</configuration>

results in MANIFEST.MF:

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

2. When Version is Specified in Export-Package in bnd.bnd or Package-Level (packageinfo):
   • Export-Package: The specified version is explicitly added to the Export-Package declaration in the manifest.
   • Import-Package (Self-Imports): Bnd will also use this version in the Import-Package declaration for self-imports.

<bnd><![CDATA[
    Bundle-Version: 1.0.0
    -exportcontents: example.a, example.b, example.c;version=1.0.1
    Import-Package: example.c;version="${range;[==,=+)}", *
]]></bnd>

results in Manifest.mf

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

I guess my questions are (@pkriens , @bjhargrave ):

1. Is my observation correct?
2. if that is expected behavior? What is the explanation / rationale for this nuanced behavior?
3. what if there was an option (if technically possible) to force that Case 1 behaves like Case 2? ( I interpret that this is somehow the expectation of @Marcono1234 ?) e.g. analog to the option -nodefaultversion true

In the code it looks like for Import-Package this defaultVersion is explicitly set to null while Export-Package get the defaultVersion.

Looks like this was added a long long time ago.

If the behavior stays unchanged as it is, it would be good to document it e.g. in the FAQ

[Marcono1234]

3. what if there was an option (technically possible) to force that Case 1 behaves like Case 2? ( I interpret that this is somehow the expectation of @Marcono1234 ?)

It would certainly be great if there was a way to enable this behavior. But based on what I read here and in the Gson issue it feels to me (as someone external) that case 2 should be the default and it is a bug that the version is not propagated by bnd internally, despite being present.

---

In the code it looks like for Import-Package this defaultVersion is explicitly set to null while Export-Package get the defaultVersion

Not sure if passing a defaultVersion for Import-Package here is the solution. Wouldn't that then even be used for imported packages which are not self-imports?
(Or cleanupVersion would have to be adjusted to only use the default version for self-imports.)

(But I am not very familiar with the code.)

[chrisrueger]

Not sure if passing a defaultVersion for Import-Package here is the solution. Wouldn't that then even be used for imported packages which are not self-imports? (Or cleanupVersion would have to be adjusted to only use the default version for self-imports.)

Yes you are right, it is likely not the correct place in the code. I was just pointing to this specific place, because it is at least somehow related. My impression is, the challange is to find the place where self-imports are identified. Not familiar with the code either.

[chrisrueger]

@Marcono1234 I added draft PR #6238 as a base for discussion.

[chrisrueger]

@Marcono1234 Have a look at comment by @bjhargrave about the -noimport:=true instruction in #6238 (comment)

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.

I didn't know about -noimport:=true.
My interpretation is in the gson's pom.xml
they had to write something like this:

-exportcontents:\
                    com.google.gson,\
                    com.google.gson.annotations;-noimport:=true,\
                    com.google.gson.reflect,\
                    com.google.gson.stream

Question to @bjhargrave
What about an option / instruction or something to force automatically append -noimport:=true to all self-imported exports? (e.g. -noimport:=self)

Basically like saying: "All packages which I export and re-import I want them to never be imported from any other bundle".

[bjhargrave]

What about an option / instruction or something to force automatically append -noimport:=true to all self-imported exports? (e.g. -noimport:=self)

Basically like saying: "All packages which I export and re-import I want them to never be imported from any other bundle".

If you never intend to import an exported package, then -noimport:=true already does what you want. How would self change things?

A bundle cannot import from its own export. When a bundle declares it both exports and imports a package, only one of those is chosen by the framework during bundle resolution. So the bundle either exports without import or imports without export. So importing from one's self does not exist. If a bundle is exporting a package, any internal references to the package will be to the bundle's copy of the package.

[chrisrueger]

Thanks @bjhargrave I was just playing around with -noimport:=true. I realize my question did not make sense.

@Marcono1234 so I guess it comes down to this:

GSON needs to append -noimport:=true to all their exports which they do not want to re-import and keep Import-Package just using * in their pom.xml:

Import-Package: sun.misc;resolution:=optional, *

                -exportcontents:\
                    com.google.gson,\
                    com.google.gson.annotations;-noimport:=true,\
                    com.google.gson.reflect,\
                    com.google.gson.stream

This will cause that com.google.gson.annotations will not appear in the Import-Package instruction. Thus it cannot be imported from another bundle.

alternatively they could use the shortcut -exportcontents: *;-noimport:=true to apply it to all of them.

I just tested this with your bnd-test.zip of your other issue.

<bnd><![CDATA[
                            	Bundle-Version: 1.0.1
                                -exportcontents: *;-noimport:=true
                                Import-Package: *
                            ]]></bnd>

will result in the 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: java.lang

Update: adding link to documentation: https://bnd.bndtools.org/heads/export_package.html and a related discussion

[bjhargrave]

GSON needs to append -noimport:=true to all their exports which they want to re-import

append -noimport:=true to all their exports which they do not want to re-import

[chrisrueger]

append -noimport:=true to all their exports which they do not want to re-import

Thanks @bjhargrave , I updated my answer above.

On that note: I added a link to this discussion which I think is related.

[Marcono1234]

Thanks a lot for all your research, work and support on this!

append -noimport:=true to all their exports which they do not want to re-import

That is the question, what Gson wants. Personally (not being a direct member of Gson) I think it wants to produce an OSGi manifest which is usable by users, and does not cause any issues for them. Unfortunately, I don't know how to achieve that, and I don't know either if your suggestion to remove the "self-import" with -noimport:=true could cause other problems.
But it is certainly worth a try.

I am wondering though if other projects are affected by this default bnd behavior as well, and if in that case this "self-import (without version range?)" should be enabled by default.

[chrisrueger]

Thanks for getting back @Marcono1234

That is the question, what Gson wants. Personally (not being a direct member of Gson) I think it wants to produce an OSGi manifest which is usable by users, and does not cause any issues for them. Unfortunately, I don't know how to achieve that, and I don't know either if your suggestion to remove the "self-import" with -noimport:=true could cause other problems.
But it is certainly worth a try.

I am wondering though if other projects are affected by this default bnd behavior as well, and if in that case this "self-import (without version range?)" should be enabled by default.

My impression is, that changing default behavior won't happen, since it is usually dangerous for everybody out there relying on the current behavior.

BUT: I think that instead of changing default behavior we should make this a prominent documentation change and change the "default recommentation" we give to library authors.

The OSGi spec already defines strict boundaries when package substition (the current default) should be done, and I assume this is something a lot of non-OSGi library authors are not aware of - nor do a lot of them know what that means. I think changing the default-recommendation to something like "you should use -noimport:=true" most of the time could be worth-while. But I could be wrong.

Since I don't feel qualified enough to make this decision alone it would be great to get more opinion for the OSGi veterans around here.

I have written something in a related discussion in https://bnd.discourse.group/t/using-noimport-with-export-annotation/253/7 and maybe you could add your thoughts there too. I think the result of the the discussion could be something like a new "How-to OSGi-fy article for library authors" which is linked in various places in the current manual.

[Marcono1234]

I think the result of the the discussion could be something like a new "How-to OSGi-fy article for library authors"

Yes that would definitely be useful. The current situation is rather confusing, where there are three different views on what is correct:

• You should not disable self-imports normally (Discourse discussion)
• You should specify a version range for the self-import (Gson discussion)
• You should not use an import version range based on the bundle version (discussion in other PR), and as suggested above the self-import should be removed (at least for Gson's case)

I share the sentiment of that Discourse discussion, that for someone with little / no OSGi knowledge who wants to provide an OSGi manifest for their library for convenience, it is difficult to understand what the 'correct' way of doing this is.

Note: Maybe another possibility is also that the analysis in google/gson#2725 is incorrect and the self-import is not actually the cause?

[chrisrueger]

@Marcono1234 I have created PR google/gson#2735 to remove the self-import of gson.annotations via -noimport. Let's see how it plays out.

Meanwhile I have discovered a statement which makes me think -noimport is the right thing to do here.

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 suggest closing this issue here if you agree.

[pkriens]

Can we close this?

[chrisrueger]

I think so.
I addressed the main obstacle in #6267 and #6270