replaced Require-Bundle with Import Packages

[juergen-albert]

For unclear reasons Require-Bundle was introduced, which makes it unnecessarily hard to work with bundles outside of equinox, that have some strong reliance on eclipse core runtime. The use of Import-Packages here allows to circumvent this strong requirement in some cases with creative repackaging.

[juergen-albert]

Sidenote: As I was building it locally with tycho, the changes in my manifest did not make it in the build jar until I changed the Version. I found my Manifest correctly copied to the target directory, but the result seemed to come from a previous version. Is this normal or am I missing something?

[akurtakov]

The Require-bundle was introduced by @HannesWell in 97a02a0#diff-98e2e2939a98231d03432cd076fd94aa6396649b18bebff255b384f91134567c so I would let him review this one.

[HannesWell]

For unclear reasons Require-Bundle was introduced, which makes it unnecessarily hard to work with bundles outside of equinox, that have some strong reliance on eclipse core runtime. The use of Import-Packages here allows to circumvent this strong requirement in some cases with creative repackaging.

The reasoning in the mentioned change was that the FileLocator class relies on not-speced behavior.
For reference, this was discussed in https://git.eclipse.org/r/c/equinox/rt.equinox.bundles/+/188440/8#message-f2f90f7307c7963914f3c81351b9c0ee09ee12ea

So if the current state does not work for you we maybe should go for the alternative suggested by @tjwatson:

Alternatively we could consider adding a capability to org.eclipse.osgi for adapt functionality types that are beyond the spec'ed adapt types

Provide-Capability: org.eclipse.osgi.bundle.adapt; org.eclipse.osgi.bundle.adapt:List="File,..."

Then common bundle could require with:

Require-Capability: org.eclipse.osgi.bundle.adapt; filter:="(org.eclipse.osgi.bundle.adapt=File)"

I would be fine with that as well, but the adaptable types should be fully qualified class names.

[github-actions]

Test Results

     24 files  ±0       24 suites  ±0   11m 45s ⏱️ -36s
2 146 tests ±0  2 101 ✔️ ±0  44 💤 ±0  1 ❌ ±0 
2 190 runs  ±0  2 145 ✔️ ±0  44 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 873be56. ± Comparison against base commit d1fbae8.

[tjwatson]

For unclear reasons Require-Bundle was introduced, which makes it unnecessarily hard to work with bundles outside of equinox, that have some strong reliance on eclipse core runtime. The use of Import-Packages here allows to circumvent this strong requirement in some cases with creative repackaging.

I understand the usecase, but question why one would have a hard requirement to provision equinox bundles to other frameworks besides Equinox itself. Furthermore, with the limited team we have today that needs to focus on the Eclipse IDE usage of Equinox, why we need to spend time on such a thing that gets zero testing within our own tests of the project.

[laeubi]

Sidenote: As I was building it locally with tycho, the changes in my manifest did not make it in the build jar until I changed the Version. I found my Manifest correctly copied to the target directory, but the result seemed to come from a previous version. Is this normal or am I missing something?

If the bundle version do not change then the build artifact is replaced by the baseline version, this is to prevent distribute the same version with slightly different content, but you should have get a warning about that.

[laeubi]

The reasoning in the mentioned change was that the FileLocator class relies on not-speced behavior.

I must confess that even after reading the gerrit I don't understand what is the "not-speced behavior" and why it requires a require-bundle here? even if FileLocator won't work with other frameworks, otherparts of Equinox commons might do.

[maho7791]

For unclear reasons Require-Bundle was introduced, which makes it unnecessarily hard to work with bundles outside of equinox, that have some strong reliance on eclipse core runtime. The use of Import-Packages here allows to circumvent this strong requirement in some cases with creative repackaging.

I understand the usecase, but question why one would have a hard requirement to provision equinox bundles to other frameworks besides Equinox itself. Furthermore, with the limited team we have today that needs to focus on the Eclipse IDE usage of Equinox, why we need to spend time on such a thing that gets zero testing within our own tests of the project.

For unclear reasons Require-Bundle was introduced, which makes it unnecessarily hard to work with bundles outside of equinox, that have some strong reliance on eclipse core runtime. The use of Import-Packages here allows to circumvent this strong requirement in some cases with creative repackaging.

I understand the usecase, but question why one would have a hard requirement to provision equinox bundles to other frameworks besides Equinox itself. Furthermore, with the limited team we have today that needs to focus on the Eclipse IDE usage of Equinox, why we need to spend time on such a thing that gets zero testing within our own tests of the project.

As explaination: we use QVT or OCL that use e.g. o.e.core.runtime.CoreException. As this moved to o.e.equinox.common, we have that strong requirement to Equinox OSGi and it is not possible to use such bundles with other OSGi Frameworks anymore.

[tjwatson]

As explaination: we use QVT or OCL that use e.g. o.e.core.runtime.CoreException. As this moved to o.e.equinox.common, we have that strong requirement to Equinox OSGi and it is not possible to use such bundles with other OSGi Frameworks anymore.

Just to be clear, you must be able to run equinox.common on other frameworks? We have no tests for doing that, so it is luck that it works.

[laeubi]

This reminds me of:

• Provide a new installBundle method that accepts a Manifest osgi/osgi#566

what would allow to make a new bundle manifest quite easy that only exposes the required packages, maybe BND can even repack a dependency somehow?

But even more worse o.e.core.runtime is a split package, so maybe a much more better solution would be to seperate the o.e.core.runtime into one bundle with less dependencies and reexport this from commons (what would require a require-bundle but ... )

[tjwatson]

I must confess that even after reading the gerrit I don't understand what is the "not-speced behavior" and why it requires a require-bundle here? even if FileLocator won't work with other frameworks, otherparts of Equinox commons might do.

It was the use of:

return Optional.ofNullable(bundle.adapt(File.class));

Where we depend on Bundle.adapt to be able to adapt to a File. Before we looked for a implementation of URLConverter service to convert a bundle resource URL to a File. In "theory" another bundle could provide such a service implementation on another framework.

If someone is willing to spend time developing testcases that prove equinox.common and other things remain "working" on other frameworks then I would be more open to having the project support such a scenario. But the current state of no tests make it very hard to claim any of this works.

[maho7791]

As explaination: we use QVT or OCL that use e.g. o.e.core.runtime.CoreException. As this moved to o.e.equinox.common, we have that strong requirement to Equinox OSGi and it is not possible to use such bundles with other OSGi Frameworks anymore.

Just to be clear, you must be able to run equinox.common on other frameworks? We have no tests for doing that, so it is luck that it works.

Because other bundle have import requirements to org.eclipse.core.runtime.* we might end up in o.e.equinox.common. In that case o.e.core.runtime.CoreException. From my point of view this is more a problem related to the split packages in org.eclipse.core.runtime and org.eclipse.equinox.common. From what I can see is that our QVT bundles require just some org.eclipse.core.runtime API, means interfaces IProgressMonitor and NullProgressMonitor or CoreException. Having a pure org.eclipse.core.runtime API bundle would be the greatest thing :-)

[maho7791]

I must confess that even after reading the gerrit I don't understand what is the "not-speced behavior" and why it requires a require-bundle here? even if FileLocator won't work with other frameworks, otherparts of Equinox commons might do.

It was the use of:

return Optional.ofNullable(bundle.adapt(File.class));

Where we depend on Bundle.adapt to be able to adapt to a File. Before we looked for a implementation of URLConverter service to convert a bundle resource URL to a File. In "theory" another bundle could provide such a service implementation on another framework.

If someone is willing to spend time developing testcases that prove equinox.common and other things remain "working" on other frameworks then I would be more open to having the project support such a scenario. But the current state of no tests make it very hard to claim any of this works.

If you assist us in form of reviews, we would invest some time creating test cases to be able to start restructuring of bundles, that are backed with tests

[laeubi]

It was the use of:

return Optional.ofNullable(bundle.adapt(File.class));

Where we depend on Bundle.adapt to be able to adapt to a File.

But that's exactly what I mean, we do not really depend on that working, as it is an optional it just might be that for other frameworks this always returns an empty optional (while as you said it is not forbidden, just not required that a bundle can be adapted to a File).

[tjwatson]

But that's exactly what I mean, we do not really depend on that working, as it is an optional it just might be that for other frameworks this always returns an empty optional (while as you said it is not forbidden, just not required that a bundle can be adapted to a File).

Who is "we"? I assume "someone" depends on it working. If not, then shouldn't we deprecate the method and mark for removal?

[laeubi]

Who is "we"? I

The commons bundle / equinox

I assume "someone" depends on it working

users of that method can still depend on it, it will just always give an empty optional if not used in equinox, so users that require this simply must choose equinox, as with other equinox features (from my understanding mostly Equinox itself and Eclipse really 'depend' on the feature so I don't see a general issue here.

[tjwatson]

users of that method can still depend on it, it will just always give an empty optional if not used in equinox, so users that require this simply must choose equinox, as with other equinox features (from my understanding mostly Equinox itself and Eclipse really 'depend' on the feature so I don't see a general issue here.

There are two problems with this:

1. Nothing in the API documents what is and is not supported on other frameworks.
2. Nothing is ever tested on other frameworks.

Both are situations that make it hard to claim any support on other frameworks and make it harder for maintainers to keep the project working.

[tjwatson]

Stepping back, if others want to loosen the dependency declared by equinox.common to allow resolution on other frameworks then I will not stand in the way. But I still do not completely understand the usecase for allowing such a thing when a very large portion of the rest of the Eclipse runtime will fall flat without the Equinox framework. If such usecases are valid, they are built on a house of cards right now. I am not confident the existing maintainers of the project are committed to doing all the work to specify what is and is not supported on other frameworks and put in place tests that ensure what does work continues to work.

[laeubi]

There are two problems with this:

But it is also not documented that it only works on equinox, the point is that it was said that Require-Bundle was choose because it is some kind of requirement, but I don't think it is, because as you said most of the time this will be used with Equinox and is tested with Equinox, so there is no need for Require-Bundle anyways.
For the other cases, the users are already needing take special care, and the Require-Bundle does not really help these users. So I think, we can safely remove / replace the Require-Bundle without any issues.

Beside that, I have the feeling that on the long term one should slip equinox in distinct, independent parts (like for example it is done for the maven-....-plugin that are all separate sub-projects but still be used together as "the maven tool")

I have opened:

• Extract service implementations into seperate sub-projects that are plain maven projects #254

[HannesWell]

Personally I would not object to switch back to Import-Package and provide that bundle as is without any warranty for any other Framework except Equinox.
As already said the FileLocator is prepared for that scenario, but of course other parts maybe behave less graceful when running inside another framework impl.

It would then be up to the users that use those bundles in another framework to find out if that works or not. Nevertheless they should be made aware of the fact that they are building on a house of cards as Tom said and not blame equinox in the end.

But the suggestion to better split concerns seems reasonable to me, at least on bundle level.

[maho7791]

As explaination: we use QVT or OCL that use e.g. o.e.core.runtime.CoreException. As this moved to o.e.equinox.common, we have that strong requirement to Equinox OSGi and it is not possible to use such bundles with other OSGi Frameworks anymore.

Just to be clear, you must be able to run equinox.common on other frameworks? We have no tests for doing that, so it is luck that it works.

No, but I have to. This is what I wanted to avoid. Its is absolutely ok, the equinox.common is strongly wired to org.eclipse.osgi.
The problem is more, that because of one Exception class that moved from o.e.core.runtime bundle to o.e.equinox.common bundle, I stick with Equinox, just because of the spli-package problem.

In an ideal world I could consume the Excpetion class from an core.runtime.api bundle :-)

[laeubi]

I think one path would be to first reduce the number of references to the org.eclipse.runtime.core bundle, ILog seems a good candidate to move it as a first step, I created

• Provide static acessors in the ILog interface as an alternative to Platform eclipse-platform/eclipse.platform#498

to prepare for this.

[laeubi]

@juergen-albert what do you think about adding a required capability of type "osgi.framework = equinox" here and provided that from the org.eclipse.osgi bundle?
That then should address the concerns of @tjwatson that this bundle should be used with equinox but still not need to use require-bundle to express that and import package can be used instead...