Prefer explicitly included plugins for Eclipse/OSGi launches

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

@@ -262,7 +262,7 @@ private static boolean isTestWorkspaceProject(Resource f) {
 	 *
 	 * @return a set of bundle ids
 	 */
-	private static Collection<NameVersionDescriptor> getImplicitDependencies() {
+	public static Collection<NameVersionDescriptor> getImplicitDependencies() {
 		try {
 			ITargetPlatformService service = PDECore.getDefault().acquireService(ITargetPlatformService.class);
 			if (service != null) {

ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java

@@ -50,7 +50,9 @@
 import org.eclipse.core.runtime.Status;
 import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
+import org.eclipse.osgi.service.resolver.BaseDescription;
 import org.eclipse.osgi.service.resolver.BundleDescription;
+import org.eclipse.osgi.service.resolver.State;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.pde.core.plugin.IMatchRules;
 import org.eclipse.pde.core.plugin.IPluginBase;
@@ -59,10 +61,12 @@
 import org.eclipse.pde.core.plugin.PluginRegistry;
 import org.eclipse.pde.core.target.ITargetDefinition;
 import org.eclipse.pde.core.target.ITargetPlatformService;
+import org.eclipse.pde.internal.build.BundleHelper;
 import org.eclipse.pde.internal.build.IPDEBuildConstants;
 import org.eclipse.pde.internal.core.DependencyManager;
 import org.eclipse.pde.internal.core.FeatureModelManager;
 import org.eclipse.pde.internal.core.PDECore;
+import org.eclipse.pde.internal.core.PluginModelManager;
 import org.eclipse.pde.internal.core.TargetPlatformHelper;
 import org.eclipse.pde.internal.core.ifeature.IFeature;
 import org.eclipse.pde.internal.core.ifeature.IFeatureChild;
@@ -75,7 +79,6 @@
 import org.eclipse.pde.internal.launching.PDEMessages;
 import org.eclipse.pde.launching.IPDELauncherConstants;
 import org.osgi.framework.Version;
-import org.osgi.resource.Resource;
 
 public class BundleLauncherHelper {
 
@@ -166,12 +169,7 @@ private static void addRequiredBundles(Map<IPluginModelBase, String> bundle2star
 		RequirementHelper.addApplicationLaunchRequirements(appRequirements, configuration, bundle2startLevel);
 
 		boolean includeOptional = configuration.getAttribute(IPDELauncherConstants.INCLUDE_OPTIONAL, true);
-		Set<BundleDescription> requiredDependencies = includeOptional //
-				? DependencyManager.getDependencies(bundle2startLevel.keySet(), DependencyManager.Options.INCLUDE_OPTIONAL_DEPENDENCIES)
-				: DependencyManager.getDependencies(bundle2startLevel.keySet());
-
-		requiredDependencies.stream().map(Resource.class::cast) //
-				.map(PluginRegistry::findModel).filter(Objects::nonNull) //
+		computeDependencies(bundle2startLevel.keySet(), includeOptional, true) //
 				.forEach(p -> addDefaultStartingBundle(bundle2startLevel, p));
 	}
 
@@ -223,16 +221,12 @@ private static Map<IPluginModelBase, String> getMergedBundleMapFeatureBased(ILau
 		launchPlugins.addAll(additionalPlugins.keySet());
 
 		if (addRequirements) {
-			// Add all missing  plug-ins required by the application/product set in the config
+			// Add all missing plug-ins required by the application/product set in the config
 			List<String> appRequirements = RequirementHelper.getApplicationLaunchRequirements(configuration);
 			RequirementHelper.addApplicationLaunchRequirements(appRequirements, configuration, launchPlugins, launchPlugins::add);
 
 			// Get all required plugins
-			Set<BundleDescription> additionalBundles = DependencyManager.getDependencies(launchPlugins);
-			for (BundleDescription bundle : additionalBundles) {
-				IPluginModelBase plugin = getRequiredPlugin(bundle.getSymbolicName(), bundle.getVersion().toString(), IMatchRules.PERFECT, defaultPluginResolution);
-				launchPlugins.add(Objects.requireNonNull(plugin));// should never be null
-			}
+			computeDependencies(launchPlugins, false, isWorkspace(defaultPluginResolution)).forEach(launchPlugins::add);
 		}
 
 		// Create the start levels for the selected plugins and add them to the map
@@ -546,6 +540,79 @@ private static void addBundleToMap(Map<IPluginModelBase, String> map, IPluginMod
 		map.put(bundle, startData);
 	}
 
+	// --- dependency resolution ---
+
+	private static Stream<IPluginModelBase> computeDependencies(Set<IPluginModelBase> includedPlugins, boolean includeOptional, boolean preferWorkspaceBundles) {
+		if (includedPlugins.isEmpty()) {
+			return Stream.empty();
+		}
+		// Create and resolve the new 'launch'-state where bundles explicitly included in the launch are preferred. Then compute the requirement closure on that 'launch'-state.
+		Map<BundleDescription, IPluginModelBase> launchBundlePlugins = new HashMap<>(includedPlugins.size() * 4 / 3 + 1);
+		Set<BundleDescription> launchBundles = reresolveBundlesPreferringIncludedBundles(includedPlugins, launchBundlePlugins, preferWorkspaceBundles);
+		State launchState = launchBundles.iterator().next().getContainingState();
+
+		DependencyManager.getImplicitDependencies().stream().map(descriptor -> {
+			String versionStr = descriptor.getVersion();
+			Version version = versionStr != null ? Version.parseVersion(versionStr) : null;
+			return launchState.getBundle(descriptor.getId(), version);
+		}).forEach(launchBundles::add);
+
+		DependencyManager.Options[] options = includeOptional //
+				? new DependencyManager.Options[] {DependencyManager.Options.INCLUDE_OPTIONAL_DEPENDENCIES}
+				: new DependencyManager.Options[] {};
+		Set<BundleDescription> closure = DependencyManager.findRequirementsClosure(launchBundles, options);
+		return closure.stream().map(launchBundlePlugins::get).map(Objects::requireNonNull) //
+				.filter(p -> !includedPlugins.contains(p));
+	}
+
+	private static Set<BundleDescription> reresolveBundlesPreferringIncludedBundles(Set<IPluginModelBase> includedPlugins, Map<BundleDescription, IPluginModelBase> launchBundlePlugins, boolean preferWorkspaceBundles) {
+		PluginModelManager modelManager = PDECore.getDefault().getModelManager();
+		State tpState = modelManager.getState().getState();
+
+		State launchState = BundleHelper.getPlatformAdmin().getFactory().createState(true);
+		launchState.setPlatformProperties(tpState.getPlatformProperties());
+
+		// Collect all bundles explicitly included in the launch
+		for (IPluginModelBase plugin : includedPlugins) {
+			addPluginBundle(plugin, launchState, launchBundlePlugins, tpState);
+		}
+		Set<BundleDescription> launchBundles = new HashSet<>(launchBundlePlugins.keySet());
+
+		// Iterate workspace- and TP-models separately to avoid shadowing of TP models by workspace models
+		Stream.of(modelManager.getWorkspaceModels(), modelManager.getExternalModels()).flatMap(Arrays::stream) //
+				.filter(IPluginModelBase::isEnabled).filter(p -> !includedPlugins.contains(p)) //
+				.forEach(plugin -> addPluginBundle(plugin, launchState, launchBundlePlugins, tpState));
+
+		launchState.getResolver().setSelectionPolicy(Comparator
+				// prefer bundles explicitly included in the launch 
+				.comparing((BaseDescription d) -> !launchBundles.contains(d.getSupplier())) //false<true
+				.thenComparing(d -> { // choose bundles originating from the preferred location (workspace or TP)
+					boolean isWorkspaceBundle = launchBundlePlugins.get(d.getSupplier()).getUnderlyingResource() != null;
+					return isWorkspaceBundle != preferWorkspaceBundles; //false<true
+				}).thenComparing(tpState.getResolver().getSelectionPolicy()));
+
+		launchState.resolve(false);
+		return launchBundles;
+	}
+
+	private static void addPluginBundle(IPluginModelBase plugin, State launchState, Map<BundleDescription, IPluginModelBase> launchBundlePlugin, State tpState) {
+		BundleDescription bundle = plugin.getBundleDescription();
+		if (bundle != null) {
+			if (bundle.getContainingState() != tpState) { // consistency check
+				throw new IllegalStateException("Plugins have different TP state"); //$NON-NLS-1$
+			}
+			BundleDescription launchBundle = launchState.getFactory().createBundleDescription(bundle);
+			if (!launchState.addBundle(launchBundle)) {
+				throw new IllegalStateException("Failed to add bundle to launch state: " + launchBundle); //$NON-NLS-1$
+			}
+			if (launchBundlePlugin.put(launchBundle, plugin) != null) {
+				throw new IllegalStateException("Duplicated launch bundle for plugin: " + plugin); //$NON-NLS-1$
+			}
+		}
+	}
+
+	// -- start data ---
+
 	public static String getStartData(BundleDescription desc, String defaultStartData) {
 		String runLevel = resolveSystemRunLevelText(desc);
 		String auto = resolveSystemAutoText(desc);

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/classpathresolver/ClasspathResolverTest.java

@@ -357,8 +357,7 @@ private static <V> V setStaticField(Class<?> cl, String fieldName, V newValue) t
 	@SafeVarargs
 	private static void createWorkspacePluginProjects(
 			Entry<NameVersionDescriptor, Map<String, String>>... workspacePlugins) throws CoreException {
-		Set<NameVersionDescriptor> descriptions = Map.ofEntries(workspacePlugins).keySet();
-		List<IProject> pluginProjects = ProjectUtils.createWorkspacePluginProjects(descriptions);
+		List<IProject> pluginProjects = ProjectUtils.createWorkspacePluginProjects(Map.ofEntries(workspacePlugins));
 		while (pluginProjects.stream().anyMatch(ClasspathResolverTest::isUpdatePending)) {
 			Thread.yield(); // await async classpath update of projects
 		}

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/FeatureBasedLaunchTest.java

@@ -29,6 +29,8 @@
 import static org.eclipse.pde.ui.tests.util.ProjectUtils.createPluginProject;
 import static org.eclipse.pde.ui.tests.util.TargetPlatformUtil.bundle;
 import static org.eclipse.pde.ui.tests.util.TargetPlatformUtil.resolution;
+import static org.osgi.framework.Constants.EXPORT_PACKAGE;
+import static org.osgi.framework.Constants.IMPORT_PACKAGE;
 import static org.osgi.framework.Constants.REQUIRE_BUNDLE;
 import static org.osgi.framework.Constants.RESOLUTION_OPTIONAL;
 
@@ -1377,6 +1379,71 @@ public void testGetMergedBundleMap_automaticallyAddRequirements() throws Throwab
 				targetBundle("plugin.z", "1.0.0")));
 	}
 
+	@Test
+	public void testGetMergedBundleMap_automaticallyAddRequirements_multipleProviders() throws Throwable {
+		// Test that, in case multiple-providers of a capability exists, the one
+		// explicitly included into the launch is preferred, when adding
+		// required dependencies
+
+		var targetBundles = Map.ofEntries( //
+				bundle("plugin.a", "1.0.0", //
+						entry(IMPORT_PACKAGE, "pack.a")), //
+
+				bundle("plugin.x", "1.0.0", //
+						entry(EXPORT_PACKAGE, "pack.a")), //
+				bundle("plugin.y", "1.0.0", //
+						entry(EXPORT_PACKAGE, "pack.a")));
+
+		createPluginProject("plugin.z", "1.0.0", Map.ofEntries(//
+				entry(EXPORT_PACKAGE, "pack.a")));
+
+		List<NameVersionDescriptor> targetFeatures = List.of( //
+				targetFeature("feature.a", "1.0.0", f -> {
+					addIncludedPlugin(f, "plugin.a", "1.0.0");
+					addIncludedPlugin(f, "plugin.x", "1.0.0");
+				}), //
+				targetFeature("feature.b", "1.0.0", f -> {
+					addIncludedPlugin(f, "plugin.a", "1.0.0");
+					addIncludedPlugin(f, "plugin.y", "1.0.0");
+				}), //
+				targetFeature("feature.c", "1.0.0", f -> {
+					addIncludedPlugin(f, "plugin.a", "1.0.0");
+				}));
+
+		setTargetPlatform(targetBundles, targetFeatures);
+
+		ILaunchConfigurationWorkingCopy wc = createFeatureLaunchConfig();
+		wc.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, true);
+		wc.setAttribute(IPDELauncherConstants.FEATURE_PLUGIN_RESOLUTION, IPDELauncherConstants.LOCATION_WORKSPACE); // default
+
+		wc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.a:external"));
+		assertGetMergedBundleMap(wc, Set.of(//
+				targetBundle("plugin.a", "1.0.0"), //
+				targetBundle("plugin.x", "1.0.0")));
+
+		// The second feature included the other provider, so it's expected that
+		// only that one is included
+		wc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.b:external"));
+		assertGetMergedBundleMap(wc, Set.of(//
+				targetBundle("plugin.a", "1.0.0"), //
+				targetBundle("plugin.y", "1.0.0")));
+
+		// If FEATURE_PLUGIN_RESOLUTION=LOCATION_WORKSPACE, bundles originating
+		// from the workspace are to prefer:
+		wc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.c:external"));
+		assertGetMergedBundleMap(wc, Set.of(//
+				targetBundle("plugin.a", "1.0.0"), //
+				workspaceBundle("plugin.z", "1.0.0")));
+
+		// If FEATURE_PLUGIN_RESOLUTION=LOCATION_EXTERNAL, bundles originating
+		// from the TP are to prefer. And without other distinction the bundle
+		// with the higher version and then 'lower' id is preferred.
+		wc.setAttribute(IPDELauncherConstants.FEATURE_PLUGIN_RESOLUTION, IPDELauncherConstants.LOCATION_EXTERNAL);
+		assertGetMergedBundleMap(wc, Set.of(//
+				targetBundle("plugin.a", "1.0.0"), //
+				targetBundle("plugin.x", "1.0.0")));
+	}
+
 	static Map<BundleLocationDescriptor, String> getEclipseAppRequirementClosureForRunningPlatform(
 			DependencyManager.Options... closureOptions) throws Exception {
 		// ensure app requirements are registered (done at class initialization)

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/LaunchConfigurationMigrationTest.java

@@ -34,8 +34,8 @@ public class LaunchConfigurationMigrationTest extends AbstractLaunchTest {
 
 	@BeforeClass
 	public static void setupPluginProjects() throws Exception {
-		ProjectUtils.createPluginProject("org.eclipse.pde.plugin1", "org.eclipse.pde.plugin1", null);
-		ProjectUtils.createPluginProject("org.eclipse.pde.plugin2", "org.eclipse.pde.plugin2", null);
+		ProjectUtils.createPluginProject("org.eclipse.pde.plugin1", "org.eclipse.pde.plugin1", "0.0.0");
+		ProjectUtils.createPluginProject("org.eclipse.pde.plugin2", "org.eclipse.pde.plugin2", "0.0.0");
 	}
 
 	@Test

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/PluginBasedLaunchTest.java

@@ -22,6 +22,8 @@
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.osgi.framework.Constants.EXPORT_PACKAGE;
+import static org.osgi.framework.Constants.IMPORT_PACKAGE;
 import static org.osgi.framework.Constants.REQUIRE_BUNDLE;
 import static org.osgi.framework.Constants.RESOLUTION_OPTIONAL;
 
@@ -869,6 +871,66 @@ public void testGetMergedBundleMap_automaticallyAddRequirements() throws Excepti
 				targetBundle("plugin.b", "1.0.0")))));
 	}
 
+	@Test
+	public void testGetMergedBundleMap_automaticallyAddRequirements_multipleProviders() throws Exception {
+
+		var targetPlatformBundles = Map.ofEntries( //
+				bundle("plugin.a", "1.0.0", //
+						entry(IMPORT_PACKAGE, "pack.a")), //
+
+				bundle("plugin.x", "1.0.0", //
+						entry(EXPORT_PACKAGE, "pack.a")), //
+				bundle("plugin.y", "1.0.0", //
+						entry(EXPORT_PACKAGE, "pack.a")));
+
+		var workspacePlugins = ofEntries( //
+				bundle("plugin.z", "1.0.0", //
+						entry(EXPORT_PACKAGE, "pack.a")) //
+				);
+
+		Consumer<ILaunchConfigurationWorkingCopy> basicSetup = wc -> {
+			wc.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, true);
+		};
+		{ // three providers are available and none is yet included
+			// -> the one originating from the workspace is preferred
+			Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
+				basicSetup.accept(wc);
+				wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES, Set.of("plugin.a*1.0.0"));
+			};
+			Set<BundleLocationDescriptor> expectedBundles = Set.of( //
+					targetBundle("plugin.a", "1.0.0"), //
+					workspaceBundle("plugin.z", "1.0.0"));
+
+			assertGetMergedBundleMap(workspacePlugins, targetPlatformBundles, launchConfigSetup, expectedBundles);
+		}
+		{ // three providers are available and one is included
+			// -> select included one
+			Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
+				basicSetup.accept(wc);
+				wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES,
+						Set.of("plugin.a*1.0.0", "plugin.x*1.0.0"));
+			};
+			Set<BundleLocationDescriptor> expectedBundles = Set.of( //
+					targetBundle("plugin.a", "1.0.0"), //
+					targetBundle("plugin.x", "1.0.0"));
+
+			assertGetMergedBundleMap(workspacePlugins, targetPlatformBundles, launchConfigSetup, expectedBundles);
+		}
+		{// three providers are available and one is included
+			// -> select included one
+			Consumer<ILaunchConfigurationWorkingCopy> launchConfigSetup = wc -> {
+				basicSetup.accept(wc);
+				wc.setAttribute(IPDELauncherConstants.SELECTED_TARGET_BUNDLES,
+						Set.of("plugin.a*1.0.0", "plugin.y*1.0.0"));
+			};
+			Set<BundleLocationDescriptor> expectedBundles = Set.of( //
+					targetBundle("plugin.a", "1.0.0"), //
+					targetBundle("plugin.y", "1.0.0"));
+
+			assertGetMergedBundleMap(workspacePlugins, targetPlatformBundles, launchConfigSetup, expectedBundles);
+		}
+	}
+
 	@Test
 	public void testTwoVersionsOfSameBundleConfigIni() throws Exception {
 		var workspacePlugins = ofEntries( //
@@ -1033,7 +1095,7 @@ private static void assertGetMergedBundleMap(Consumer<ILaunchConfigurationWorkin
 
 	private void setUpWorkspace(Map<NameVersionDescriptor, Map<String, String>> workspacePlugins,
 			Map<NameVersionDescriptor, Map<String, String>> targetPlugins) throws Exception {
-		ProjectUtils.createWorkspacePluginProjects(workspacePlugins.keySet());
+		ProjectUtils.createWorkspacePluginProjects(workspacePlugins);
 		TargetPlatformUtil.setDummyBundlesAsTarget(targetPlugins, List.of(), tpJarDirectory);
 	}
 
@@ -1049,8 +1111,7 @@ private static ILaunchConfigurationWorkingCopy createPluginLaunchConfig(String n
 
 	private static final Pattern WHITESPACE = Pattern.compile("\\s+");
 
-	private Path getConfigurationFolder(ILaunchConfigurationWorkingCopy launchConfig)
-			throws CoreException {
+	private Path getConfigurationFolder(ILaunchConfigurationWorkingCopy launchConfig) throws CoreException {
 		ILaunch launch = new Launch(launchConfig, ILaunchManager.RUN_MODE, null);
 		var config = new EclipseApplicationLaunchConfiguration();
 		String commandLine = config.showCommandLine(launchConfig, ILaunchManager.RUN_MODE, launch, null);