Bug 576888 - Consider included child-features for feature-launches
Change-Id: I32789dba25c16805085c34a52e633222046e0380
Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
Reviewed-on: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/186946
Tested-by: PDE Bot <pde-bot@eclipse.org>
diff --git a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
index 7eaa6ef..a34419c 100644
--- a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
+++ b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
@@ -15,6 +15,7 @@
* Hannes Wellmann - Bug 577118 - Handle multiple Plug-in versions in launching facility
* Hannes Wellmann - Bug 576886 - Clean up and improve BundleLaunchHelper and extract String literal constants
* Hannes Wellmann - Bug 576887 - Handle multiple versions of features and plug-ins for feature-launches
+ * Hannes Wellmann - Bug 576888 - Consider included child-features for feature-launches
*******************************************************************************/
package org.eclipse.pde.internal.launching.launcher;
@@ -171,15 +172,25 @@
Set<String> selectedFeatures = configuration.getAttribute(IPDELauncherConstants.SELECTED_FEATURES, emptySet());
Map<IFeature, String> feature2pluginResolution = new HashMap<>();
+ Queue<IFeature> pendingFeatures = new ArrayDeque<>();
for (String currentSelected : selectedFeatures) {
String[] attributes = currentSelected.split(FEATURE_PLUGIN_RESOLUTION_SEPARATOR);
if (attributes.length > 1) {
String id = attributes[0];
String pluginResolution = attributes[1];
IFeature feature = getFeature(id, featureMaps);
- if (feature != null) {
- feature2pluginResolution.put(feature, pluginResolution);
- }
+ addFeatureIfAbsent(feature, pluginResolution, feature2pluginResolution, pendingFeatures); // feature should be absent
+ }
+ }
+
+ while (!pendingFeatures.isEmpty()) { // perform exhaustive breath-first-search for included features
+ IFeature feature = pendingFeatures.remove();
+ String pluginResolution = feature2pluginResolution.get(feature); // inherit resolution from including feature
+
+ IFeatureChild[] includedFeatures = feature.getIncludedFeatures();
+ for (IFeatureChild featureChild : includedFeatures) {
+ IFeature child = getIncludedFeature(featureChild.getId(), featureChild.getVersion(), featureMaps);
+ addFeatureIfAbsent(child, pluginResolution, feature2pluginResolution, pendingFeatures);
}
}
return feature2pluginResolution;
@@ -204,6 +215,13 @@
return getMaxElement(features, f -> true, comparing(f -> Version.parseVersion(f.getVersion())));
}
+ private static void addFeatureIfAbsent(IFeature feature, String resolution, Map<IFeature, String> featurePluginResolution, Queue<IFeature> pendingFeatures) {
+ if (feature != null && featurePluginResolution.putIfAbsent(feature, resolution) == null) {
+ // Don't add feature more than once to not override the resolution if already present (e.g. a child was specified explicitly)
+ pendingFeatures.add(feature); // ... and to not process it more than once
+ }
+ }
+
private static boolean isWorkspace(String location) {
if (IPDELauncherConstants.LOCATION_WORKSPACE.equalsIgnoreCase(location)) {
return true;
@@ -213,6 +231,13 @@
throw new IllegalArgumentException("Unsupported location: " + location); //$NON-NLS-1$
}
+ private static final Comparator<IFeature> NEUTRAL_COMPARATOR = comparing(f -> 0);
+
+ private static IFeature getIncludedFeature(String id, String version, Map<String, List<List<IFeature>>> prioritizedFeatures) {
+ List<List<IFeature>> features = prioritizedFeatures.get(id);
+ return getIncluded(features, f -> true, IFeature::getVersion, NEUTRAL_COMPARATOR, version);
+ }
+
private static final Predicate<IPluginModelBase> ENABLED_VALID_PLUGIN_FILTER = p -> p.getBundleDescription() != null && p.isEnabled();
private static final Function<IPluginModelBase, String> GET_PLUGIN_VERSION = m -> m.getPluginBase().getVersion();
private static final Comparator<IPluginModelBase> COMPARE_PLUGIN_RESOLVED = comparing(p -> p.getBundleDescription().isResolved());
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/FeatureBasedLaunchTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/FeatureBasedLaunchTest.java
index a31b815..20d3a8d 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/FeatureBasedLaunchTest.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/FeatureBasedLaunchTest.java
@@ -393,6 +393,184 @@
}
}
+ // --- included features ---
+
+ @Test
+ public void testGetMergedBundleMap_includedFeatureWithDefaultVersion() throws Throwable {
+ // plug-in names contain version-like suffixes to have no chance to
+ // interfere with conveniences of plug-in resolution
+ List<NameVersionDescriptor> targetBundles = List.of( //
+ bundle("plugin.a.e.100", "1.0.0"), //
+ bundle("plugin.a.w.101", "1.0.0"), //
+ bundle("plugin.z", "1.0.0"));
+
+ createFeatureProject("feature.a", "1.0.3", f -> {
+ addIncludedPlugin(f, "plugin.a.w.101", "1.0.0");
+ });
+ createFeatureProject("feature.a", "1.0.0", f -> {
+ addIncludedPlugin(f, "plugin.z", "1.0.0");
+ });
+
+ List<NameVersionDescriptor> targetFeatures = List.of( //
+ targetFeature("feature.z", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.a", DEFAULT_VERSION);
+ }), //
+
+ targetFeature("feature.a", "1.0.2", f -> {
+ addIncludedPlugin(f, "plugin.a.e.100", "1.0.0");
+ }), //
+ targetFeature("feature.a", "1.0.1", f -> {
+ addIncludedPlugin(f, "plugin.z", "1.0.0");
+ }));
+
+ setTargetPlatform(targetBundles, targetFeatures);
+
+ {
+ ILaunchConfigurationWorkingCopy lcW = createFeatureLaunchConfig();
+ lcW.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.z:default"));
+ lcW.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
+
+ assertGetMergedBundleMap("feature-location workspace", lcW, Set.of( //
+ targetBundle("plugin.a.w.101", "1.0.0")));
+ }
+ {
+ ILaunchConfigurationWorkingCopy lcE = createFeatureLaunchConfig();
+ lcE.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.z:default"));
+ lcE.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_EXTERNAL);
+
+ assertGetMergedBundleMap("feature-location external", lcE, Set.of( //
+ targetBundle("plugin.a.e.100", "1.0.0")));
+ }
+ }
+
+ @Test
+ public void testGetMergedBundleMap_includedFeatureWithSpecificVersion() throws Throwable {
+ // plug-in names contain version-like suffixes to have no chance to
+ // interfere with conveniences of plug-in resolution
+ List<NameVersionDescriptor> targetBundles = List.of( //
+ bundle("plugin.a.w.100", "1.0.0"), //
+ bundle("plugin.a.w.300", "1.0.0"), //
+ bundle("plugin.a.e.100", "1.0.0"), //
+ bundle("plugin.a.e.200", "1.0.0"), //
+ bundle("plugin.a.e.400", "1.0.0"), //
+ bundle("plugin.z", "1.0.0"));
+
+ createFeatureProject("feature.a", "1.0.0", f -> {
+ addIncludedPlugin(f, "plugin.a.w.100", "1.0.0");
+ });
+ createFeatureProject("feature.a", "3.0.0", f -> {
+ addIncludedPlugin(f, "plugin.a.w.300", "1.0.0");
+ });
+
+ List<NameVersionDescriptor> targetFeatures = List.of( //
+ targetFeature("feature.a", "1.0.0", f -> {
+ addIncludedPlugin(f, "plugin.a.e.100", "1.0.0");
+ }), //
+ targetFeature("feature.a", "2.0.0", f -> {
+ addIncludedPlugin(f, "plugin.a.e.200", "1.0.0");
+ }), //
+ targetFeature("feature.a", "4.0.0", f -> {
+ addIncludedPlugin(f, "plugin.a.e.400", "1.0.0");
+ }), //
+
+ targetFeature("feature.b", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.a", "1.0.0");
+ }), //
+ targetFeature("feature.c", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.a", "1.2.0");
+ }), //
+ targetFeature("feature.d", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.a", "2.0.0");
+ }), //
+ targetFeature("feature.e", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.a", "3.0.0");
+ }), //
+ targetFeature("feature.f", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.a", "1.0.0.someQualifier");
+ }));
+
+ setTargetPlatform(targetBundles, targetFeatures);
+
+ // Perfect version-match in primary location (while secondary location
+ // has matches too)
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.b:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
+
+ assertGetMergedBundleMap("feature-location workspace", lc, Set.of( //
+ targetBundle("plugin.a.w.100", "1.0.0")));
+ }
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.b:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_EXTERNAL);
+
+ assertGetMergedBundleMap("feature-location external", lc, Set.of( //
+ targetBundle("plugin.a.e.100", "1.0.0")));
+ }
+ // Unqualified version-match in primary location
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.f:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
+
+ assertGetMergedBundleMap("feature-location workspace", lc, Set.of( //
+ targetBundle("plugin.a.w.100", "1.0.0")));
+ }
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.f:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_EXTERNAL);
+
+ assertGetMergedBundleMap("feature-location external", lc, Set.of( //
+ targetBundle("plugin.a.e.100", "1.0.0")));
+ }
+ // no version-match at all (for included features the latest features of
+ // a location is added if there is no match in the primary location)
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.c:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
+
+ assertGetMergedBundleMap("feature-location workspace no match", lc, Set.of( //
+ targetBundle("plugin.a.w.300", "1.0.0")));
+ }
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.c:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_EXTERNAL);
+
+ assertGetMergedBundleMap("feature-location external no match", lc, Set.of( //
+ targetBundle("plugin.a.e.400", "1.0.0")));
+ }
+ // Perfect version match only in secondary location (but another version
+ // is present in the primary too).
+ // To be able to conveniently override a specific version of a feature,
+ // which is actually included (transitively) by a feature from the TP,
+ // by a feature from the workspace (which happens frequently when
+ // one has just updated the feature version) the latest feature from the
+ // primary location is taken if one is present and none matches the
+ // specified version exactly (with or without considering qualifiers).
+ // See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=576887
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.d:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_WORKSPACE);
+
+ assertGetMergedBundleMap("feature-location workspace but exact match is external", lc, Set.of( //
+ targetBundle("plugin.a.w.300", "1.0.0")));
+ }
+ {
+ ILaunchConfigurationWorkingCopy lc = createFeatureLaunchConfig();
+ lc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of("feature.e:default"));
+ lc.setAttribute(IPDELauncherConstants.FEATURE_DEFAULT_LOCATION, IPDELauncherConstants.LOCATION_EXTERNAL);
+
+ assertGetMergedBundleMap("feature-location external but exact match is in workspace", lc, Set.of(//
+ targetBundle("plugin.a.e.400", "1.0.0")));
+ }
+ }
+
// --- required/imported plug-in dependencies ---
@Test
@@ -659,7 +837,8 @@
"feature.d:default"));
assertGetMergedBundleMap(wc, Set.of( //
- targetBundle("plugin.a", "1.0.0")));
+ targetBundle("plugin.a", "1.0.0"), //
+ targetBundle("plugin.z", "1.0.0")));
}
@Test
@@ -717,6 +896,45 @@
targetBundle("plugin.f", "2.0.0"), "6:true"));
}
+ @Test
+ public void testGetMergedBundleMap_inheritanceOfPluginResolution() throws Throwable {
+ createPluginProject("plugin.a", "1.0.0");
+ createPluginProject("plugin.b", "1.0.0");
+ createPluginProject("plugin.d", "1.0.0");
+ createPluginProject("plugin.e", "1.0.0");
+
+ List<NameVersionDescriptor> targetBundles = List.of( //
+ bundle("plugin.a", "1.0.0"), //
+ bundle("plugin.b", "1.0.0"), //
+ bundle("plugin.c", "1.0.0"), //
+ bundle("plugin.d", "1.0.0"), //
+ bundle("plugin.e", "1.0.0"), //
+ bundle("plugin.z", "1.0.0"));
+
+ List<NameVersionDescriptor> targetFeatures = List.of( //
+ targetFeature("feature.a", "1.0.0", f -> {
+ addIncludedFeature(f, "feature.b", "1.0.0");
+ addIncludedFeature(f, "feature.d", "1.0.0");
+ }), //
+ targetFeature("feature.b", "1.0.0", f -> {
+ addIncludedPlugin(f, "plugin.b", "1.0.0");
+ }), //
+ targetFeature("feature.d", "1.0.0", f -> {
+ addIncludedPlugin(f, "plugin.d", "1.0.0");
+ }));
+
+ setTargetPlatform(targetBundles, targetFeatures);
+
+ ILaunchConfigurationWorkingCopy wc = createFeatureLaunchConfig();
+ wc.setAttribute(IPDELauncherConstants.SELECTED_FEATURES, Set.of( //
+ "feature.a:external", //
+ "feature.d:workspace"));
+
+ assertGetMergedBundleMap(wc, Set.of( //
+ targetBundle("plugin.b", "1.0.0"), //
+ workspaceBundle("plugin.d", "1.0.0")));
+ }
+
// --- utility methods ---
private static interface CoreConsumer<E> {
diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/LaunchAction.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/LaunchAction.java
index 627e717..d6477c1 100644
--- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/LaunchAction.java
+++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/LaunchAction.java
@@ -134,7 +134,9 @@
}
private void refreshFeatureLaunchAttributes(ILaunchConfigurationWorkingCopy wc, IPluginModelBase[] models) {
- Set<String> selectedFeatures = Arrays.stream(getUniqueFeatures())
+ FeatureModelManager fmm = PDECore.getDefault().getFeatureModelManager();
+ Set<String> selectedFeatures = Arrays.stream(fProduct.getFeatures()) //
+ .map(f -> fmm.findFeatureModel(f.getId(), f.getVersion())).filter(Objects::nonNull)
.map(m -> formatFeatureEntry(m.getFeature().getId(), IPDELauncherConstants.LOCATION_DEFAULT))
.collect(Collectors.toCollection(LinkedHashSet::new));
