Skip to content

Commit 90c4bc1

Browse files
laeubilaeubi
committed
Always consider fragments that specify a platform filter
A fragment can declare a Eclipse-PlatformFilter to be only installed on a given platform. If such fragment is currently wired to a host bundle, it is quite likely that fragment is needed but the host usually do not declare an Eclipse-ExtensibleAPI so we currently miss to add such fragment making the native code loading fails. This now adds some code to store the platform filter and if one is given the fragment is always included. Fix #1929
1 parent d9ad053 commit 90c4bc1

11 files changed

Lines changed: 103 additions & 6 deletions

File tree

‎ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathUtilCore.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,17 @@ public static boolean hasExtensibleAPI(IPluginModelBase model) {
224224
return false;
225225
}
226226

227+
public static String getPlatformFilter(IPluginModelBase model) {
228+
IPluginBase pluginBase = model.getPluginBase();
229+
if (pluginBase instanceof BundlePlugin plugin) {
230+
return plugin.getPlatformFilter();
231+
}
232+
if (pluginBase instanceof BundleFragment fragment) {
233+
return fragment.getPlatformFilter();
234+
}
235+
return null;
236+
}
237+
227238
public static boolean isPatchFragment(Resource desc) {
228239
IPluginModelBase model = PluginRegistry.findModel(desc);
229240
return model instanceof IFragmentModel i ? isPatchFragment(i.getFragment()) : false;

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,19 @@ public enum Options {
6868
* Specifies to include all non-test fragments into the closure (must
6969
* not be combined with {@link #INCLUDE_ALL_FRAGMENTS}).
7070
*/
71-
INCLUDE_NON_TEST_FRAGMENTS;
71+
INCLUDE_NON_TEST_FRAGMENTS,
72+
/**
73+
* Option that can be set to include native fragments, that are ones
74+
* that define an {@link ICoreConstants#PLATFORM_FILTER} in their
75+
* manifest.
76+
*/
77+
INCLUDE_PLATFORM_FRAGMENTS,
78+
/**
79+
* Option that can be set to include native fragments, that are ones
80+
* that define an {@link ICoreConstants#PLATFORM_FILTER} in their
81+
* manifest.
82+
*/
83+
INCLUDE_EXTENSIBLE_FRAGMENTS;
7284
}
7385

7486
/**
@@ -168,6 +180,8 @@ public static Set<BundleDescription> findRequirementsClosure(Collection<BundleDe
168180
boolean includeOptional = optionSet.contains(Options.INCLUDE_OPTIONAL_DEPENDENCIES);
169181
boolean includeAllFragments = optionSet.contains(Options.INCLUDE_ALL_FRAGMENTS);
170182
boolean includeNonTestFragments = optionSet.contains(Options.INCLUDE_NON_TEST_FRAGMENTS);
183+
boolean includeNativeFragments = optionSet.contains(Options.INCLUDE_PLATFORM_FRAGMENTS);
184+
boolean includeExtensibleFragments = optionSet.contains(Options.INCLUDE_EXTENSIBLE_FRAGMENTS);
171185
if (includeAllFragments && includeNonTestFragments) {
172186
throw new AssertionError("Cannot combine INCLUDE_ALL_FRAGMENTS and INCLUDE_NON_TEST_FRAGMENTS"); //$NON-NLS-1$
173187
}
@@ -188,14 +202,22 @@ public static Set<BundleDescription> findRequirementsClosure(Collection<BundleDe
188202
if (wiring == null || !wiring.isInUse()) {
189203
continue;
190204
}
191-
if (includeAllFragments || includeNonTestFragments || isExtensibleApi(bundle)) {
205+
if (includeAllFragments || includeNonTestFragments
206+
|| (includeExtensibleFragments && isExtensibleApi(bundle))) {
192207
// A fragment's host is already required by a wire
193208
for (BundleDescription fragment : bundle.getFragments()) {
194209
if (includeAllFragments || !isTestWorkspaceProject(fragment)) {
195210
addNewRequiredBundle(fragment, closure, pending);
196211
}
197212
}
198213
}
214+
if (includeNativeFragments) {
215+
for (BundleDescription fragment : bundle.getFragments()) {
216+
if (isNativeFragment(fragment) && !isTestWorkspaceProject(fragment)) {
217+
addNewRequiredBundle(fragment, closure, pending);
218+
}
219+
}
220+
}
199221

200222
if (isFragment(wiring.getRevision())) {
201223
// Requirements of a fragment are hosted at the host, which
@@ -231,6 +253,14 @@ public static Set<BundleDescription> findRequirementsClosure(Collection<BundleDe
231253
return closure;
232254
}
233255

256+
private static boolean isNativeFragment(BundleDescription fragment) {
257+
Object userObject = fragment.getUserObject();
258+
if (userObject instanceof IPluginModelBase model) {
259+
return ClasspathUtilCore.getPlatformFilter(model) != null;
260+
}
261+
return false;
262+
}
263+
234264
private static boolean isExtensibleApi(BundleDescription bundleDescription) {
235265
if (bundleDescription.getFragments().length == 0) {
236266
return false;

‎ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/PDEAuxiliaryState.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ static class PluginInfo {
9797
String localization;
9898
String bundleSourceEntry;
9999
boolean exportsExternalAnnotations;
100+
String platformFilter;
100101
}
101102

102103
/**
@@ -379,6 +380,7 @@ protected void addAuxiliaryData(BundleDescription desc, Map<String, String> mani
379380
info.libraries = getClasspath(manifest);
380381
info.hasExtensibleAPI = "true".equals(manifest.get(ICoreConstants.EXTENSIBLE_API)); //$NON-NLS-1$
381382
info.isPatchFragment = "true".equals(manifest.get(ICoreConstants.PATCH_FRAGMENT)); //$NON-NLS-1$
383+
info.platformFilter = manifest.get(ICoreConstants.PLATFORM_FILTER);
382384
info.localization = manifest.get(Constants.BUNDLE_LOCALIZATION);
383385
info.hasBundleStructure = hasBundleStructure;
384386
info.bundleSourceEntry = manifest.get(ICoreConstants.ECLIPSE_SOURCE_BUNDLE);

‎ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/bundle/BundleFragment.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,8 @@ public boolean isPatch() {
122122
return "true".equals(getValue(ICoreConstants.PATCH_FRAGMENT, false)); //$NON-NLS-1$
123123
}
124124

125+
public String getPlatformFilter() {
126+
return getValue(ICoreConstants.PLATFORM_FILTER, false);
127+
}
128+
125129
}

‎ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/bundle/BundlePlugin.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,8 @@ public boolean exportsExternalAnnotations() {
5757
return "true".equals(getValue(ICoreConstants.ECLIPSE_EXPORT_EXTERNAL_ANNOTATIONS, false)); //$NON-NLS-1$
5858
}
5959

60+
public String getPlatformFilter() {
61+
return getValue(ICoreConstants.PLATFORM_FILTER, false);
62+
}
63+
6064
}

‎ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/ILaunchingPreferenceConstants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ public interface ILaunchingPreferenceConstants {
2525

2626
// Main preference page
2727
public static final String PROP_AUTO_MANAGE = "Preferences.Launching.automanageDependencies"; //$NON-NLS-1$
28+
public static final String PROP_AUTO_MANAGE_EXTENSIBLE_FRAGMENTS = "Preferences.Launching.automanageDependencies.includeExtensibleFragments"; //$NON-NLS-1$
29+
public static final String PROP_AUTO_MANAGE_PLATFORM_FRAGMENTS = "Preferences.Launching.automanageDependencies.includePlatformFragments"; //$NON-NLS-1$
2830
public static final String PROP_RUNTIME_WORKSPACE_LOCATION = "Preferences.Launching.runtimeWorkspaceLocation"; //$NON-NLS-1$
2931
public static final String PROP_RUNTIME_WORKSPACE_LOCATION_IS_CONTAINER = "Preferences.Launching.runtimeWorkspaceLocationIsContainer"; //$NON-NLS-1$
3032
public static final String PROP_JUNIT_WORKSPACE_LOCATION = "Preferences.Launching.junitWorkspaceLocation"; //$NON-NLS-1$

‎ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/PreferenceInitializer.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ public void initializeDefaultPreferences() {
3636
prefs.putBoolean(ILaunchingPreferenceConstants.PROP_JUNIT_ADD_NEW_WORKSPACE_PLUGINS, false);
3737
prefs.putBoolean(ILaunchingPreferenceConstants.PROP_JUNIT_VALIDATE_LAUNCH, true);
3838
prefs.putBoolean(ILaunchingPreferenceConstants.ADD_SWT_NON_DISPOSAL_REPORTING, true);
39+
prefs.putBoolean(ILaunchingPreferenceConstants.PROP_AUTO_MANAGE_EXTENSIBLE_FRAGMENTS, true);
40+
prefs.putBoolean(ILaunchingPreferenceConstants.PROP_AUTO_MANAGE_PLATFORM_FRAGMENTS, true);
3941

4042
// copy over instance scope prefs from UI plugin
4143
IEclipsePreferences oldInstancePrefs = InstanceScope.INSTANCE.getNode(IPDEConstants.UI_PLUGIN_ID);

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import org.eclipse.pde.internal.core.DependencyManager;
6767
import org.eclipse.pde.internal.core.FeatureModelManager;
6868
import org.eclipse.pde.internal.core.PDECore;
69+
import org.eclipse.pde.internal.core.PDEPreferencesManager;
6970
import org.eclipse.pde.internal.core.PluginModelManager;
7071
import org.eclipse.pde.internal.core.TargetPlatformHelper;
7172
import org.eclipse.pde.internal.core.ifeature.IFeature;
@@ -74,6 +75,7 @@
7475
import org.eclipse.pde.internal.core.ifeature.IFeatureModel;
7576
import org.eclipse.pde.internal.core.ifeature.IFeaturePlugin;
7677
import org.eclipse.pde.internal.core.util.VersionUtil;
78+
import org.eclipse.pde.internal.launching.ILaunchingPreferenceConstants;
7779
import org.eclipse.pde.internal.launching.IPDEConstants;
7880
import org.eclipse.pde.internal.launching.PDELaunchingPlugin;
7981
import org.eclipse.pde.internal.launching.PDEMessages;
@@ -557,10 +559,18 @@ private static Stream<IPluginModelBase> computeDependencies(Set<IPluginModelBase
557559
return launchState.getBundle(descriptor.getId(), version);
558560
}).forEach(launchBundles::add);
559561

560-
DependencyManager.Options[] options = includeOptional //
561-
? new DependencyManager.Options[] {DependencyManager.Options.INCLUDE_OPTIONAL_DEPENDENCIES}
562-
: new DependencyManager.Options[] {};
563-
Set<BundleDescription> closure = DependencyManager.findRequirementsClosure(launchBundles, options);
562+
PDEPreferencesManager launchingStore = PDELaunchingPlugin.getDefault().getPreferenceManager();
563+
Set<DependencyManager.Options> options = new HashSet<>();
564+
if (includeOptional) {
565+
options.add(DependencyManager.Options.INCLUDE_OPTIONAL_DEPENDENCIES);
566+
}
567+
if (launchingStore.getBoolean(ILaunchingPreferenceConstants.PROP_AUTO_MANAGE_EXTENSIBLE_FRAGMENTS)) {
568+
options.add(DependencyManager.Options.INCLUDE_EXTENSIBLE_FRAGMENTS);
569+
}
570+
if (launchingStore.getBoolean(ILaunchingPreferenceConstants.PROP_AUTO_MANAGE_PLATFORM_FRAGMENTS)) {
571+
options.add(DependencyManager.Options.INCLUDE_PLATFORM_FRAGMENTS);
572+
}
573+
Set<BundleDescription> closure = DependencyManager.findRequirementsClosure(launchBundles, options.toArray(DependencyManager.Options[]::new));
564574
return closure.stream().map(launchBundlePlugins::get).map(Objects::requireNonNull) //
565575
.filter(p -> !includedPlugins.contains(p));
566576
}

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2711,6 +2711,12 @@ public class PDEUIMessages extends NLS {
27112711

27122712
public static String LaunchingPreferencePage_description;
27132713

2714+
public static String LaunchingPreferencePage_GroupComputingOptions;
2715+
2716+
public static String LaunchingPreferencePage_IncludeExtensibleFragments;
2717+
2718+
public static String LaunchingPreferencePage_IncludePlatformFragments;
2719+
27142720
public static String RemoveLazyLoadingDirectiveResolution_remove;
27152721

27162722
public static String RemoveAutomaticModuleResolution_remove;

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2193,6 +2193,9 @@ LauncherSection_launcherName=Launcher Name:
21932193
LauncherSection_dialogTitle=Image Selection
21942194
LauncherSection_dialogMessage=Select an image:
21952195
LaunchingPreferencePage_description=Settings for Plug-in launches
2196+
LaunchingPreferencePage_GroupComputingOptions=When computing required Plug-ins
2197+
LaunchingPreferencePage_IncludeExtensibleFragments=Include Fragment of Extensible-API Plug-ins
2198+
LaunchingPreferencePage_IncludePlatformFragments=Include Platform Specific Fragment
21962199
RemoveLazyLoadingDirectiveResolution_remove=Remove lazy activation header
21972200
RemoveAutomaticModuleResolution_remove=Remove automatic module name header
21982201
ProductDefinitonWizardPage_applicationDefinition=<form><p>An Eclipse product must be associated with an <a href="applications">application</a>, the default entry point for the product when it is running.</p></form>

0 commit comments

Comments
 (0)