Prefer explicitly included plugins for Eclipse/OSGi launches

[HannesWell]

When launching an application create a new Equinox resolver state, where explicitly included plugins/bundles are preferred and compute the requirements closure from that state. This avoids adding other providers of a capability if another provider is already (explicitly) included in a launch, just because the former is wired to the requirement in the target-platform state. Potentially this also makes the set of launched plugins more compact.

Fixes #1604

Alternative approach to #1618.

Requires (and currently includes)

• Reduce re-computation of plugins included in Eclipse/OSGi launches #1724

[github-actions]

Test Results

   285 files  ±0     285 suites  ±0   48m 14s ⏱️ + 2m 11s
 3 611 tests +2   3 535 ✅ +2   76 💤 ±0  0 ❌ ±0 
11 025 runs  +6  10 794 ✅ +6  231 💤 ±0  0 ❌ ±0 

Results for commit 86e818e. ± Comparison against base commit 98a5134.

♻️ This comment has been updated with latest results.

[HannesWell]

@merks I'm current trying to reproduce the problem in #1618 (comment). I used the following target definition and a simple feature-based Eclipse app launch that just contains the org.eclipse.sdk feature.

<target includeMode="feature" name="featureBased">
	<locations>
		<location includeAllPlatforms =   "false"   includeConfigurePhase  = "true" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/releases/2025-03/"/>
			<unit id="org.eclipse.rap.jface"/>
			<unit id="org.eclipse.sdk.feature.group"/>
			<!--unit id="org.eclipse.emf.rap.sdk.feature.group"/-->
		</location>
	</locations>
</target>

Unfortunately in the current state the org.eclipse.rap.rwt and org.eclipse.rap.jface bundles are still pulled into the launch, so I have to find another way to ensure that the new launch-state prefers the explicitly included plugins/bundles... :/
Just adding the included bundles first seems not to be sufficient, if anyone already has a better idea how to ensure that, please let me know.

In a second step we could also then try to avoid the second state creation that's currently done as part of the launch-validation in:

eclipse.pde/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/BundleValidationOperation.java

Lines 59 to 69 in e63f3f9

	fState = FACTORY.createState(true);
	for (IPluginModelBase fModel : fModels) {
	BundleDescription bundle = fModel.getBundleDescription();
	if (bundle != null) {
	fState.addBundle(FACTORY.createBundleDescription(bundle));
	}
	subMonitor.split(1);
	}
	fState.setPlatformProperties(fProperties);
	fState.resolve(false);
	subMonitor.split(1);

[laeubi]

I think the initial problem to only offer a subset of bundles to the resolver remains, so while it will be good to use the resolver to ensure the result is runnable (like done in the validation dialog) and maybe reduce the inital set I'm not sure the Resolver alone helps here, e.g in Tycho P2 is used to compute the inital set of bundles (also to not make the work harder for the resolver to resolve things that never will be used).

The resolver supports incremental resolve, but again you then need to choose what bundles to refresh and then you may end up with unwanted stuff in the result again.

So I think one needs to replicate what the P2 Slicer does, that is using a set of root bundles, then incrementally add providers if the requirement is not satisfied by any previous bundle. Then this initial set is passed to the Planner (what would be the Resolver/state here).

As an alternative one could look at the "new" OSGi resolver that allows more control over the process of resolving but its not that trivial you can look at how BND does it with biz.aQute.resolve.RunResolution.resolve(Project, Processor, Collection<ResolutionCallback>, ResolverLogger) or use a bnd run for that like shown here:

• https://github.com/eclipse-pde/eclipse.pde/pull/1620/files#diff-b5198d92fb6f54b9dc000bc8411312b4280610606e3be776df582c1b1c4f59f7R174
• https://github.com/eclipse-pde/eclipse.pde/pull/1620/files#diff-8c82bb511c93f25e13706ba45aec766256a6421a7c13acd49464adc9fd374ef9R135

but in any case someone needs to decide if a provider should be chosen or not (by excluding them) or how to sort providers (in wich case the resolver might still decide to use another one).

[laeubi]

Should not the launchBundle be added to the initialBundles?

[HannesWell]

This is correct as that set collected the initial (or better named incluced) bundles from the TP-state so that in the second pass the remaining bundles from the TP state can be added.

[laeubi]

I think you can use State.getResolver() and then Resolver.setSelectionPolicy(Comparator<BaseDescription>) but maybe @tjwatson has a better idea.

[HannesWell]

Thank you! That's what I was looking for, but I just had very vague memories and therefore didn't found it before.

While looking at this I noticed that the PDEState already uses this to enhance the default behavior described in Resolver.setSelectionPolicy() to consider if a bundle originates from a workspace-project:

eclipse.pde/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/PDEState.java

Lines 83 to 87 in e63f3f9

	final String systemBSN = getSystemBundle();
	Comparator<BaseDescription> policy = systemBundlesFirst(systemBSN)
	.thenComparing(BaseDescription::getVersion, HIGHER_VERSION_FIRST)
	.thenComparing(BaseDescription::getSupplier, HIGHER_LOCAL_VERSION_FIRST);
	fState.getResolver().setSelectionPolicy(policy);

So we can reuse this here and just have to define a first comparator that prefers explicitly included bundles and then delegates to the tpState's policy.

[HannesWell]

I think the initial problem to only offer a subset of bundles to the resolver remains, so while it will be good to use the resolver to ensure the result is runnable (like done in the validation dialog) and maybe reduce the inital set I'm not sure the Resolver alone helps here, e.g in Tycho P2 is used to compute the inital set of bundles (also to not make the work harder for the resolver to resolve things that never will be used).

Keeping the work as low as possible for the resolver actually sounds good, but I'm not sure how to replicate this without reproducing a significant portion of the revolvers task. Wouldn't we need to collect all the capabilities that could satisfy each requirement and then add the provider to the state? To do that efficiently we would have to build indices again, which will probably a lot of work if it should be efficient.
Ideally we could command the resolver to just resolve a given set of bundles and their requirements, but I assume that's not possible with the old resolver. IIRC the 'new' resolver has such methods?

But besides that, at least if the resolver respects result of the selection policy I think this should be a save solution for the problem, shouldn't it? And if one wants to make sure that a specific provider is used, then that bundle has to be (explicitly) included into the product or launch-config, similar like it's for example the case with optional requirements.

However, with the current state, using a further enhanced selection policy, the RAP bundles are at least not pulled into the runtime (unfortunately the launch then fails with other errors I wasn't able to solve yet). So at least for @merks case I hope this is a solution. But still this has to be further cleaned-up.

Btw. another way that should avoid the explicit comparator would be to make sure that included bundles have a lower bundle-id. Currently the ids are copied from the TP state and it's relatively cumbersome to assign new ones. Furthermore this would also only help if the unwanted provider and included provider have the same version.

[laeubi]

Keeping the work as low as possible for the resolver actually sounds good, but I'm not sure how to replicate this without reproducing a significant portion of the revolvers task.

The resolvers task is to wire a consistent class space for all bundles and this is much more work than a simple slice operation that only collects what could be a potential match.

Actually the resolver works in different phases:

1. All providers for a requirement a collected (that is where the sorting comes into play) usually sorted with "highest version is better", if there are equal ones highest bundle id wins.
2. The highest ranked provider is chosen for a requirement if possible
3. The classspace is computed and checked for violations
4. If there is any violation, per-mutate the solution (e.g. choose the next possible option) and check again
5. If there is a timeout or no options left, fail in some way or the other

The "Task 1" has to be done anyways, there is no way to prevent this work, but here we want to strike out any other provider if the user has explicitly chosen one what might be result in an impossible state of course.

Wouldn't we need to collect all the capabilities that could satisfy each requirement and then add the provider to the state? To do that efficiently we would have to build indices again, which will probably a lot of work if it should be efficient.

Yes I don't see how it could be avoided, but that task is quite efficient already, and at the best will save a lot more time as the problem is potentially NP-hard, even one more options could increase the resolver time. I also doubt we really need high sophisticated index, a simple set is usually sufficient.

One might want to use some optimizations e.g. if requirements for a package / bundle where already queried before as they are usually used multiple times, so one would use one set that contains all requirements already checked and then first check all capabilities already provided (as presented here) but that's not much magic in the end.

But besides that, at least if the resolver respects result of the selection policy I think this should be a save solution for the problem, shouldn't it

The resolver will first try a higher ranked provider, but there is no guarantee its chosen. And if the user chose bad, the resolver will need to pick another one and possibly waste a lot of time.

another way that should avoid the explicit comparator would be to make sure that included bundles have a lower bundle-id

I really don't like that it is more to make sure we have a consistent ordering always.

[HannesWell]

4. If there is any violation, per-mutate the solution (e.g. choose the next possible option) and check again

If another solution has to be selected because the state cannot resolved otherwise I think we have to accept that. Just not including the corresponding provider will lead to resolution errors at runtime, which is not desired.

[HannesWell]

With the latest updates all existing test should now pass and new tests to cover the fixed issue are added for feature and plugin based launches.

I have also looked into the points discussed above and personally and think we really have to use the resolver here, as adding too few bundles to the runtime (respectively excluding too many), could lead to resolution errors due to class-space consistency violations and adding too many bundles to the runtime could lead to the situation described in #1604. So we should really included exactly the minimally required set of bundles to be able to cleanly resolve the bundles explicitly included in a launch. For that we need the resolver. We instruct the resolver to prefer the bundles already included, but if others are necessary, I think we have to accept that, as otherwise it leads to runtime errors.

The only drawback I see in this approach is the created second state is resolved completely and not only a given set of root bundles plus their dependency closure. I looked into ways to achieve this, but didn't find a working solution for that.
There is a method on the state that looked promising:
https://github.com/eclipse-equinox/equinox/blob/586e30bb9befd265fc404ad3496b6fabb26eb781/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/service/resolver/State.java#L399-L411

But this also resolved all not yet resolved bundles, which in this case affects all bundles and therefore makes no difference. Maybe the new resolver Equinox is using can only partially resolve the set of bundles (at least one can request it from the framework). But from my experience with the TP the actual resolution, even of the entire TP, is fast.

With that I consider this as ready.

[laeubi]

The only drawback I see in this approach is the created second state is resolved completely and not only a given set of root bundles plus their dependency closure.

I think in general it is good if we resolve the bundles, as long as all bundles are included. bndtools is working similar with there bndrun/resolve approach. I think one can reduce the set of bundles used for resolving, but we could better do it as a separate step, and it would best to have

• Generalize the Capabilities class and move it to exported package eclipse-equinox/equinox#935

for this.

With that I consider this as ready.

Then lets merge this now so we make it available as soon as possible!

[HannesWell]

The only drawback I see in this approach is the created second state is resolved completely and not only a given set of root bundles plus their dependency closure.

I think in general it is good if we resolve the bundles, as long as all bundles are included. bndtools is working similar with there bndrun/resolve approach. I think one can reduce the set of bundles used for resolving, but we could better do it as a separate step, and it would best to have

* [Generalize the Capabilities class and move it to exported package eclipse-equinox/equinox#935](https://github.com/eclipse-equinox/equinox/pull/935)

Sure we can always optimize this further.

With that I consider this as ready.

Then lets merge this now so we make it available as soon as possible!

Yes. I just added a few more comments in the tests and plan to submit this then.

Thanks everyone for the discussion. I think this closes another, longer standing and potentially very annoying, corner-case in the launch procedure of Eclipse app and makes it even more precise.