bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
527 stars 305 forks source link

bnd-baseline-maven-plugin: <diffignore>Bundle-Version</diffignore> leads to completely ignoring package deletions #5290

Closed kwin closed 2 years ago

kwin commented 2 years ago

The usage of <diffignore>Bundle-Version</diffignore> as being documented in the readme in the context of #3446 leads to completely ignoring incompatible package removals.

To reproduce just take a project which doesn't pass package baselining due to a formerly exported package being deleted and add the plugin configuration:

<diffignores>
    <diffignore>Bundle-Version</diffignore>
</diffignores>

Now the build succeeds. So we either need to fix this behaviour or need another flag to disable semantic bundle versioning (but keep semantic package versioning)

bjhargrave commented 2 years ago

Can you please provide a small github repo which demonstrates the problem? Then we can investigate. Thanks.

kwin commented 2 years ago

I will come up with an additional test case for that behaviour in https://github.com/bndtools/bnd/blob/b52e7978029b4eea358aa295a580802042ba5b6b/biz.aQute.bndlib.tests/test/test/baseline/BaselineTest.java. Hopefully I can get that done today.

bjhargrave commented 2 years ago

The usage of <diffignore>Bundle-Version</diffignore> as being documented in the readme in the context of #3446 leads to completely ignoring all package incompatibilities.

I don't see that this is true in my normal development work on Bnd. We ignore Bundle-Version in the Bnd build

https://github.com/bndtools/bnd/blob/b25057bda79b087bed5f3856c343bbe6e8a5ebca/cnf/build.bnd#L27-L28

And I always get baseline errors when I add a member and fail to change the package version.

kwin commented 2 years ago

My example is completely removing a package. That does no longer lead to an issue when you diffignore Bundle-Version. This is probably an edge case as this would in fact be allowed if the bundle version would get a major increment.

bjhargrave commented 2 years ago

My example is completely removing a package.

If you ignore Bundle-Version, there is nothing to "diff" the removed package against given that Bundle-Version considers the aggregated packages. So I think the code is behaving as expected.

kwin commented 2 years ago

Right, but the intent is rather, that I don't always have to increment the major bundle version for every major package change. On the other hand I still want to enforce a major version bundle change in case of complete package deletions. I think this is also what you would like to have in Bnd, because otherwise non-deliberate API package removals might go unnoticed.

bjhargrave commented 2 years ago

I think you either diff Bundle-Version because you care about the aggregate packages in the artifact and want to make some attempt at semantic versioning of the bundle version, or you accept that the Bundle-Version is just a marketing version (in Bnd we increment the version of all artifacts at each release even when they don't otherwise change) and then it is up to the developers to manage the specific artifacts that packages are part of.

kwin commented 2 years ago

or you accept that the Bundle-Version is just a marketing version

Even if that is the case, it would be good if one would somehow need to deliberate acknowledge an API package removal. Otherwise you might accidentally break backwards-compatibility. I would love to use the baselining feature also for that edge case. Maybe an explicit baselining configuration that the removal was fine should do it. WDYT?

bjhargrave commented 2 years ago

Otherwise you might accidentally break backwards-compatibility

Removing a package from an artifact does not break backwards-compatibility with the package since the package is what other artifacts are importing. It just means that some other artifact which exports the package is needed.

This just happened to me with org.apache.felix.scr. Version 2.2.2 of org.apache.felix.scr stopped including and exporting the org.osgi.service.component package, so when updating from 2.2.0 to 2.2.2, I also had to include the org.osgi.service.component artifact in the resolution to provide the org.osgi.service.component package.

The main use of baselining is about changes in a package not about which artifact provides the package.

bjhargrave commented 2 years ago

Maybe an explicit baselining configuration that the removal was fine should do it. WDYT?

Any such change would need to be done at the base bndlib API level and then exposed through the maven and gradle plugins as well as in Bnd workspace builds. That is, we need to keep parity in function like baselining across all the ways in which Bnd is used.