Skip to content

org.eclipse.core.contenttype: osgi.extender=osgi.component requirement #2162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laeubi merged 1 commit into from
Sep 16, 2025
Merged

org.eclipse.core.contenttype: osgi.extender=osgi.component requirement #2162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion runtime/bundles/org.eclipse.core.contenttype/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.core.contenttype; singleton:=true
Bundle-Version: 3.9.700.qualifier
Bundle-Version: 3.9.800.qualifier
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Require-Bundle: org.eclipse.equinox.preferences;bundle-version="[3.2.0,4.0.0)",
Expand All @@ -22,3 +22,5 @@ Import-Package: javax.xml.parsers,
Bundle-RequiredExecutionEnvironment: JavaSE-17
Automatic-Module-Name: org.eclipse.core.contenttype
Service-Component: OSGI-INF/org.eclipse.core.internal.content.ContentTypeManager.xml
Require-Capability: osgi.extender;
filter:="(&(osgi.extender=osgi.component)(version>=1.2)(!(version>=2.0)))"
Copy link
Copy Markdown
Contributor

@laeubi laeubi Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project settings say we use 1.4 (but currently only 1.1 features are used) so maybe we should just increase to 1.4 now to be safe in the future. On the other hand one likely will not find any compatible SCR impl for 1.2 anymore anyways :-)

Copy link
Copy Markdown
Member Author

@sratz sratz Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this from the neighboring org.eclipse.e4.core.services for consistency.

3 changes: 2 additions & 1 deletion runtime/tests/org.eclipse.core.tests.harness/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Eclipse Core Tests Harness
Bundle-SymbolicName: org.eclipse.core.tests.harness;singleton:=true
Bundle-Version: 3.17.300.qualifier
Bundle-Version: 3.17.400.qualifier
Bundle-Vendor: Eclipse.org
Export-Package: org.eclipse.core.tests.harness;version="2.0",
org.eclipse.core.tests.harness.session,
org.eclipse.core.tests.session;version="2.0"
Require-Bundle: org.junit,
org.apache.felix.scr,
org.eclipse.test.performance,
org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)",
org.eclipse.jdt.junit.runtime,
Expand Down
25 changes: 23 additions & 2 deletions .../org/eclipse/core/tests/harness/session/customization/CustomSessionConfigurationImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
Expand All @@ -36,6 +37,8 @@
import org.osgi.framework.Constants;
import org.osgi.framework.FrameworkUtil;
import org.osgi.framework.Version;
import org.osgi.framework.wiring.BundleWire;
import org.osgi.framework.wiring.BundleWiring;

@SuppressWarnings("restriction")
public class CustomSessionConfigurationImpl implements CustomSessionConfiguration {
Expand All @@ -48,7 +51,7 @@ public class CustomSessionConfigurationImpl implements CustomSessionConfiguratio
private static final String PROP_SHARED_CONFIG_AREA = "osgi.sharedConfiguration.area";
private static final String TEMP_DIR_PREFIX = "eclipse_session_configuration";

private final Collection<BundleReference> bundleReferences = new ArrayList<>();
private final Collection<BundleReference> bundleReferences = new LinkedHashSet<>();
private Path configurationDirectory;
private boolean readOnly = false;
private boolean cascaded = false;
Expand All @@ -58,6 +61,18 @@ public CustomSessionConfigurationImpl() {
addMinimalBundleSet();
}

private static void collectDependencies(Bundle bundle, Collection<Bundle> dependencyClosure) {
if (!dependencyClosure.add(bundle)) {
return;
}
BundleWiring wiring = bundle.adapt(BundleWiring.class);
if (wiring != null) {
for (BundleWire wire : wiring.getRequiredWires(null)) {
collectDependencies(wire.getProviderWiring().getBundle(), dependencyClosure);
}
}
}

@SuppressWarnings("deprecation")
private void addMinimalBundleSet() {
// Just use any class from the bundles we want to add as minimal bundle set
Expand All @@ -71,6 +86,12 @@ private void addMinimalBundleSet() {
addBundle(org.eclipse.core.runtime.content.IContentType.class); // org.eclipse.core.contenttype
addBundle(org.eclipse.equinox.app.IApplication.class); // org.eclipse.equinox.app

// org.apache.felix.scr + dependencies
Bundle scrBundle = FrameworkUtil.getBundle(org.apache.felix.scr.info.ScrInfo.class);
Collection<Bundle> scrAndDependencies = new HashSet<>();
collectDependencies(scrBundle, scrAndDependencies);
Copy link
Copy Markdown
Contributor

@laeubi laeubi Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe better list dependencies (are there many?) explicitly to not pull in too much or optional things unintentionally?

Copy link
Copy Markdown
Member Author

@sratz sratz Sep 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's these:

org.apache.felix.scr
org.osgi.service.cm
org.eclipse.osgi
org.osgi.service.metatype
org.apache.felix.gogo.runtime
org.osgi.service.event
org.osgi.service.component
org.osgi.util.promise
org.osgi.util.function
org.osgi.util.promise

I thought about importing a package from each and specifying them each explicitly. But I am not sure how stable that would be and how often dependencies change here and we'd have to adapt.

Copy link
Copy Markdown
Contributor

@laeubi laeubi Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks safe to me.

Copy link
Copy Markdown
Member Author

@sratz sratz Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dependencies are in fact optional:

org.osgi.service.metatype
org.osgi.service.cm
org.apache.felix.gogo.runtime

but I guess this won't hurt - it's still pretty minimal and they don't depend on anything else we don't already need.

And it's easier to maintain like this.

Copy link
Copy Markdown
Member Author

@sratz sratz Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your gut feeling was right, this was indeed pulling in too much.
org.eclipse.osgi is used as the osgi.framework in the config.ini, so we should not redundantly add that to osgi.bundles as well, as this would cause error log entries.

scrAndDependencies.forEach(this::addBundle);

addBundle(org.eclipse.core.tests.harness.TestHarnessPlugin.class); // org.eclipse.core.tests.harness
addBundle(org.eclipse.test.performance.Performance.class); // org.eclipse.test.performance

Expand Down
Loading