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 an equinox.api bundle #711

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Dec 4, 2024

Currently we have usually have bundles that are mixed with API and
implementation, even worse those also consists of split packages, most
notable the 'org.eclipse.core.runtime' package that makes it hard to use
the API independent from the implementation and to not break existing
usages it is not easily possible to improve the situation because of the
widely use of require bundle in eclipse. Also we do not even have good
API check support for exported packages and split-packages in general
what has caused problems in the past.

This now tries to improve the situation in the following way:

  • add a new equinox.api bundle that host the actual API
  • require-bundle it on the old places with reexport to ensure backward
    compatibility
  • add tycho-baseline-mojo to check for package version consistency
    according to semantic-versioning

As a result, previous consumers of org.eclipse.equinox.common and
org.eclipse.equinox.registry can now safely use import-package instead
allowing for easy replacement of the API if desired.

Currently we have usually have bundles that are mixed with API and
implementation, even worse those also consists of split packages, most
notable the 'org.eclipse.core.runtime' package that makes it hard to use
the API independent from the implementation and to not break existing
usages it is not easily possible to improve the situation because of the
widely use of require bundle in eclipse. Also we do not even have good
API check support for exported packages and split-packages in general
what has caused problems in the past.

This now tries to improve the situation in the following way:
-  add a new equinox.api bundle that host the actual API
- require-bundle it on the old places with reexport to ensure backward
compatibility
- add tycho-baseline-mojo to check for package version consistency
according to semantic-versioning

As a result, previous consumers of org.eclipse.equinox.common and
org.eclipse.equinox.registry can now safely use import-package instead
allowing for easy replacement of the API if desired.
Copy link

github-actions bot commented Dec 4, 2024

Test Results

0 files   -   663  0 suites   - 663   0s ⏱️ - 1h 17m 23s
0 tests  - 2 209  0 ✅  - 2 161  0 💤  -  47  0 ❌  - 1 
0 runs   - 6 771  0 ✅  - 6 627  0 💤  - 143  0 ❌  - 1 

Results for commit cb35d63. ± Comparison against base commit caf78f7.

@vogella
Copy link
Contributor

vogella commented Dec 4, 2024

A bold move and a great one. Would also be nice to have a way to deprecate the usage of the old export, so that users see that they should adjust there dependencies.

Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.api
Bundle-Version: 1.0.0.qualifier
Export-Package: org.eclipse.core.runtime;version="3.7.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes another bundle export this package split package.

image

I assume some attributes will be needed. I'm not sure if other importers will need to change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume some attributes will be needed. I'm not sure if other importers will need to change?

No the goal is to have one package without mandatory attributes so users that want to reference the new api exported package do not need to specify anything and old users (that use an attribute) get the one as before.

Actually we then can replace all these special imports with regular ones.

@tjwatson
Copy link
Contributor

tjwatson commented Dec 4, 2024

I don't see how this approach achieves the goals. Is the goal have a single bundle that exports all the parts of org.eclipse.core.runtime? If so then don't you need all the stuff from the legacy org.eclipse.core.runtime bundle also? You now will have two exporters of org.eclipse.core.runtime with no mandatory directives. That makes it very much unsafe to do import package on org.eclipse.core.runtime because you will not know which version you will get wired to.

If we want to do this then I suggest moving all the parts of the org.eclipse.core.runtime to platform (org.eclipse.equinox.common, org.eclipose.equinox.registry). Then put ALL the API from org.eclipse.core.runtime into some "API" bundle and then have all the various bundles do the requires on that.

But that all seems very much an academic exercise that I see not solving any real problem (much like the original notion of splitting org.eclipse.core.runtime into smaller "parts"). I think all would be much more maintainable if we simply moved all the parts (including the API) back to the org.eclipse.core.runtime bundle. I don't see any advantage in separating the API from the bundle here because we don't have any real usecases where we want a different implementation of the API than we provide by this project.

We would still need to provide the "parts" bundles like equinox.common and equinox.registry, but they would be shell bundles that simply require the core.runtime bundle and export the facade "part" of the org.eclipse.core.runtime bundle.

@laeubi
Copy link
Member Author

laeubi commented Dec 4, 2024

I don't see how this approach achieves the goals. Is the goal have a single bundle that exports all the parts of org.eclipse.core.runtime? If so then don't you need all the stuff from the legacy org.eclipse.core.runtime bundle also?

Yes in the end there should only be one org.eclipse.core.runtime package

I think all would be much more maintainable if we simply moved all the parts (including the API) back to the org.eclipse.core.runtime bundle.

In general yes, but I don't see how it would be possible to break exiting users that e.g. currently require-bundle equinox-common. Therefore my idea was to create a new bundle.

I don't see any advantage in separating the API from the bundle here because we don't have any real usecases where we want a different implementation of the API than we provide by this project.

The problem is more that the implementation have very specific structure and especially the

Require-Bundle: org.eclipse.osgi;bundle-version="[3.18.0,4.0.0)";visibility:=reexport

is something that really make things worse, so I don't see how to make this ever go away an have a "clean" org.eclipse.core.runtime bundle.

And as the package contains some very basic interfaces (e.g. IProgressMonitor and/or CoreException) this is pulled in almost everywhere. And this is for example also mentioned here that people like to substitute the real bundle/api:

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.

4 participants