Skip to content

Prevent generating import-package version ranges for unstable API #6702

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kwin
Copy link
Contributor

@kwin kwin commented Jul 24, 2025

This closes #6701

@kwin kwin force-pushed the feature/prevent-import-ranges-for-unstable-api branch 3 times, most recently from 16b2fca to 5b35184 Compare July 25, 2025 06:25
@kwin kwin marked this pull request as ready for review July 25, 2025 06:46
@chrisrueger
Copy link
Contributor

chrisrueger commented Jul 25, 2025

Looks good to me. The only thought I have is wether this warning should be always emitted (like you have it now) or only with -pedantic: true.

Furthermore: Where could this behavior be described in the docs. Maybe somewhere here?

@kwin
Copy link
Contributor Author

kwin commented Jul 25, 2025

There is already some functionality which prints warnings about missing versions on import-package headers:

if (isPedantic() && noimports.size() != 0) {
warning("Imports that lack version ranges: %s", noimports);
}
which is given out with -pedantic: true. I am gonna rework my patch to not emit any warning and just rely on the existing logic for the warning. However this warning does not indicate why the version range is missing....

@kwin
Copy link
Contributor Author

kwin commented Jul 25, 2025

After having a 2nd thought I would rather prefer to always emit a warn when importing unstable API (not only with pedantic). This is different from the case where you explicitly import a package without a version when bnd cannot find any matching dependency in the classpath. I will just look into preventing the duplicate warning when depending on unstable API. WDYT @chrisrueger

@chrisrueger
Copy link
Contributor

I have no strong feelings about this just wanted to bring it up for discussion. I'm fine with the warning always. Maybe let's wait for some more feedback. I mentioned this pr in the bnd slack and @peterkir and @reckart had some comments.

@reckart
Copy link
Contributor

reckart commented Jul 25, 2025

How about leaving the range as-is and instead you could add package-level versioning to your exports? That way, if a consumer of your artifact upgrades from 0.x to 1.x, they may still get package-level 0.x compatibility - wouldn't that resolve the issue as well?

Alternatively, you could explicitly export using *;version=0.0.0 while still in a phase where you do not want to have ranges being generated.

@kwin kwin force-pushed the feature/prevent-import-ranges-for-unstable-api branch from 5b35184 to b9a1a8e Compare July 25, 2025 12:37
@kwin
Copy link
Contributor Author

kwin commented Jul 25, 2025

I would argue that version ranges are not useful for unstable API (not complying with semantic versioning) in almost all of the cases. Rather generating a version range for unstable APIs should be a manual thing (as this would be special and cannot be derived from semantic versioning rules), but the default should be no restriction.

@kwin kwin force-pushed the feature/prevent-import-ranges-for-unstable-api branch from b9a1a8e to 9fc763d Compare July 25, 2025 12:52
@@ -2117,7 +2126,7 @@ void augmentImports(Packages imports, Packages exports) throws Exception {

String result = importAttributes.get(Constants.VERSION_ATTRIBUTE);
if ((result == null || !Verifier.isVersionRange(result))
&& complainAboutMissingVersionRange(packageRef, ee)) {
&& complainAboutMissingVersionRange(packageRef, ee) && !isUnstable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about passing isUnstable to the complainAboutMissingVersionRange method? so this method is the one deciding wether to complain or not. Maybe that would be the better place, as it also considers JDK-packages - JDK packages do not have a version which is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to expose a different explanation for unstable APIs being referenced and for non-versioned or unknown dependencies being referenced.

@peterkir
Copy link
Contributor

I would argue that version ranges are not useful for unstable API (not complying with semantic versioning) in almost all of the cases. Rather generating a version range for unstable APIs should be a manual thing (as this would be special and cannot be derived from semantic versioning rules), but the default should be no restriction.

Why is it not complying with with semantic versioning.
As long as I don't have a public/official consumer, I always use 0.x versions and have a proper tooling with bnd.
I'm starting my APIs with 0.1.0 and use already baselining and API tooling.
This would break existing workspaces 😞
So from my point of view I disapprove this PR.
You can use 'Export-Package: *:version=' or 'Export-Package: *:version=0.0.0'
If you do not want use the a concrete versioning.

@kwin
Copy link
Contributor Author

kwin commented Jul 26, 2025

This does not change export versions. It will merely no longer generate import version ranges for unstable API. A version range (0.x, 1.0] is not really useful as unstable API may break with every non major version increment but must not necessarily break when promoting to 1.x.

I'm starting my APIs with 0.1.0 and use already baselining

I am really keen to know how that works for you as baselining checks against 0.x packages will never emit violations…

@laeubi
Copy link
Contributor

laeubi commented Jul 31, 2025

I don't think that there is any mean for "unstable API" in the OSGi specification. Exported packages per se has to comply with the OSGi semantic versioning rules, so there is no exception where it is "allowed" to break it. If you want something like make your consumers aware that you do not plan to follow these rules, I would recommend to use @API Guardian (what is supported by bnd already if I remember correctly).

So I think the assumption

It will merely no longer generate import version ranges for unstable API. A version range (0.x, 1.0] is not really useful as unstable API may break with every non major version increment but must not necessarily break when promoting to 1.x.

is simply wrong and does not match the OSGi specification and what bnd does here is generally valid. The main problem with an export without a version actually is that one can not define a lower bound but this does not mean an upper bound is of no use!

Eclipse makes some use of this by do not version the API of packages but only on the bundle level in wich case one then need to require the bundle with a suitable range (what is a much stronger requirement).

@chrisrueger
Copy link
Contributor

I digged a bit in git to find out where the bnd treatment of versions <= 1 comes. I found:

@laeubi
Copy link
Contributor

laeubi commented Jul 31, 2025

This does not change export versions. It will merely no longer generate import version ranges for unstable API. A version range (0.x, 1.0] is not really useful as unstable API may break with every non major version increment but must not necessarily break when promoting to 1.x.

If you are really concerned for this I think the solution is not to use no version range but instead bnd must generate a provider range that would be (0.x, 0.x+1].

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.

Don't generate "import-package" version restrictions for preliminary APIs (i.e. major version being 0)
5 participants